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

Failing test on v10.x-staging #23633

Closed
richardlau opened this issue Oct 13, 2018 · 3 comments
Closed

Failing test on v10.x-staging #23633

richardlau opened this issue Oct 13, 2018 · 3 comments
Labels
test Issues and PRs related to the tests.

Comments

@richardlau
Copy link
Member

Refs: #23630 (comment)

It looks like there's a failing test on v10.x-staging due to a backport.

https://ci.nodejs.org/job/node-test-commit-linuxone/nodes=rhel72-s390x/6096/console

18:49:40 not ok 1661 parallel/test-stream-wrap-drain
18:49:40   ---
18:49:40   duration_ms: 0.87
18:49:40   severity: fail
18:49:40   exitcode: 1
18:49:40   stack: |-
18:49:40     /data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-stream-wrap-drain.js:8
18:49:40     const { ShutdownWrap } = internalBinding('stream_wrap');
18:49:40                              ^
18:49:40     
18:49:40     TypeError: internalBinding is not a function
18:49:40         at Object.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-stream-wrap-drain.js:8:26)
18:49:40         at Module._compile (internal/modules/cjs/loader.js:688:30)
18:49:40         at Object.Module._extensions..js (internal/modules/cjs/loader.js:699:10)
18:49:40         at Module.load (internal/modules/cjs/loader.js:598:32)
18:49:40         at tryModuleLoad (internal/modules/cjs/loader.js:537:12)
18:49:40         at Function.Module._load (internal/modules/cjs/loader.js:529:3)
18:49:40         at Function.Module.runMain (internal/modules/cjs/loader.js:741:12)
18:49:40         at startup (internal/bootstrap/node.js:285:19)
18:49:40         at bootstrapNodeJSCore (internal/bootstrap/node.js:739:3)
18:49:40   ...

The failing test was added to v10.x-staging in 7af84fc.

It looks like lib/internal/test/binding.js differs between v10.x-staging and master:
v10.x-staging:
https://github.com/nodejs/node/blob/2d4d546bcb2a896b9771753f90e1dacb2e706d0b/lib/internal/test/binding.js#L1-L14

master:

'use strict';
process.emitWarning(
'These APIs are exposed only for testing and are not ' +
'tracked by any versioning system or deprecation process.',
'internal/test/binding');
module.exports = { internalBinding };

and it looks like this change is 9f5cc1f#diff-01901c3bbffdf0cd41374f4f2a02bdb8 which is from semver-major #22029.

cc @nodejs/releasers @nodejs/backporters

@richardlau richardlau added the test Issues and PRs related to the tests. label Oct 13, 2018
@richardlau
Copy link
Member Author

I think the fix is to change
https://github.com/nodejs/node/blob/7af84fccb95aa9b1ad30d9327e20a303e8f3c58f/test/parallel/test-stream-wrap-drain.js#L7-L8

to

const { ShutdownWrap } = process.binding('stream_wrap'); 

and remove
https://github.com/nodejs/node/blob/7af84fccb95aa9b1ad30d9327e20a303e8f3c58f/test/parallel/test-stream-wrap-drain.js#L1

but I'm unsure as I'm not currently on a build environment to test locally.

What do we want to do -- PR ontop of the current v10.x-staging or back out 7af84fc and manually backport #23294?

@Trott
Copy link
Member

Trott commented Oct 13, 2018

@targos is reading over my shoulder and says: The solution is to back out the commit, cherry-pick the commit, and make the modification before pushing to the v10.x-staging.

@Trott
Copy link
Member

Trott commented Oct 13, 2018

Done. (@targos talked me through it.) Thanks!

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.
Projects
None yet
Development

No branches or pull requests

2 participants