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

export * from stream when process.env.READABLE_STREAM === 'disable' #399

Merged
merged 1 commit into from
Feb 27, 2019

Conversation

trxcllnt
Copy link
Contributor

@trxcllnt trxcllnt commented Feb 15, 2019

Fixes #398.

@mcollina I'm not sure how to test this other than creating a new script that does READABLE_STREAM=disable npm test. That approach seems wrong (unless this library is supposed to stay ahead of node's API), because I get the following two failures:

TypeError [ERR_INVALID_CALLBACK]: Callback must be a function
    at popCallback (internal/streams/pipeline.js:61:11)
    at pipeline (internal/streams/pipeline.js:66:20)
    at /home/ptaylor/dev/readable-stream/test/parallel/test-stream-pipeline.js:77:5
    at _tryBlock (/home/ptaylor/dev/readable-stream/node_modules/assert/assert.js:425:5)
    at _throws (/home/ptaylor/dev/readable-stream/node_modules/assert/assert.js:444:12)
    at Function.assert.throws (/home/ptaylor/dev/readable-stream/node_modules/assert/assert.js:474:3)
    at Object.<anonymous> (/home/ptaylor/dev/readable-stream/test/parallel/test-stream-pipeline.js:76:10)
    at Module._compile (internal/modules/cjs/loader.js:721:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:732:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
test/parallel/test-stream-pipe-unpipe-streams.js ...... 1/1
test/parallel/test-stream-pipe-without-listenerCount.js .. 1/1
test/parallel/test-stream-pipeline.js ................. 0/1
  not ok test/parallel/test-stream-pipeline.js
    timeout: 30000
    file: test/parallel/test-stream-pipeline.js
    command: /home/ptaylor/.nvm/versions/node/v11.6.0/bin/node
    args:
      - test/parallel/test-stream-pipeline.js
    stdio:
      - 0
      - pipe
      - 2
    cwd: /home/ptaylor/dev/readable-stream
    exitCode: 1

test/parallel/test-stream-readable-async-iterators.js .Mismatched noop function calls. Expected exactly 1, actual 0.
    at Object.mustCall (/home/ptaylor/dev/readable-stream/test/common/index.js:343:10)
    at Object.<anonymous> (/home/ptaylor/dev/readable-stream/test/parallel/test-stream-readable-async-iterators.js:647:21)
    at Module._compile (internal/modules/cjs/loader.js:721:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:732:10)
    at Module.load (internal/modules/cjs/loader.js:620:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:560:12)
    at Function.Module._load (internal/modules/cjs/loader.js:552:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:774:12)
    at executeUserCode (internal/bootstrap/node.js:342:17)
test/parallel/test-stream-readable-async-iterators.js .. 1/2
  not ok test/parallel/test-stream-readable-async-iterators.js
    timeout: 30000
    file: test/parallel/test-stream-readable-async-iterators.js
    command: /home/ptaylor/.nvm/versions/node/v11.6.0/bin/node
    args:
      - test/parallel/test-stream-readable-async-iterators.js
    stdio:
      - 0
      - pipe
      - 2
    cwd: /home/ptaylor/dev/readable-stream
    exitCode: 1

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM this is ok

@ulisesbocchio
Copy link

Having this issue also on node 16. ETA for the release?

@trxcllnt
Copy link
Contributor Author

trxcllnt commented Nov 4, 2021

@ulisesbocchio are you having an issue introduced by this PR (the one I fixed in #457)?

@ulisesbocchio
Copy link

@trxcllnt yes. I'm having the issue described in #457 which seems to be present in the current 3.6.0 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants