-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
Run tests against VS Code as node process #50434
Conversation
@@ -66,6 +66,11 @@ async function exec(cmd, args, options = {}) { | |||
} | |||
exports.exec = exec; | |||
|
|||
async function execNode(args, options = {}) { | |||
return exec(process.execPath, [...process.execArgv, ...args], options); |
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.
Normally, you'd use fork to do this, but we have our own custom helper to run external programs, so to fix the fact that we don't pass execArgv
down, I added another.
(In a perfect world, I'd just get rid of this and use execa
.)
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
However, there is some good news; testing locally, |
@@ -0,0 +1,5 @@ | |||
const flag = "--ms-enable-electron-run-as-node"; |
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.
VS Code requiring this flag really screws with things, hence this terrible hook.
In theory you want to run the this in the full perf suite - not as much in CI, right? |
What would we be testing for VS Code? Performance or totally breaking? I was expecting the latter because flags would start overflowing and tests would fail. |
So a conforming implementation will guarantee the 32-bit semantics continue to work - I believe there's no "implementation-defined" behavior on numbers in this regard. The problem is more about the performance breakdown from automatic boxing and the associated deoptimizations once you go past the SMI upper limit. |
This is a goofy idea I thought of in #50382 (comment); VS Code runs typescript via
ELECTRON_RUN_IN_NODE
; with some trickery, we can create a shim which calls VS Code and pretends to be node.This way, we can (hopefully) be told if we introduce a problem that only affects our use in electron's node, e.g. if pointer compression is enabled and we overflow an integer or something.