test,async_hooks: stabilize tests on Windows by refack · Pull Request #13381 · nodejs/node

refack · 2017-06-01T23:09:52Z

test-callback-error - test: improve async-hooks/test-callback-error #13559
test-fseventwrap - now pass assumed (test: increase coverage of async_hooks #13336)
test-graph.signal
test-pipeconnectwrap - now pass assumed (test: increase coverage of async_hooks #13336)
test-signalwrap
test-ttywrap.readstream
test-writewrap - now pass assumed (test: increase coverage of async_hooks #13336)

new

test-emit-before-after

Details

=== 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. (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. (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. (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

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,async_hooks

jasnell · 2017-06-01T23:59:08Z

I am a bit curious about the name of this file... test-graph.signal.js ?

/cc @thlorenz @addaleax ?

Most of the async hooks can be represented as a graph/tree (see https://dprof.js.org). The test-graph.*.js files check that the graph is correct. The test-graph.signal.js file check that the graph regarding signals is correct.

yeah, I get the purpose of the test, just not why it doesn't follow the naming pattern of other tests (specifically, why test-graph.signal instead of test-graph-signal)

I don't think there is any reason, we can rename.

AndreasMadsen · 2017-06-02T13:01:44Z

@refack Thanks for looking into these. Unfortunately I can't really help much, but I can give some thoughts and guesses, hopefully, it helps.

test\async-hooks\test-fseventwrap.js: looks like a GC sensitive test. tick(2, fn) seams very suspicious.
test\async-hooks\test-pipeconnectwrap.js: again GC issue, tick(5) is suspicious.
test/async-hooks/test-signalwrap.js: also a signal on windows issue, if I'm not mistaken.
async-hooks/test-ttywrap.readstream.js: again GC issue, tick(2, fn) is suspicious.
async-hooks\test-writewrap.js: could be a "race condition" (sort of) between the server onconnection and client onconnect. We have seen this being OS dependent in another test case.

To summarize, I don't think there are any actual bugs the tests are just not robust enough.

refack · 2017-06-09T20:31:53Z

Post #13336 run:

Details

=== release test-emit-before-after ===
Path: async-hooks/test-emit-before-after
assert.js:60
throw new errors.AssertionError({
^
AssertionError [ERR_ASSERTION]: 'Error: before(): asyncId or triggerId is less than zero (asyncId: -1, triggerId: -1)\r' === 'Error: before(): asyncId or triggerId is le

ss than zero (asyncId: -1, triggerId: -1)'

at Object. (D:\code\node\test\async-hooks\test-emit-before-after.js:19: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-emit-before-after.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. (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-signalwrap ===

Path: async-hooks/test-signalwrap

assert.js:60

throw new errors.AssertionError({

^
AssertionError [ERR_ASSERTION]: 0 === 1

at Object. (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 ()

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

refack · 2017-06-10T14:16:50Z

"Force" CI on Windows: https://ci.nodejs.org/job/node-stress-single-test/1291/nodes=win10/
Sanity on linuxone: https://ci.nodejs.org/job/node-test-commit-linuxone/6535/

refack · 2017-06-10T14:26:24Z

New linuxone: https://ci.nodejs.org/job/node-test-commit-linuxone/6536/

refack · 2017-06-10T14:48:59Z

Again: https://ci.nodejs.org/job/node-stress-single-test/1292/nodes=win10/

refack · 2017-06-10T14:51:12Z

@DavidCai1993 @nodejs/async_hooks PTAL at b0f0119

refack · 2017-06-10T17:32:34Z

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

refack · 2017-06-10T19:33:42Z

reported flake #13603
rerun arm: https://ci.nodejs.org/job/node-test-commit-arm/10259/

AndreasMadsen · 2017-07-01T20:50:55Z

@refack Could you rebase this? Then I will take a closer look.

refack · 2017-07-01T22:19:21Z

https://ci.nodejs.org/job/node-stress-single-test/1313/nodes=win10/

AndreasMadsen

LGTM, assuming the CI is green on all platforms.

Thanks for taking care of this, it is was a bit embarrassing that we didn't have tests enabled for windows.

refack · 2017-07-02T00:29:33Z

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

refack · 2017-07-02T02:00:57Z

Arm fail is known: #14015

PR-URL: nodejs#13381 Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

refack · 2017-07-02T02:27:00Z

Landed in 372b85d, 7022260

PR-URL: nodejs#13381 Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

PR-URL: #13381 Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

nodejs-github-bot added async_hooks Issues and PRs related to the async hooks subsystem. test Issues and PRs related to the tests. labels Jun 1, 2017

refack added the wip Issues and PRs that are still a work in progress. label Jun 1, 2017

refack mentioned this pull request Jun 1, 2017
build,windows: merge test suite groups #13378
Merged
4 tasks

refack self-assigned this Jun 1, 2017

refack added the windows Issues and PRs related to the Windows platform. label Jun 1, 2017

jasnell reviewed Jun 1, 2017
View reviewed changes

refack force-pushed the stabalize-async-tests-windows branch from 52583d0 to b0f0119Compare June 10, 2017 14:13

refack removed the wip Issues and PRs that are still a work in progress. label Jun 10, 2017

refack changed the title ~~[WIP] test,async_hooks: stabilize tests on Windows~~test,async_hooks: stabilize tests on WindowsJun 10, 2017

refack force-pushed the stabalize-async-tests-windows branch from 5d009c8 to f83c32dCompare June 10, 2017 17:27

refack force-pushed the stabalize-async-tests-windows branch from f83c32d to 8feb68cCompare July 1, 2017 22:17

AndreasMadsen approved these changes Jul 1, 2017
View reviewed changes

refack added 2 commits July 1, 2017 22:02

test,async_hooks: stabilize tests on Windows
372b85d
PR-URL: nodejs#13381 Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

build: add async-hooks testing to vcbuild.bat
7022260
PR-URL: nodejs#13381 Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

refack force-pushed the stabalize-async-tests-windows branch from 8feb68c to 7022260Compare July 2, 2017 02:05

refack merged commit 7022260 into nodejs:masterJul 2, 2017

refack deleted the stabalize-async-tests-windows branch July 2, 2017 02:33

addaleax pushed a commit to addaleax/node that referenced this pull request Jul 3, 2017
test,async_hooks: stabilize tests on Windows
9a74b8d
PR-URL: nodejs#13381 Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

addaleax pushed a commit to addaleax/node that referenced this pull request Jul 3, 2017
build: add async-hooks testing to vcbuild.bat
55eceef
PR-URL: nodejs#13381 Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

addaleax mentioned this pull request Jul 3, 2017
v8.2.0 proposal #13744
Merged

addaleax pushed a commit that referenced this pull request Jul 11, 2017
test,async_hooks: stabilize tests on Windows
53a36de
PR-URL: #13381 Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

addaleax pushed a commit that referenced this pull request Jul 11, 2017
build: add async-hooks testing to vcbuild.bat
b11944b
PR-URL: #13381 Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

AndreasMadsen added the dont-land-on-v6.x label Jul 18, 2017

addaleax pushed a commit that referenced this pull request Jul 18, 2017
test,async_hooks: stabilize tests on Windows
3d9bc01
PR-URL: #13381 Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

addaleax pushed a commit that referenced this pull request Jul 18, 2017
build: add async-hooks testing to vcbuild.bat
651af59
PR-URL: #13381 Reviewed-By: Andreas Madsen <amwebdk@gmail.com>

refack removed their assignment Oct 20, 2018

Uh oh!

Uh oh!

Conversation

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

Uh oh!

new

Checklist

Affected core subsystem(s)

Uh oh!

jasnellJun 1, 2017

Choose a reason for hiding this comment

Uh oh!

refackJun 2, 2017

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasnellJun 2, 2017

Choose a reason for hiding this comment

Uh oh!

AndreasMadsenJun 2, 2017

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

refack commented Jun 9, 2017

Uh oh!

refack commented Jun 10, 2017

Uh oh!

refack commented Jun 10, 2017

Uh oh!

refack commented Jun 10, 2017

Uh oh!

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

Uh oh!

Uh oh!

refack commented Jun 10, 2017

Uh oh!

refack commented Jun 10, 2017

Uh oh!

AndreasMadsen commented Jul 1, 2017

Uh oh!

refack commented Jul 1, 2017

Uh oh!

AndreasMadsen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

refack commented Jul 2, 2017

Uh oh!

refack commented Jul 2, 2017

Uh oh!

refack commented Jul 2, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

refack commented Jun 1, 2017•
edited
Loading

AndreasMadsenJun 2, 2017•
edited
Loading

AndreasMadsen commented Jun 2, 2017•
edited
Loading

refack commented Jun 10, 2017•
edited
Loading

AndreasMadsen left a comment •
edited
Loading