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: Add zlib close-after-error regression test #6270

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

zlib, test

Description of change

Add a regression test based on the report in #6034. The bug itself has been fixed in #5982.

@addaleax addaleax added zlib Issues and PRs related to the zlib subsystem. test Issues and PRs related to the tests. labels Apr 19, 2016
Add a regression test based on the report in
nodejs#6034.
@addaleax
Copy link
Member Author

@cjihrig
Copy link
Contributor

cjihrig commented Apr 19, 2016

LGTM. CI is green.

@santigimeno
Copy link
Member

LGTM

1 similar comment
@jasnell
Copy link
Member

jasnell commented Apr 19, 2016

LGTM

@jasnell
Copy link
Member

jasnell commented Apr 20, 2016

Wanna go ahead and land this one @addaleax ? :-)

@addaleax
Copy link
Member Author

@jasnell Sure, I just didn’t really see any reason not to wait the recommended time so that everyone can point out the gazillion style nits I usually produce 😉

@addaleax
Copy link
Member Author

Landed in f8e507e.

@addaleax addaleax closed this Apr 20, 2016
@addaleax addaleax deleted the zlib-6034-regression-test branch April 20, 2016 15:47
addaleax added a commit that referenced this pull request Apr 20, 2016
Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@jasnell
Copy link
Member

jasnell commented Apr 20, 2016

;-) no style nits in this one! For really small commits like this (tests, docs, lints, etc), if there's obvious consensus then it's usually ok to land a bit sooner than the 48 hours.

MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins
Copy link
Contributor

@jasnell @addaleax lts?

@addaleax
Copy link
Member Author

@thealphanerd Should be included together with #5982, yes.

@MylesBorins
Copy link
Contributor

thanks for the swift response, and for all the awesome work you've been putting in lately!!!

MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request Apr 25, 2016
Add a regression test based on the report in
nodejs#6034.

PR-URL: nodejs#6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
jasnell pushed a commit that referenced this pull request Apr 26, 2016
Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 17, 2016
Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 18, 2016
Add a regression test based on the report in
#6034.

PR-URL: #6270
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 18, 2016
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. zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants