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 failures on node v14-v16 #395

Closed
joeyparrish opened this issue Mar 22, 2022 · 3 comments · Fixed by #396
Closed

Test failures on node v14-v16 #395

joeyparrish opened this issue Mar 22, 2022 · 3 comments · Fixed by #396

Comments

@joeyparrish
Copy link
Contributor

I get the following test failure on Linux after fixing the macOS failures in #393 with PR #394

  1) socket cleanup
       should cleanup sockets on ERR_STREAM_PREMATURE_CLOSE (using stream.pipeline):

      Uncaught AssertionError: expected 1 to equal 0
      + expected - actual

      -1
      +0
      
      at Assertion.fail (node_modules/should/cjs/should.js:275:17)
      at Assertion.value (node_modules/should/cjs/should.js:356:19)
      at Timeout._onTimeout (test/socket_cleanup_spec.js:73:33)
      at listOnTimeout (node:internal/timers:557:17)
      at processTimers (node:internal/timers:500:7)
@joeyparrish
Copy link
Contributor Author

Reverting the fix in PR #392 does not fix this test failure. Same for the fix in PR #394.

joeyparrish added a commit to joeyparrish/needle that referenced this issue Mar 22, 2022
This fixes a failing socket cleanup test by destroying the request on
error.  This test did not fail on macOS.

Closes tomas#395
@joeyparrish
Copy link
Contributor Author

I can't tell from the git commit history why this was commented out, but uncommenting this line in the test fixes it:

    stream.pipeline(resp, writable, function(err) {
      err.code.should.eql('ERR_STREAM_PREMATURE_CLOSE')
      // if (err) resp.request.destroy();
    });

@tomas, any insight on that? I'll put up a PR in case that's the right fix. If it's not... I'll need you to figure it out. I'm out of ideas.

@joeyparrish
Copy link
Contributor Author

joeyparrish commented Mar 22, 2022

I should have mentioned, this failure was on node v16. When I run tests on macOS with node v16 or v14, I get this failure, too. So it's not specific to Linux after all.

@joeyparrish joeyparrish changed the title Test failures on Linux Test failures on node v14-v16 Mar 22, 2022
joeyparrish added a commit to joeyparrish/needle that referenced this issue Mar 22, 2022
This fixes a failing socket cleanup test by destroying the request on
error.  This test did not fail on macOS.

Closes tomas#395
joeyparrish added a commit to joeyparrish/needle that referenced this issue Mar 22, 2022
This fixes a failing socket cleanup test by destroying the request on
error.  This test did not fail on node v17.

Closes tomas#395
joeyparrish added a commit to joeyparrish/needle that referenced this issue Mar 22, 2022
This fixes a failing socket cleanup test by destroying the request on
error.  This test did not fail on node v17.

Closes tomas#395
@tomas tomas closed this as completed in #396 Apr 7, 2022
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 a pull request may close this issue.

1 participant