Skip to content
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: refactor test-vm-sigint #13902

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Jun 24, 2017

  • Use common.mustNotCall() to confirm SIGINT listeners are not being
    invoked.
  • Improve assertion check on integer child argument.
  • Add blank line per test writing guide.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test vm

* Use common.mustNotCall() to confirm SIGINT listeners are not being
  invoked.
* Improve assertion check on integer child argument.
* Add blank line per test writing guide.
@Trott Trott added test Issues and PRs related to the tests. vm Issues and PRs related to the vm subsystem. labels Jun 24, 2017
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jun 24, 2017
@Trott
Copy link
Member Author

Trott commented Jun 24, 2017

Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only test failure looks unrelated.

@Trott
Copy link
Member Author

Trott commented Jun 29, 2017

Closed in 99dc9d0

@Trott Trott closed this Jun 29, 2017
Trott added a commit that referenced this pull request Jun 29, 2017
* Use common.mustNotCall() to confirm SIGINT listeners are not being
  invoked.
* Improve assertion check on integer child argument.
* Add blank line per test writing guide.

PR-URL: #13902
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Jun 29, 2017
* Use common.mustNotCall() to confirm SIGINT listeners are not being
  invoked.
* Improve assertion check on integer child argument.
* Add blank line per test writing guide.

PR-URL: #13902
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@addaleax addaleax mentioned this pull request Jun 29, 2017
addaleax pushed a commit that referenced this pull request Jul 11, 2017
* Use common.mustNotCall() to confirm SIGINT listeners are not being
  invoked.
* Improve assertion check on integer child argument.
* Add blank line per test writing guide.

PR-URL: #13902
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Jul 18, 2017
* Use common.mustNotCall() to confirm SIGINT listeners are not being
  invoked.
* Improve assertion check on integer child argument.
* Add blank line per test writing guide.

PR-URL: #13902
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

This does not land cleanly in LTS. Please feel free to manually backport by following the guide. Please also feel free to replace do-not-land if it is being backported

It is worth mentioning that after fixing the conflicts there were a bunch of errors on v6.x

=== release test-vm-sigint === Path: parallel/test-vm-sigint /Users/mborins/code/node/v6.x/test/parallel/test-vm-sigint.js:23 assert.ok(Number.isInteger(listeners)); ^

ReferenceError: listeners is not defined
at Object. (/Users/mborins/code/node/v6.x/test/parallel/test-vm-sigint.js:23:30)
at Module._compile (module.js:570:32)
at Object.Module._extensions..js (module.js:579:10)
at Module.load (module.js:487:32)
at tryModuleLoad (module.js:446:12)
at Function.Module._load (module.js:438:3)
at Module.runMain (module.js:604:10)
at run (bootstrap_node.js:389:7)
at startup (bootstrap_node.js:149:9)
at bootstrap_node.js:502:3
/Users/mborins/code/node/v6.x/test/parallel/test-vm-sigint.js:23
assert.ok(Number.isInteger(listeners));
^

ReferenceError: listeners is not defined
at Object. (/Users/mborins/code/node/v6.x/test/parallel/test-vm-sigint.js:23:30)
at Module._compile (module.js:570:32)
at Object.Module._extensions..js (module.js:579:10)
at Module.load (module.js:487:32)
at tryModuleLoad (module.js:446:12)
at Function.Module._load (module.js:438:3)
at Module.runMain (module.js:604:10)
at run (bootstrap_node.js:389:7)
at startup (bootstrap_node.js:149:9)
at bootstrap_node.js:502:3

assert.js:81
throw new assert.AssertionError({
^
AssertionError: 1 === 0
at ChildProcess.child.on.common.mustCall (/Users/mborins/code/node/v6.x/test/parallel/test-vm-sigint.js:51:12)
at ChildProcess. (/Users/mborins/code/node/v6.x/test/common/index.js:464:15)
at emitTwo (events.js:106:13)
at ChildProcess.emit (events.js:191:7)
at maybeClose (internal/child_process.js:891:16)
at Socket. (internal/child_process.js:342:11)
at emitOne (events.js:96:13)
at Socket.emit (events.js:188:7)
at Pipe._handle.close [as _onclose] (net.js:497:12)
Command: out/Release/node /Users/mborins/code/node/v6.x/test/parallel/test-vm-sigint.js

@Trott Trott deleted the refactor-test-vm-sigint branch January 13, 2022 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants