test: add an zlib binding addon test

[addaleax]

Checklist

• make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

Affected core subsystem(s)

test/zlib

Description of change

Add a test addon that makes use of the zlib implementation bundled with node, checking that a compression/decompression round-trip works.

This is largely based on the already-existing OpenSSL addon.

Fixes: #7535

Initial CI: https://ci.nodejs.org/job/node-test-commit/4488/

[jasnell]

That was fast!

[jasnell]

LGTM if CI is green.

[addaleax]

CI is actually green, with one unrelated Windows test failure.

[ghost]

Smart test. Thanks for fixing this.

[bnoordhuis]

LGTM. Unrelated failure on the 2,vs2015,win2012r2 buildbot.

[cjihrig]

LGTM

[jasnell]

New C since there were a couple (apparently unrelated) failures in the last run: https://ci.nodejs.org/job/node-test-pull-request/3647/

[jasnell]

more unrelated build bot failures in CI :-( ... few tests still pending tho.

[addaleax]

This time, apparently the binding used in the test here failed to be created on CI, although that went perfectly fine in the previous CI, and I can’t see anything hinting at the cause of that… /cc @nodejs/build?

[addaleax]

Trying CI once more: https://ci.nodejs.org/job/node-test-commit/4594/

[jasnell]

CI is green. Landing

[jasnell]

Landed in 4c9b0bd

[MylesBorins]

@addaleax it looks like this is causing failures on windows on v4.x

https://ci.nodejs.org/job/node-test-binary-windows/RUN_SUBSET=1,VS_VERSION=vs2013,label=win2008r2/4279/tapTestReport/test.tap-259/

not ok 259 addons/zlib-binding/test
# module.js:327
# throw err;
# ^
# 
# Error: Cannot find module './build/Release/binding'
# at Function.Module._resolveFilename (module.js:325:15)
# at Function.Module._load (module.js:276:25)
# at Module.require (module.js:353:17)
# at require (internal/module.js:12:17)
# at Object.<anonymous> (c:\workspace\node-test-binary-windows\RUN_SUBSET\1\VS_VERSION\vs2013\label\win2008r2\test\addons\zlib-binding\test.js:6:17)
# at Module._compile (module.js:409:26)
# at Object.Module._extensions..js (module.js:416:10)
# at Module.load (module.js:343:32)
# at Function.Module._load (module.js:300:12)
# at Function.Module.runMain (module.js:441:10)

I'm going to avoid backing it out for right now as I'm landing PR's and don't want to cause more rebases... but will be looking at backing it out after a few things land. If you could look into it and find a solution before then that would be rad!

[ghost]

It is supposed to fail under Windows (4.x) since the fix of zlib for Windows is only applied to the 6.x of Node.js.

[MylesBorins]

would you be willing to submit a pr to v4.x that sets it as flaky? or would
we be better not backporting

On Mon, Oct 10, 2016, 2:16 PM Alex Hultman notifications@github.com wrote:

It is supposed to fail under Windows since the fix of zlib for Windows is
only applied to the 6.x of Node.js.

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
#8039 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAecV2nnmzpG_kuz6X0PM4epCtP7aVp1ks5qyoDxgaJpZM4JgmYK
.

[ghost]

I think the idea was to not backport any of those fixes (zlib, openssl) to 4.x since some addons have kind of ignored to really fix the real issue and instead do link to their own versions of openssl and zlib. So when 6.x added its own exposure of zlib and openssl some addons broke because of colliding symbols or something like that.

[addaleax]

Yeah, if this is backported to v4, the test should probably be marked as flaky.

[MylesBorins]

Im going to go ahead and back this out of v4.x. Please feel free to submit a backport with the flaky tst set appropriately, but I don't think we need it