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

Node test process.stdin readable with a TTY fails on OSX #21112

Closed
mmastrac opened this issue Nov 7, 2023 · 2 comments · Fixed by #21267
Closed

Node test process.stdin readable with a TTY fails on OSX #21112

mmastrac opened this issue Nov 7, 2023 · 2 comments · Fixed by #21267
Assignees
Labels
node compat tests related to tests

Comments

@mmastrac
Copy link
Contributor

mmastrac commented Nov 7, 2023

Repro:

cargo run -- test -A '/Users/matt/Documents/github/deno/deno/cli/tests/unit_node/process_test.ts' --filter TTY

FAILED | 0 passed | 2 failed | 45 filtered out (4ms)

error: Uncaught BadResource: Bad resource ID
    at Stdin.close (ext:deno_io/12_io.js:244:10)
    at ReadStream.<anonymous> (ext:deno_node/_process/streams.mjs:177:37)
    at ReadStream.emit (ext:deno_node/_stream.mjs:1852:9)
    at node:net:878:14
    at ext:core/01_core.js:677:9
    at Object.runMicrotasks (ext:core/01_core.js:857:30)
    at Array.processTicksAndRejections (ext:deno_node/_next_tick.ts:53:10)
    at eventLoopTick (ext:core/01_core.js:180:29)
@mmastrac mmastrac added tests related to tests node compat labels Nov 7, 2023
@mmastrac
Copy link
Contributor Author

mmastrac commented Nov 7, 2023

Looks like it was fallout from #21026 @littledivy

git bisect says:

f62e22a6998bc4d0e2b2fc3df8d6a81aac264e7a is the first bad commit
commit f62e22a6998bc4d0e2b2fc3df8d6a81aac264e7a
Author: Divy Srivastava <dj.srivastava23@gmail.com>
Date:   Tue Oct 31 04:54:43 2023 -0700

    fix(ext/node): tty streams extends net socket (#21026)
    
    Workaround the circular references issue by using a initializer function
    to give tty stream class to `initStdin`.

@jirutka
Copy link
Contributor

jirutka commented Nov 16, 2023

This test fails even on Alpine Linux.

littledivy added a commit that referenced this issue Nov 20, 2023
Fixes #21112

Aligns more towards what Node.js does. Closing stdin more than once is a
nop.
zifeo pushed a commit to metatypedev/deno that referenced this issue Nov 22, 2023
…21267)

Fixes denoland#21112

Aligns more towards what Node.js does. Closing stdin more than once is a
nop.
crowlKats pushed a commit that referenced this issue Nov 24, 2023
Fixes #21112

Aligns more towards what Node.js does. Closing stdin more than once is a
nop.

(cherry picked from commit c97a972)
bartlomieju pushed a commit that referenced this issue Nov 24, 2023
Fixes #21112

Aligns more towards what Node.js does. Closing stdin more than once is a
nop.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
node compat tests related to tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants