build: for --enable-static, run only cctest

[danbev]

Refs: #14158

Currently when building with --enable-static and running the test target
the following error will be reported:

Building addon
/node/test/addons/01_function_arguments/
env: ./node: No such file or directory
make[1]: *** [test/addons/.buildstamp] Error 1

Note that this is with a clean build where no prior node executable was
built.

This commit suggests only running the cctest target when --enable-static
is specified.

_{[refack: added refs]}

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

build

[danbev]

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

[mscdex]

The commit message wording is a little confusing, it makes it sound as if the only time cctest is run is when --enable-static is used. Perhaps swapping the order may make it clearer (e.g. 'build: for --enable-static, run only cctest')?

[refack]

LGTM until we have a harness to test the static/dynamic library targets.
@danbev nice to have: same change in vcbuild.bat but not a blocker for me.

[danbev]

The commit message wording is a little confusing, it makes it sound as if the only time cctest is run is when --enable-static is used. Perhaps swapping the order may make it clearer (e.g. 'build: for --enable-static, run only cctest')?

Yesterday was not a good day :( I agree and reading it now it is confusing. I like your suggestion and I'll update now. Thanks.

[danbev]

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

[danbev]

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

[danbev]

test/osx failure looks unrelated

console output:

Triggering node-test-commit-osx » osx1010
Configuration node-test-commit-osx » osx1010 is still in the queue: Waiting for next available executor on test-requireio-osx1010-x64-1
node-test-commit-osx » osx1010 completed with result FAILURE
Notifying upstream projects of job completion
Finished: FAILURE

test/arm failure looks unrelated

console output:

not ok 796 parallel/test-http2-multiplex
  ---
  duration_ms: 1.425
  severity: fail
  stack: |-
    (node:28491) ExperimentalWarning: The http2 module is an experimental API.
    assert.js:42
      throw new errors.AssertionError({
      ^
    
    AssertionError [ERR_ASSERTION]: 'abcdefjj' === 'abcdefghij'
        at ClientHttp2Stream.req.on.common.mustCall (/home/iojs/build/workspace/node-test-commit-arm/nodes/ubuntu1604-arm64_odroid_c2/test/parallel/test-http2-multiplex.js:41:14)
        at ClientHttp2Stream.<anonymous> (/home/iojs/build/workspace/node-test-commit-arm/nodes/ubuntu1604-arm64_odroid_c2/test/common/index.js:509:15)
        at emitNone (events.js:110:20)
        at ClientHttp2Stream.emit (events.js:207:7)
        at endReadableNT (_stream_readable.js:1059:12)
        at _combinedTickCallback (internal/process/next_tick.js:138:11)
        at process._tickCallback (internal/process/next_tick.js:180:9)
  ...

[danbev]

@refack Would you be able to take a look at the windows related update for this?

[refack]

@refack Would you be able to take a look at the windows related update for this?

Thanks for remembering vcbuild.bat!

cmd's if is not intuitive in the way it deals with multiple statements in the then, so I suggest to flip it (if defined X "skip").
I pushed a suggestion commit with the a change that I've tested. Obviously feel free to push it out if you don't like it.

[lance]

Should $(MAKE) lint not also be run as a part of the static build?

[refack]

I'm +0 on that.
It is already run by the patched vcbuild.bat though.

[refack]

No, wait that's a problem, if there's no node binary that will fail.

[lance]

Ah now I see. Good point. 👍

~/s/node ❯❯❯ make lint                                                                                  master ⬆ ⬇ ✭ ◼ │
Running JS linter...                                                                                                   │
./node tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules --ext=.js,.md \                                │
          benchmark doc lib test tools                                                                                 │
make[1]: ./node: No such file or directory                                                                             │
make[1]: *** [jslint] Error 1

[lance]

Just so I understand... when building with --enable-static, no node binary is produced, and so therefore no node tests are run. So if I'm a developer hacking on Node core for my custom embedded node, my workflow would need to be such that when I make changes to Node core, I will need to first run make -j4 test, to build a node binary to test against, and then run make --enable-static to produce my static library?

Really just curious here, but how difficult would it be (and is it really desirable) to have a test harness that, when building with --enable-static loads the static library and feeds the tests through that. Or is this just a lot of work for negligible gain?

[danbev]

@lance Yeah, that is my understanding of the workflow at the moment.

Really just curious here, but how difficult would it be (and is it really desirable) to have a test harness that, when building with --enable-static loads the static library and feeds the tests through that. Or is this just a lot of work for negligible gain?

This idea came up earlier where we thought of doing something like this for the static lib and also when building a shared library. I have a done some work on this and hope to follow this up with a suggestion. I need to sort out a couple of issue I ran into. Hopefully the work on --enable-static we done to this point at least improves things a little.

[danbev]

@refack Thanks for the windows update!

[danbev]

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

[refack]

Cross-ref: #14986

[danbev]

Landed in a36b540

[MylesBorins]

This does not land cleanly in LTS. Please feel free to manually backport by following the guide. Please also feel free to replace do-not-land if it is being backported