-
-
Notifications
You must be signed in to change notification settings - Fork 91
Make notebook tests more reliable #2899
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
Conversation
useLegacyVscode || os.platform() === "win32" | ||
? undefined | ||
: [`--crash-reporter-directory=${crashDir}`, `--logsPath=${logsDir}`], | ||
launchArgs: ["--force-node-api-uncaught-exceptions-policy=true"], |
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 seems like a good thing but curious what prompted it
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.
Test-electron crashes on windows with edit code 0. So ci pass even though the tests crashes.
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.
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.
Looks like this argument isn't available in electron. Electron apparently enforces note options quite strictly.
Original problem is that test run crashes on windows with false negative.
https://github.com/cursorless-dev/cursorless/actions/runs/14509376075/job/40704611423?pr=2893
It appears that it's
vscode.window.showTextDocument
in ouropenNewEditor
utility that causes this. It's not always on the same test case. By default it crashes in the scope tests, but if I disable the scope test the same error occurs in our recorded tests that of course uses the sameopenNewEditor
function. I can't recreate this locally. Looks like a memory leak or similar.In this pull request I managed to get the notebook tests more reliable though so I think that is words merging and I will continue working on the crashing problem.
Continues here #2901