-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: run nightly tests against latest vscode VSCODE-340 #543
test: run nightly tests against latest vscode VSCODE-340 #543
Conversation
src/test/runTest.ts
Outdated
@@ -17,6 +17,9 @@ async function main(): Promise<any> { | |||
|
|||
// Download VS Code, unzip it and run the integration test | |||
await runTests({ | |||
// Download latest stable release. | |||
// Might be different from the vscode engine specified in package.json | |||
version: 'stable', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would make all tests run against the stable
version of VSCode. If I understand the ticket correctly, should we instead only have this on the nightly
tests? The ones that are run on the schedule: https://github.com/mongodb-js/vscode/blob/975bfde1f0c9d1335ea2fc95308a00c950db297d/.github/workflows/test-and-build.yaml#LL11C7-L11C11
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't those also using runTest.ts? I thought this is the same test setup that runs npm run test
that goes through runTest.ts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or do you want only nightly tests to run again stable and others against the version in package.json? I think we do not always remember to update the engine in package.sjon so even for a local run this is fine to run against stable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good for us to run it against insiders
if the goal is to catch things early.
https://github.com/microsoft/vscode-test/blob/0ec222ef170e102244569064a12898fb203e5bb7/lib/download.ts#L170
I think running on stable vscode, while it is good to do, won't help us catch things until it's too late.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about this and don't mind using insiders if you prefer this, but even with stable it will help us to catch more bugs compared to what we have now, forgetting to update package.json :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trueee that 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we find insiders
to be unreliable for ci testing then we can move back to stable
.
Description
Run test suites on the latest stable release.
Checklist
Motivation and Context
Types of changes