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: make test-v8-serdes work without stdin #16382

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Oct 22, 2017

If stdin was closed or referred to a file, this didn't work,
because it was accessed via file descriptor.

Instead, use another generic native object.

cherry-picked from ayojs/ayo#63

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 22, 2017
@danbev
Copy link
Contributor Author

danbev commented Oct 22, 2017

@addaleax
Copy link
Member

Just to mention it: ayojs/ayo@06a1e54 also fixes this but doesn’t require skipping anything … I think that should be fine to use here

@danbev
Copy link
Contributor Author

danbev commented Oct 22, 2017

Just to mention it: ayojs/ayo@06a1e54 also fixes this but doesn’t require skipping anything … I think that should be fine to use here

Very nice! Any advice on the the correct procedure for including that so the proper credit and such is given? Thanks

@addaleax
Copy link
Member

@danbev Since it’s my commit – do whatever you want with it. :) Just cherry-picking it should be fine, but whatever gets you where the code needs to be works for me 😄

If `stdin` was closed or referred to a file, this didn't work,
because it was accessed via file descriptor.

Instead, use another generic native object.

cherry-picked from ayojs/ayo#63
@mscdex mscdex added the v8 engine Issues and PRs related to the V8 dependency. label Oct 22, 2017
@danbev
Copy link
Contributor Author

danbev commented Oct 23, 2017

@danbev danbev changed the title test: check for _handle in test-v8-serdes.js test: make test-v8-serdes work without stdin Oct 23, 2017
@danbev
Copy link
Contributor Author

danbev commented Oct 25, 2017

Landed in 70670b0

@danbev danbev closed this Oct 25, 2017
@danbev danbev deleted the test-v8-serdes_handle branch October 25, 2017 05:23
apapirovski referenced this pull request Oct 25, 2017
If `stdin` was closed or referred to a file, this didn't work,
because it was accessed via file descriptor.

Instead, use another generic native object.

cherry-picked from ayojs/ayo#63

Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants