test: older shared zlib doesnt throw on create #15478
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 tasks
cjihrig
reviewed
Sep 19, 2017
| @@ -5,6 +5,10 @@ require('../common'); | |||
| const assert = require('assert'); | |||
| const zlib = require('zlib'); | |||
|
|
|||
| if (process.config.variables.node_shared_zlib && /^1\.2\.[0-8]$/.test(process.versions.zlib)) { | |||
| require('../common').skip("older versions of shared zlib don't throw on create"); | |||
There was a problem hiding this comment.
../common is already required above. You just need to assign it to a variable.
lpinca
reviewed
Sep 20, 2017
|
|
||
| const assert = require('assert'); | ||
| const zlib = require('zlib'); | ||
|
|
||
| if (process.config.variables.node_shared_zlib && /^1\.2\.[0-8]$/.test(process.versions.zlib)) { |
There was a problem hiding this comment.
Nit: long line. I think linter is not happy with it.
There was a problem hiding this comment.
Hmm... from digging through the Makefile this appears to be the lint command:
$(NODE) tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules --ext=.js,.md benchmark doc lib test tools
I ran that and it didn't show any lint in that file. I'll break the long line anyways, however.
When using compiling using `./configure --shared-zlib` older versions of the shared zlib might not through as expected by the test-zlib-failed-init test. Refs: #13697
lpinca
approved these changes
Sep 20, 2017
jasnell
approved these changes
Sep 20, 2017
MylesBorins
pushed a commit
that referenced
this pull request
Sep 22, 2017
We are currenly builing Node with --shared-zlib which happens to be version 1.2.8. The test for zlib.createDeflateRaw is expected to fail but does not when using version 1.2.8. As far as I can tell the fix referred to in the comments was introduced in version 1.2.9: - Reject a window size of 256 bytes if not using the zlib wrapper This commit suggests adding a check for the version and skipping this assert if the version is less than 1.2.9. Refs: http://zlib.net/ChangeLog.txt Backport-PR-URL: #15478 PR-URL: #13697 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
MylesBorins
pushed a commit
that referenced
this pull request
Sep 26, 2017
We are currenly builing Node with --shared-zlib which happens to be version 1.2.8. The test for zlib.createDeflateRaw is expected to fail but does not when using version 1.2.8. As far as I can tell the fix referred to in the comments was introduced in version 1.2.9: - Reject a window size of 256 bytes if not using the zlib wrapper This commit suggests adding a check for the version and skipping this assert if the version is less than 1.2.9. Refs: http://zlib.net/ChangeLog.txt Backport-PR-URL: #15478 PR-URL: #13697 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesmake lintdoesn't show any new lintAffected core subsystem(s)
test, zlib