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

build,windows: merge test suite groups #13378

Merged
merged 1 commit into from
Jun 10, 2017
Merged

Conversation

refack
Copy link
Contributor

@refack refack commented Jun 1, 2017

  • add async-hooks
  • alphabetize

Refs: #13340
Refs: #13336

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

build,test

@nodejs-github-bot 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 Jun 1, 2017
@refack
Copy link
Contributor Author

refack commented Jun 1, 2017

Ref: #13336 (comment)

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

LGTM if it passes CI 🤞

@refack
Copy link
Contributor Author

refack commented Jun 1, 2017

@refack refack self-assigned this Jun 1, 2017
@refack
Copy link
Contributor Author

refack commented Jun 1, 2017

/cc @nodejs/platform-windows @joaocgreis

Copy link
Member

@AndreasMadsen AndreasMadsen left a comment

Choose a reason for hiding this comment

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

LGTM, crossing my fingers and toes

@refack
Copy link
Contributor Author

refack commented Jun 1, 2017

@addaleax @AndreasMadsen so it build but there's a bunch of failed test in /async-hooks/

Want to divvy them up:

  • test-fseventwrap
  • test-graph.signal
  • test-pipeconnectwrap
  • test-signalwrap
  • test-ttywrap.readstream
  • test-writewrap
=== release test-fseventwrap ===
Path: async-hooks/test-fseventwrap
assert.js:60
  throw new errors.AssertionError({
  ^

AssertionError [ERR_ASSERTION]: Checking invocations at stage "when process exits":
Never called "destroy", but expected 1 invocation(s).
at checkHook (D:\code\node\test\async-hooks\hook-checks.js:45:7)
at Array.forEach (native)
at checkInvocations (D:\code\node\test\async-hooks\hook-checks.js:28:44)
at process.onexit (D:\code\node\test\async-hooks\test-fseventwrap.js:32:3)
at emitOne (events.js:120:20)
at process.emit (events.js:210:7)
Command: D:\code\node\Release\node.exe D:\code\node\test\async-hooks\test-fseventwrap.js
=== release test-graph.signal ===
Path: async-hooks/test-graph.signal
Mismatched onsigusr2 function calls. Expected exactly 2, actual 0.
at Object.exports.mustCall (D:\code\node\test\common\index.js:483:10)
at Object.<anonymous> (D:\code\node\test\async-hooks\test-graph.signal.js:11:30)
at Module._compile (module.js:569:30)
at Object.Module._extensions..js (module.js:580:10)
at Module.load (module.js:503:32)
at tryModuleLoad (module.js:466:12)
at Function.Module._load (module.js:458:3)
at Function.Module.runMain (module.js:605:10)
at startup (bootstrap_node.js:158:16)
Command: D:\code\node\Release\node.exe D:\code\node\test\async-hooks\test-graph.signal.js
=== release test-pipeconnectwrap ===
Path: async-hooks/test-pipeconnectwrap
assert.js:60
throw new errors.AssertionError({
^

AssertionError [ERR_ASSERTION]: Checking invocations at stage "pipe2, process exiting":
Never called "destroy", but expected 1 invocation(s).
at checkHook (D:\code\node\test\async-hooks\hook-checks.js:45:7)
at Array.forEach (native)
at checkInvocations (D:\code\node\test\async-hooks\hook-checks.js:28:44)
at process.onexit (D:\code\node\test\async-hooks\test-pipeconnectwrap.js:98:3)
at emitOne (events.js:120:20)
at process.emit (events.js:210:7)
Command: D:\code\node\Release\node.exe D:\code\node\test\async-hooks\test-pipeconnectwrap.js
=== release test-signalwrap ===
Path: async-hooks/test-signalwrap
assert.js:60
throw new errors.AssertionError({
^

AssertionError [ERR_ASSERTION]: one signal wrap when SIGUSR2 handler is set up
at Object.<anonymous> (D:\code\node\test\async-hooks\test-signalwrap.js:15:8)
at Module._compile (module.js:569:30)
at Object.Module._extensions..js (module.js:580:10)
at Module.load (module.js:503:32)
at tryModuleLoad (module.js:466:12)
at Function.Module._load (module.js:458:3)
at Function.Module.runMain (module.js:605:10)
at startup (bootstrap_node.js:158:16)
at bootstrap_node.js:575:3
Command: D:\code\node\Release\node.exe D:\code\node\test\async-hooks\test-signalwrap.js
=== release test-ttywrap.readstream ===
Path: async-hooks/test-ttywrap.readstream
assert.js:60
throw new errors.AssertionError({
^

AssertionError [ERR_ASSERTION]: Checking invocations at stage "when tty ended ":
Never called "before", but expected 1 invocation(s).
at checkHook (D:\code\node\test\async-hooks\hook-checks.js:45:7)
at Array.forEach (native)
at checkInvocations (D:\code\node\test\async-hooks\hook-checks.js:28:44)
at common.mustCall (D:\code\node\test\async-hooks\test-ttywrap.readstream.js:29:5)
at D:\code\node\test\common\index.js:517:15
at Immediate.ontick (D:\code\node\test\async-hooks\tick.js:7:37)
at runCallback (timers.js:800:20)
at tryOnImmediate (timers.js:762:5)
at processImmediate [as _immediateCallback] (timers.js:733:5)
Command: D:\code\node\Release\node.exe D:\code\node\test\async-hooks\test-ttywrap.readstream.js
=== release test-writewrap ===
Path: async-hooks/test-writewrap
assert.js:60
throw new errors.AssertionError({
^

AssertionError [ERR_ASSERTION]: Checking invocations at stage "when server got secure connection":
Never called "destroy", but expected 1 invocation(s).
at checkHook (D:\code\node\test\async-hooks\hook-checks.js:45:7)
at Array.forEach (native)
at checkInvocations (D:\code\node\test\async-hooks\hook-checks.js:28:44)
at checkValidWriteWrap (D:\code\node\test\async-hooks\test-writewrap.js:56:5)
at Array.forEach (native)
at checkDestroyedWriteWraps (D:\code\node\test\async-hooks\test-writewrap.js:58:6)
at Server.onsecureConnection (D:\code\node\test\async-hooks\test-writewrap.js:65:3)
at Server.<anonymous> (D:\code\node\test\common\index.js:517:15)
at emitOne (events.js:115:13)
at Server.emit (events.js:210:7)
Command: D:\code\node\Release\node.exe D:\code\node\test\async-hooks\test-writewrap.js
[00:16|% 100|+ 39|- 6]: Done

@addaleax
Copy link
Member

addaleax commented Jun 1, 2017

Sigh. Well, thank you. The sigusr2 ones probably should be skipped, and the first one might be fixed by #13369, but I’m not sure about the other ones.

@refack
Copy link
Contributor Author

refack commented Jun 1, 2017

Sigh. Well, thank you. The sigusr2 ones probably should be skipped, and the first one might be fixed by #13369, but I’m not sure about the other ones.

So I'll do them all.

@refack refack added the blocked PRs that are blocked by other issues or PRs. label Jun 1, 2017
@refack
Copy link
Contributor Author

refack commented Jun 1, 2017

Blocked by #13381 & #13336

@refack
Copy link
Contributor Author

refack commented Jun 2, 2017

@refack
Copy link
Contributor Author

refack commented Jun 2, 2017

On the change to vcbuild.bat, commands from Jenkins
Before:

running 'python tools\test.py --mode=release --run=1,4 -p tap --logfile test.tap doctool inspector known_issues message sequential parallel addons addons-napi --flaky-tests=dontcare'

After

running 'python tools\test.py --mode=release --run=1,4 -p tap --logfile test.tap async-hooks inspector known_issues message parallel sequential doctool addons addons-napi --flaky-tests=dontcare'

Copy link
Member

@joaocgreis joaocgreis left a comment

Choose a reason for hiding this comment

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

I would prefer the refactor and adding async-hooks in separate commits, but either way LGTM.

@kunalspathak
Copy link
Member

Thanks for doing this @refack . I noticed this and commented here. At that time I did notice async-hooks test were failing on windows but didn't get time to investigate.

@refack
Copy link
Contributor Author

refack commented Jun 6, 2017

Thanks for doing this @refack . I noticed this and commented here. At that time I did notice async-hooks test were failing on windows but didn't get time to investigate.

It was your comment that triggered this 👍

PR-URL: nodejs#13378
Refs: nodejs#13340
Refs: nodejs#13336
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Andreas Madsen <[email protected]>
Reviewed-By: João Reis <[email protected]>
@refack
Copy link
Contributor Author

refack commented Jun 10, 2017

Landed in df02f39

@refack refack merged commit df02f39 into nodejs:master Jun 10, 2017
@refack
Copy link
Contributor Author

refack commented Jun 10, 2017

Extra sanity test of master of Windows: https://ci.nodejs.org/job/node-test-commit-windows-fanned/9709/

@refack refack removed the blocked PRs that are blocked by other issues or PRs. label Jun 10, 2017
@refack refack deleted the vcbuild-test-merge branch June 10, 2017 21:23
@refack refack removed their assignment Jun 12, 2017
addaleax pushed a commit that referenced this pull request Jun 12, 2017
PR-URL: #13378
Refs: #13340
Refs: #13336
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Andreas Madsen <[email protected]>
Reviewed-By: João Reis <[email protected]>
@addaleax addaleax mentioned this pull request Jun 12, 2017
@MylesBorins
Copy link
Contributor

Should we land this on v6.x?

@refack
Copy link
Contributor Author

refack commented Jul 17, 2017

Yes. Doesn't land cleanly, I'll backport.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants