test: skip some binding tests on IBMi PASE by dmabupt · Pull Request #31967 · nodejs/node

dmabupt · 2020-02-26T09:34:12Z

Ldflags "-lcrypto", "-lssl" and "-lz" are needed on
IBMi PASE to build the test suite

richardlau · 2020-02-26T13:02:11Z

Ldflags "-lcrypto", "-lssl" and "-lz" are needed on
IBMi PASE to build the test suite

Those tests are testing addons can build against the openssl and zlib bindings statically linked into the Node.js binary so linking to external libraries negates the purpose of those tests. Or does Node.js on IBMi PASE always build against the system openssl/zlib?

dmabupt · 2020-02-26T15:24:27Z

Ldflags "-lcrypto", "-lssl" and "-lz" are needed on
IBMi PASE to build the test suite
Those tests are testing addons can build against the openssl and zlib bindings statically linked into the Node.js binary so linking to external libraries negates the purpose of those tests. Or does Node.js on IBMi PASE always build against the system openssl/zlib?

Yes, Node.js on IBMi PASE always links to the system libraries (no static linking). Or maybe I should skip these cases?

richardlau · 2020-02-26T15:40:24Z

Anyone know what the Linux packagers that link against shared libraries do?

addaleax

Just making sure that this does not land as-is

dmabupt · 2020-02-27T03:08:16Z

If only skip them on IBMi PASE, I can just simply edit addon.status like ARM.
If it affect all shared openssl/zlib builds, maybe we should consider to add the conditions of node_shared_openssl & node_shared_zlib like this?

Hello @addaleax ,
I tried to skip the tests using [$system==ibmi]. But it seems has no effect. So I changed the commit using the 2nd solution. Is it acceptable?
Or would you tell me how to skip these two tests on IBMi?

dmabupt · 2020-02-27T14:52:38Z

Anyone know what the Linux packagers that link against shared libraries do?

I tried to compile node.js with shared openssl&zlib on CentOS and the addon tests showed no error. I guess that even in this 'shared' configuration, the Linux node.js build can also bind to the static libraries as well. But it does not work on IBMi PASE.

mhdawson · 2020-02-27T21:49:34Z

@dmabupt how many/what tests don't pass?

dmabupt · 2020-02-28T00:40:26Z

@dmabupt how many/what tests don't pass?

Hello @mhdawson , There are 18 failed tests --
[21:48|% 100|+ 2907|- 18]: Done
Below test results are from the make test-only test of node-v14.0.0-nightly2020022720a51b9dff.
Some of the 18 failed tests are waiting for the fixes from libuv

parallel/test-dummy-stdio Error: ERR_TTY_INIT_FAILED
parallel/test-net-keepalive Error: read ECONNRESET
parallel/test-net-persistent-keepalive Error: read ECONNRESET
parallel/test-net-socket-connect-without-cb Error: read ECONNRESET
parallel/test-net-socket-close-after-end Error: read ECONNRESET
parallel/test-net-socket-connecting Error: read ECONNRESET
parallel/test-net-socket-local-address Error: read ECONNRESET
parallel/test-net-socket-ready-without-cb Error: read ECONNRESET
parallel/test-net-write-after-end-nt Error: read ECONNRESET
parallel/test-os Error: os.freemem() <= 0
parallel/test-tls-env-extra-ca Error: read ECONNRESET
parallel/test-tls-peer-certificate-multi-keys Error: read ECONNRESET
parallel/test-tls-peer-certificate-encoding Error: read ECONNRESET
parallel/test-ttywrap-invalid-fd Error: ERR_TTY_INIT_FAILED
wasi/test-wasi AssertionError [ERR_ASSERTION]
addons/openssl-binding/test
addons/zlib-binding/test
pseudo-tty/test-tty-isatty 55555 reported to be a tty, but it is not

mhdawson · 2020-03-02T22:22:39Z

So these are the only 2 that are related to the addons:

addons/openssl-binding/test
addons/zlib-binding/test

Sounds like you did tests on linux, though and excluding them in the way you suggest does not make sense since the tests still run ok there.

We should see if we can get the excludes working properly.

mhdawson · 2020-03-02T22:40:51Z

Looking at the test code seems that the platform should be detected as OS400 by the test harness and maps to 'ibmi' so using [$system==ibmi] should work.

In what file did you add that along with the tests ? I think you would have to add to .../test/addons/addon.status

and something like this

[$system==ibmi]
addons/openssl-binding/test: SKIP
addons/zlib-binding/test: SKIP

dmabupt · 2020-03-06T02:54:07Z

Looking at the test code seems that the platform should be detected as OS400 by the test harness and maps to 'ibmi' so using [$system==ibmi] should work.
In what file did you add that along with the tests ? I think you would have to add to .../test/addons/addon.status
and something like this
[$system==ibmi]
addons/openssl-binding/test: SKIP
addons/zlib-binding/test: SKIP

Yes, but I missed the addons path and wrote openssl-binding/test: SKIP. That may be the problem.

mhdawson · 2020-03-06T16:26:53Z

@dmabupt ok let us know if fixing up the paths works.

dmabupt · 2020-03-06T16:48:55Z

@dmabupt ok let us know if fixing up the paths works.

@mhdawson , Seems it still can not skip the cases --

Error: Command failed: /home/XUMENG/rpmbuild/BUILD/node-v14.0.0-nightly202003026bcea0a383/out/Release/node /home/XUMENG/rpmbuild/BUILD/node-v14.0.0-nightly202003026bcea0a383/deps/npm/node_modules/node-gyp/bin/node-gyp.js rebuild --directory=/home/XUMENG/rpmbuild/BUILD/node-v14.0.0-nightly202003026bcea0a383/test/addons/openssl-binding
ld: 0711-224 WARNING: Duplicate symbol: std::_Sp_counted_ptr<decltype(nullptr), (__gnu_cxx::_Lock_policy)2>::_M_dispose()
ld: 0711-224 WARNING: Duplicate symbol: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_destroy()
ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
ld: 0711-317 ERROR: Undefined symbol: .TLSv1_2_server_method
ld: 0711-317 ERROR: Undefined symbol: .RAND_poll
ld: 0711-317 ERROR: Undefined symbol: .RAND_bytes
collect2: error: ld returned 8 exit status
gmake[2]: *** [binding.target.mk:145: Release/obj.target/binding.node] Error 1

The patch is --

--- a/test/addons/addon.status
+++ b/test/addons/addon.status
@@ -9,3 +9,7 @@ prefix addons
[$arch==arm]
# https://github.com/nodejs/node/issues/30786
openssl-binding/test: PASS,FLAKY
+
+[$system==ibmi]
+addons/openssl-binding/test: SKIP
+addons/zlib-binding/test: SKIP

mhdawson · 2020-03-06T17:21:58Z

Is it failing in the build step as opposed to the test step?

dmabupt · 2020-03-07T04:36:53Z

Is it failing in the build step as opposed to the test step?

@mhdawson , It is failing in the make test-only step. More precisely, in the make build-addons step.

To skip the build step, we have to edit this file (not that gracefully) --

--- a/tools/build-addons.js
+++ b/tools/build-addons.js
@@ -28,7 +28,8 @@ async function runner(directoryQueue) {
return next();
throw err;
}
-
+ if(dir.indexOf('openssl-binding') >= 0 || dir.indexOf('zlib-binding') >= 0)
+ return next();
console.log(`Building addon in ${dir}`);
const { stdout, stderr } =
await execFile(process.execPath, [nodeGyp, 'rebuild', `--directory=${dir}`],

mhdawson · 2020-03-09T14:41:45Z

@dmabupt can you add guards based on IBMi based on the platform in the test files, for example in node/test/addons/zlib-binding which avoids the compile failures, that along with the excludes might work and be a better answer.

dmabupt · 2020-03-09T14:51:05Z

@dmabupt can you add guards based on IBMi based on the platform in the test files, for example in node/test/addons/zlib-binding which avoids the compile failures, that along with the excludes might work and be a better answer.

@mhdawson Do you mean updating node/test/addons/zlib-binding/binding.gyp to skip IBMi systems?

mhdawson · 2020-03-09T14:55:41Z

Was not thinking of that, more changing the code in the cc file so that it still compiles (but obviously not work), if we can make the gyp file not compile it for IBMi that might be even better.

dmabupt · 2020-03-09T15:16:24Z

Was not thinking of that, more changing the code in the cc file so that it still compiles (but obviously not work), if we can make the gyp file not compile it for IBMi that might be even better.

Updating the c code to make it build a dummy executable file on IBMi, then both the build step and test step can be passed (skipped) ?

dmabupt · 2020-03-09T15:49:50Z

Was not thinking of that, more changing the code in the cc file so that it still compiles (but obviously not work), if we can make the gyp file not compile it for IBMi that might be even better.
Updating the c code to make it build a dummy executable file on IBMi, then both the build step and test step can be passed (skipped) ?

Seems it is not that easy. The zlib js test code deflates the data compressed by the compiled C code, and then compares it with the origin buffer -- https://github.com/nodejs/node/blob/master/test/addons/zlib-binding/test.js#L12

That means we have to change the js code as well if we skip the logic in the C code.

mhdawson · 2020-03-09T16:10:35Z

But if the test is skipped then it should not matter what is in the js code right?

dmabupt · 2020-03-09T23:53:33Z

But if the test is skipped then it should not matter what is in the js code right?

@mhdawson

The openssl binding test can do this because its C code just simply return true/false.
But the zlib binding test's C code returns a zlib compressed object which should be correctly extracted by its js code.
I mean, the zlib binding test contains two parts. The C code is just the compression stage. The js code is part of the test as well -- the decompression stage.

mhdawson · 2020-03-10T20:38:45Z

@dmabupt I don't understand your point. If the test is skipped in the status file the js code will not run, correct?

dmabupt · 2020-03-11T14:06:04Z

@dmabupt I don't understand your point. If the test is skipped in the status file the js code will not run, correct?

@mhdawson
The js code will not run, but the c code will still be compiled with error. The status file does not control the compiling step. And because of the compiling failure, the test process ends, any following js code even has no chance to run.

dmabupt · 2020-03-11T15:03:53Z

Now I think if we want to minimize the code change, my initial workaround (adding -lssl, -lz to the gyp file to make the two tests pass on IBMi PASE) is still a choice.

mhdawson · 2020-03-12T18:35:02Z

@dmabupt your original change seemed to affect all platforms. It might be ok if it only affect IBM i

dmabupt · 2020-03-16T12:41:45Z

@dmabupt your original change seemed to affect all platforms. It might be ok if it only affect IBM i

My following force push actions have overwritten the first commit. In that commit, the ldflags are only added on platform ibmi.

dmabupt · 2020-03-17T09:44:01Z

Hello @mhdawson , I have reverted the commits to the initial one -> 4df86c9 . This change only affect the IBMi PASE platform and it makes the two failed tests skipped (both compile & test steps).
Hello @addaleax , Would you review the commit 4df86c9 please? I think it minimizes the impact if we just want to skip the two tests.

IBMi PASE Node.js always links to shared openssl and zlib libraries. So skip the static binding tests.

mhdawson

LGTM

nodejs-github-bot · 2020-03-29T21:52:24Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30217/

nodejs-github-bot · 2020-03-30T07:48:22Z

CI: https://ci.nodejs.org/job/node-test-pull-request/30245/

addaleax · 2020-03-30T08:19:58Z

Landed in b416993

IBMi PASE Node.js always links to shared openssl and zlib libraries. So skip the static binding tests. PR-URL: #31967 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

addaleax · 2020-04-13T22:09:52Z

@dmabupt This is failing to build on Windows machines that do not have uname available, fyi.

dmabupt · 2020-04-13T22:54:49Z

@dmabupt This is failing to build on Windows machines that do not have uname available, fyi.

Ah...yes, I should detect uname only in the aix condition. I opened another PR for the fix on Windows.

IBMi PASE Node.js always links to shared openssl and zlib libraries. So skip the static binding tests. PR-URL: #31967 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

IBM i PASE Node.js always links to shared openssl libraries. Skip recently added OpenSSL addons tests as we do for other OpenSSL addons tests on IBM i. Refs: nodejs#31967 Refs: nodejs#44148

IBM i PASE Node.js always links to shared openssl libraries. Skip recently added OpenSSL addons tests as we do for other OpenSSL addons tests on IBM i. Refs: #31967 Refs: #44148 PR-URL: #44810 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

nodejs-github-bot added addons Issues and PRs related to native addons. test Issues and PRs related to the tests. labels Feb 26, 2020

addaleax requested changes Feb 26, 2020
View reviewed changes

dmabupt changed the title ~~test: add missing ldflags on IBMi PASE~~test: skip some binding tests on IBMi PASE Feb 27, 2020

test: skip some binding tests on IBMi PASE
c3527e4
IBMi PASE Node.js always links to shared openssl and zlib libraries. So skip the static binding tests.

mhdawson approved these changes Mar 18, 2020
View reviewed changes

dmabupt requested a review from addaleax March 18, 2020 23:18

addaleax approved these changes Mar 29, 2020
View reviewed changes

addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 29, 2020

addaleax closed this Mar 30, 2020

targos mentioned this pull request Apr 13, 2020
v13.13.0 release proposal #32813
Merged

dmabupt mentioned this pull request Apr 14, 2020
test: only detect uname on supported os #32833
Closed

targos mentioned this pull request Apr 22, 2020
v12.16.3 release proposal #33009
Merged

richardlau mentioned this pull request Sep 28, 2022
test: skip some binding tests on IBMi PASE #44810
Merged

richardlau mentioned this pull request Sep 29, 2022
sequential/test-tls-psk-client fails on IBM i #44821
Closed

Uh oh!

Uh oh!

Conversation

dmabupt commented Feb 26, 2020

Uh oh!

richardlau commented Feb 26, 2020

Uh oh!

dmabupt commented Feb 26, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richardlau commented Feb 26, 2020

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

dmabupt commented Feb 27, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmabupt commented Feb 27, 2020

Uh oh!

mhdawson commented Feb 27, 2020

Uh oh!

dmabupt commented Feb 28, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mhdawson commented Mar 2, 2020

Uh oh!

mhdawson commented Mar 2, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmabupt commented Mar 6, 2020

Uh oh!

mhdawson commented Mar 6, 2020

Uh oh!

dmabupt commented Mar 6, 2020

Uh oh!

mhdawson commented Mar 6, 2020

Uh oh!

dmabupt commented Mar 7, 2020

Uh oh!

mhdawson commented Mar 9, 2020

Uh oh!

dmabupt commented Mar 9, 2020

Uh oh!

mhdawson commented Mar 9, 2020

Uh oh!

dmabupt commented Mar 9, 2020

Uh oh!

dmabupt commented Mar 9, 2020

Uh oh!

mhdawson commented Mar 9, 2020

Uh oh!

dmabupt commented Mar 9, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mhdawson commented Mar 10, 2020

Uh oh!

dmabupt commented Mar 11, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmabupt commented Mar 11, 2020

Uh oh!

mhdawson commented Mar 12, 2020

Uh oh!

dmabupt commented Mar 16, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dmabupt commented Mar 17, 2020

Uh oh!

mhdawson left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Mar 29, 2020

Uh oh!

nodejs-github-bot commented Mar 30, 2020

Uh oh!

addaleax commented Mar 30, 2020

Uh oh!

addaleax commented Apr 13, 2020

Uh oh!

dmabupt commented Feb 26, 2020•
edited
Loading

dmabupt commented Feb 27, 2020•
edited
Loading

dmabupt commented Feb 28, 2020•
edited
Loading

mhdawson commented Mar 2, 2020•
edited
Loading

dmabupt commented Mar 9, 2020•
edited
Loading

dmabupt commented Mar 11, 2020•
edited
Loading

dmabupt commented Mar 16, 2020•
edited
Loading

dmabupt commented Apr 13, 2020•
edited
Loading