test: run 'abort' suite on Windows by refack · Pull Request #15056 · nodejs/node · GitHub

For now a C++ node::Abort() failure exits with code 3 on Windows

Fixes: #14012
Refs: #14013

CI: https://ci.nodejs.org/job/node-test-commit/12055/

/cc @nodejs/platform-windows @nodejs/testing

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Affected core subsystem(s)

test,process

@nodejs/build PTAL do you think aborting on windows will somehow create core dumps (via the unpredictable diagsvc)?

New CI: https://ci.nodejs.org/job/node-test-commit/12056/

Looks like abort/test-process-abort-exitcode needs a change (failing on Windows in CI).

typo: process

Looks like there's a relevant test failure. Also very interested in whether we can turn off core files in test.py for Windows or not.

Also very interested in whether we can turn off core files in test.py for Windows or not.

Windows does not generate core files by default. So it's actually a matter to finding a way to turn them on (if we find a way to validate them)...

Windows does not generate core files by default.

Awesome. Then it's just the failing test...

@Trott done
Windows CI: https://ci.nodejs.org/job/node-test-commit-windows-fanned/11461/

LGTM if CI is green

PR-URL: nodejs#15056 Fixes: nodejs#14012 Refs: nodejs#14013 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: nodejs/node#15056 Fixes: nodejs/node#14012 Refs: nodejs/node#14013 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: #15056 Fixes: #14012 Refs: #14013 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

This was relying on a semver major change. If possible in future please appropriately tag dont-land, thanks.

It isn't relying per se. #13947 should have changed test-process-abort-exitcode.js on Windows to match the new behaviour -- Presumably this was missed because prior to this PR the abort tests weren't included in the CI runs.

Perhaps this PR should have been two commits -- one to unbreak the tests (i.e. the bit that is dependent on the semver-major change) and a separate commit to enable the abort tests in the list of testsuites to run on Windows (which should be backported if #14013 is backported otherwise there'll be a disparity between the tests that are run on Windows vs. everywhere else).

@richardlau please feel free to submit a backport if you think a partial change makes sense on 8.x

@MylesBorins Backport PR #15460 (since it's build/test you could land it irrespective of a new release)

PR-URL: nodejs#15056 Fixes: nodejs#14012 Refs: nodejs#14013 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: #15056 Fixes: #14012 Refs: #14013 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

Backport PR: #16442

PR-URL: nodejs#15056 Fixes: nodejs#14012 Refs: nodejs#14013 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

Backport-PR-URL: #16442 PR-URL: #15056 Fixes: #14012 Refs: #14013 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

refack added flaky-test Issues and PRs related to the tests with unstable failures on the CI. process Issues and PRs related to the process subsystem. test Issues and PRs related to the tests. windows Issues and PRs related to the Windows platform. labels Aug 28, 2017

refack self-assigned this Aug 28, 2017

refack requested review from Trott and gireeshpunathil August 28, 2017 02:39

nodejs-github-bot added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Aug 28, 2017

refack force-pushed the run-abort-on-windows branch from f0940f6 to 7634941 Compare August 28, 2017 03:41

addaleax approved these changes Aug 30, 2017

View reviewed changes

jasnell approved these changes Aug 30, 2017

View reviewed changes

Trott requested changes Sep 2, 2017

View reviewed changes

refack force-pushed the run-abort-on-windows branch from 7634941 to 98ba860 Compare September 2, 2017 23:25

Trott approved these changes Sep 3, 2017

View reviewed changes

refack force-pushed the run-abort-on-windows branch 2 times, most recently from 98d8db3 to 98ba860 Compare September 3, 2017 21:17

test,process: run 'abort' suite on Windows

233d1e2

PR-URL: nodejs#15056 Fixes: nodejs#14012 Refs: nodejs#14013 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

refack force-pushed the run-abort-on-windows branch from 98ba860 to 233d1e2 Compare September 3, 2017 21:20

refack merged commit 233d1e2 into nodejs:master Sep 3, 2017

refack mentioned this pull request Sep 3, 2017

PSA: started running the "abort" test suite on Windows nodejs/build#862

Closed

refack deleted the run-abort-on-windows branch September 4, 2017 21:53

MylesBorins mentioned this pull request Sep 10, 2017

v8.5.0 proposal #15308

Merged

MylesBorins added the dont-land-on-v8.x label Sep 11, 2017

MylesBorins mentioned this pull request Sep 11, 2017

deps: update V8 to 6.1 #14730

Closed

5 tasks

refack removed their assignment Sep 18, 2017

refack mentioned this pull request Sep 18, 2017

[v8.x backport] test,process: run 'abort' suite on Windows #15460

Closed

3 tasks

MylesBorins added backported-to-v8.x and removed dont-land-on-v8.x labels Oct 17, 2017

MylesBorins mentioned this pull request Oct 17, 2017

backport updates to windows build script to v6.x nodejs/Release#238

Closed

3 tasks

MylesBorins added the backport-requested-v6.x label Oct 17, 2017

refack mentioned this pull request Oct 24, 2017

[v6.x backport] test,process: run 'abort' suite on Windows #16442

Closed

2 tasks

This was referenced Nov 3, 2017

v4.8.6 proposal #16500

Merged

v6.12.0 proposal #16263

Merged

test: run 'abort' suite on Windows by refack · Pull Request #15056 · nodejs/node · GitHub

Uh oh!

Uh oh!

test: run 'abort' suite on Windows - #15056

test: run 'abort' suite on Windows#15056refack merged 1 commit intonodejs:masternodejs/node:masterfrom refack:run-abort-on-windowsrefack/node:run-abort-on-windowsCopy head branch name to clipboard

Conversation

refack commented Aug 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Affected core subsystem(s)

Uh oh!

refack commented Aug 28, 2017

Uh oh!

refack commented Aug 28, 2017

Uh oh!

Trott commented Aug 28, 2017

Uh oh!

addaleax Aug 30, 2017

Choose a reason for hiding this comment

Uh oh!

Trott left a comment

Choose a reason for hiding this comment

Uh oh!

refack commented Sep 2, 2017

Uh oh!

Trott commented Sep 2, 2017

Uh oh!

refack commented Sep 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Trott left a comment

Choose a reason for hiding this comment

Uh oh!

MylesBorins commented Sep 11, 2017

Uh oh!

richardlau commented Sep 11, 2017

Uh oh!

MylesBorins commented Sep 11, 2017

Uh oh!

refack commented Sep 18, 2017

Uh oh!

MylesBorins commented Oct 17, 2017

Uh oh!

refack commented Oct 24, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

test: run 'abort' suite on Windows#15056
refack merged 1 commit into
nodejs:masterfrom
refack:run-abort-on-windows

refack commented Aug 28, 2017 •
edited
Loading

refack commented Sep 2, 2017 •
edited
Loading