Skip to content

Comments

test: refactor test-vm-sigint#13902

Closed
Trott wants to merge 1 commit intonodejs:masterfrom
Trott:refactor-test-vm-sigint
Closed

test: refactor test-vm-sigint#13902
Trott wants to merge 1 commit intonodejs:masterfrom
Trott:refactor-test-vm-sigint

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

@Trott
test: refactor test-vm-sigint
8aefb47 
* 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

CI: https://ci.nodejs.org/job/node-test-pull-request/8819/

cjihrig
cjihrig approved these changes Jun 25, 2017
View reviewed changes
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
@Trott
test: refactor test-vm-sigint
99dc9d0 
* 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
@Trott @addaleax
test: refactor test-vm-sigint
4630dc5 
* 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
Merged
addaleax pushed a commit that referenced this pull request Jul 11, 2017
@Trott @addaleax
test: refactor test-vm-sigint
e9a3cc4 
* 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
@Trott @addaleax
test: refactor test-vm-sigint
98ec8aa 
* 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

MylesBorins commented Aug 14, 2017

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

Details === 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

Reviewers

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@cjihrig cjihrig cjihrig approved these changes

+1 more reviewer

@gibfahn gibfahn gibfahn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

test Issues and PRs related to the tests. vm Issues and PRs related to the vm subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Trott @MylesBorins @jasnell @lpinca @cjihrig @gibfahn @nodejs-github-bot