-
Notifications
You must be signed in to change notification settings - Fork 342
test: cleanup chakracore customization #580
Conversation
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.
@@ -21,4 +21,4 @@ async function runTests() { | |||
assert.strictEqual(0, (await instance.expectShutdown()).exitCode); | |||
} | |||
|
|||
runTests().catch((err) => { console.log(err); process.exit(-1); }); |
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.
Is this just to make us the same as upstream? I believe that we added this because without it, if the test fails it is often via a timeout with no additional information. I suppose we should just upstream that change though.
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 believe this was actually a problem before we had unhandled promise rejection handling. It should correctly print the unhandled rejection and exit with an error now.
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 it should do the right thing:
> .\Release\node.exe --expose-internals .\test\sequential\test-inspector-break-e.js
[test] Connecting to a child Node process
[test] Testing /json/list
[err] Debugger listening on ws://127.0.0.1:2957/8bb3dc33-f1f0-43be-98f3-f36d7e0e68d0
[err] For help, see: https://nodejs.org/en/docs/inspector
[err]
[err] Debugger attached.
[err]
Error: foo
at runTests (E:\GitHub\node-chakracore\test\sequential\test-inspector-break-e.js:19:3)
at Generator.prototype.next (native code)
at _tickCallback (internal/process/next_tick.js:68:7)
1
> $LASTEXITCODE
1
* Undo obsolete test modifications and enable more tests. * Remove unnecessary deltas with upstream * Fix lint issues PR-URL: nodejs#580 Reviewed-By: Seth Brenith <sethb@microsoft.com> Reviewed-By: Jimmy Thomson <jithomso@microsoft.com>
It looks like |
* Undo obsolete test modifications and enable more tests. * Remove unnecessary deltas with upstream * Fix lint issues PR-URL: nodejs#580 Reviewed-By: Seth Brenith <sethb@microsoft.com> Reviewed-By: Jimmy Thomson <jithomso@microsoft.com>
Undo obsolete test modifications and enable more tests.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes