node-stress-single-test job doesn't compile test bindings

[richardlau]

Refs: nodejs/node#8667 (comment)

It looks like node-stress-single-test cannot runs tests that require bindings (e.g., gc, https://ci.nodejs.org/job/node-stress-single-test/1760/nodes=win10-1p-vs2015/) as it doesn't build the tests.

[gibfahn]

Yep, the windows version runs neither the build-addons or the build-addons-napi commands.

What should I actually run to build addons? I don't see addons build options in vcbuild.bat, just the test versions:

https://github.com/nodejs/node/blob/c46e36b073678ea410ecc649f34c5346a9eb735a/vcbuild.bat#L75-L76

If you let me know what to add I'm happy to add it.

[gabrielschulhof]

Looks like we need to increase the granularity, because test-addons-napi both builds and tests.

[gibfahn]

The other problem is that once we've added this we'll need to get it backported, and probably do "if vcbuild.bat has this option then run it", the equivalent of vcbuild.bat build-addons || true.

[maclover7]

@richardlau Are you able to confirm if this has been solved?

[richardlau]

I don't think any changes were made, but let's find out: https://ci.nodejs.org/job/node-stress-single-test/1869/ 😄

[richardlau]

@maclover7
Still failing due to addons not being compiled:
https://ci.nodejs.org/job/node-stress-single-test/1869/nodes=win2016-1p-vs2017/console

23:43:07   Generating code

23:54:23   All 153726 functions were compiled because no usable IPDB/IOBJ from previous compilation was found.

23:54:27   Finished generating code

23:55:10   cctest.vcxproj -> c:\workspace\node-stress-single-test\nodes\win2016-1p-vs2017\Release\\cctest.exe

23:55:18 TAP version 13
23:55:18 1..1
23:55:18 not ok 1 gc/test-http-client-onerror
23:55:18   ---
23:55:18   duration_ms: 0.239
23:55:18   severity: fail
23:55:18   exitcode: 1
23:55:18   stack: |-
23:55:18     internal/modules/cjs/loader.js:596
23:55:18         throw err;
23:55:18         ^
23:55:18     
23:55:18     Error: Cannot find module './build/Release/binding'
23:55:18         at Function.Module._resolveFilename (internal/modules/cjs/loader.js:594:15)
23:55:18         at Function.Module._load (internal/modules/cjs/loader.js:520:25)
23:55:18         at Module.require (internal/modules/cjs/loader.js:650:17)
23:55:18         at require (internal/modules/cjs/helpers.js:20:18)
23:55:18         at Object.<anonymous> (c:\workspace\node-stress-single-test\nodes\win2016-1p-vs2017\test\gc\test-http-client-onerror.js:14:14)
23:55:18         at Module._compile (internal/modules/cjs/loader.js:702:30)
23:55:18         at Object.Module._extensions..js (internal/modules/cjs/loader.js:713:10)
23:55:18         at Module.load (internal/modules/cjs/loader.js:612:32)
23:55:18         at tryModuleLoad (internal/modules/cjs/loader.js:551:12)
23:55:18         at Function.Module._load (internal/modules/cjs/loader.js:543:3)
23:55:18   ...
23:55:18 1   OK: 0   NOT OK: 1   TOTAL: 5

nodejs/node#19637 introduced new targets for vcbuild.bat to build addons (although looking again they may not build the gc addons) but as far as I can see they are not being used in the job:

22:18:12 c:\workspace\node-stress-single-test\nodes\win2016-1p-vs2017>call vcbuild.bat release nosign x64 

As @gibfahn pointed out in #1097 (comment) ideally the PR needs to be backported so that the same target can be run against any of the release lines and the stress job changed to run the target prior to running the tests.

[maclover7]

@richardlau Would you be able to handle the backport, and then once backported, we can edit the CI job configuration?

[richardlau]

Would you be able to handle the backport, and then once backported, we can edit the CI job configuration?

Yes, but not until next week at the earliest.

[Trott]

I believe this was resolved and is now broken again, but I'm about to fix it.

Question, though:

In https://ci.nodejs.org/job/node-stress-single-test/configure, I'm about to edit the "Steps to run if condition is met" section to replace this:

  PYTHON=python make build-addons
  PYTHON=python make build-addons-napi

with this:

  PYTHON=python make test-build

Where do I document this change after I make it, or is there a repo, or do I not worry about it much because Jenkins keeps history, or what? I don't know the process. Maybe I need to be re-onboarded some time.

[richardlau]

@Trott I think the protocol is to log in an issue (such as this one) with the link to the job configuration history change and a brief summary of what was done (mainly for visibility, I don't think any of us are constantly monitoring job change history for all of the jobs in the CI).

e.g. https://ci.nodejs.org/job/node-stress-single-test/jobConfigHistory/showDiffFiles?timestamp1=2018-11-29_23-19-20&timestamp2=2018-12-07_11-55-34 is the change you just made.

re. the change, can you check the job still works on the older releases (e.g. 8.x) as well as master?

[richardlau]

I believe this was resolved and is now broken again, but I'm about to fix it.

Oh and it's probably not resolved as I think the original issue was Windows, which uses vcbuild.bat and not the Makefile targets. I'll re-add it to my list of todos (especially as I can now edit the stress job).

[Trott]

re. the change, can you check the job still works on the older releases (e.g. 8.x) as well as master?

Works on master (I just ran a test), but won't work on 8.x, I'm pretty sure, unless/until the change in the Makefile is backported there. FWIW, I don't know that we've ever run node-stress-single-test on anything other than master. For flaky tests (as opposed to routinely failing tests) on release branches, we tend to just shrug rather than do a whole bunch of troubleshooting.

If we want this task to work in release branches, I guess I could try something like PYTHON=python make test-build || make test-build-addons test-build-addons-napi?

[richardlau]

hmm, the test-build target exists back in 6.x
https://github.com/nodejs/node/blob/92968b6fda3d9df1a5d318347146397cc5960afc/Makefile#L239
so maybe it will work. Although that then begs the question why the job wasn't using test-build in the first place.

¯\_(ツ)_/¯

[github-actions]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.