build, windows: build and test add-ons on test-ci

[blobor]

Pull Request check-list

Please make sure to review and check all of these items:

• Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
  this change (including linting)?
• Is the commit message formatted according to CONTRIBUTING.md?

Affected core subsystem(s)

build on Windows OS

Description of change

added build-addons task, it allows to build and test native addons during test-ci task.
Basicaly it should work in same way like Makefile "build-addons" task.

Fixes: #2537

[rvagg]

ok then https://ci.nodejs.org/job/node-test-pull-request/2053/

lgtm tho this is stretching my .bat skillz

/cc @nodejs/platform-windows

[blobor]

looks like build is failing

[rvagg]

oh right, that's because we build binaries on one server and fan the tests across a number of non-build servers, so it's going to be a bit more complicated than just activating this in test-ci, @joaocgreis over to you to weigh in on the feasability of this, do we do this for the rpi fanned tests already?

[blobor]

@rvagg I saw in build logs:

c:\workspace\node-test-binary-windows\RUN_SUBSET\0\VS_VERSION\vcbt2015\label\win10>tar xavf binary/binary.tar.gz 
config.gypi
icu_config.gypi
Release/node.exe
Release/openssl-cli.exe
Release/cctest.exe

So, before we start running tests, we unpacking binaries.
Basically, I just need to move "node_exe" variable assignment on the top. And this should fix this issue.

[blobor]

Build could fail on "load-long-path" test due to: #3667

[blobor]

Hey @rvagg, could you please try tun CI build again?

[joaocgreis]

Hi @blobor , thanks for contributing!

In the CI server we test Windows and Raspberry Pis with some added logic, because we compile and run tests in different machines. The Pis are already running addons tests, the test runners execute make test/addons/.buildstamp before calling test.py (...) addons directly, without using make. However, windows still uses test-ci. You can simulate the current commands locally with:

vcbuild.bat release nosign x64
tar cavf ../binary.tar.gz config.gypi icu_config.gypi Release/node.exe Release/openssl-cli.exe Release/cctest.exe
git clean -fdx
tar xavf ../binary.tar.gz
vcbuild.bat release nosign x64 nobuild test-ci ignore-flaky

(we use the tar bundled with Git for Windows, but it doesn't have to be tar, just have to save those exact files and delete everything else)

We can start saving node.lib if it is needed.

I think this would be great if you could keep the build_addons variable, but instead of adding a if /i "%1"=="build-addons", add a if /i "%1"=="test-addons" that sets build_addons and only tests addons? addons also needs to be added to test.

[blobor]

Hey @joaocgreis, thanks for your reply.

I've made changes accordingly to your comment. Could you please review them?

[joaocgreis]

The correct place for this is after all set config= have happened, before the configure_flags are prepared, exactly at

node/vcbuild.bat

Line 93 in 089c6a4

. @blobor can you move this?

[joaocgreis]

The quotes should not be stored in the variable, but added to every place where the variable is used, to be consistent with all other variables in this file.

[joaocgreis]

@blobor I did a careful review, left some comments. Can you address them? Thanks!

I've updated the CI job to pack node.lib, so next time I expect it to pass.

Current status is:

• release x64: builds and tests, all ok
• release x86: fails load-long-path, as @blobor mentioned above
• debug: fails all but repl-domain-abort, as it does in Unix

I believe this should land with the release x86 failure. That configuration is not tested in CI, so it will not block. That failure is not a consequence of this and is being tracked at #3667 , so this might even be useful in fixing it.

[blobor]

@joaocgreis, thanks for review. I've made those changes.

[joaocgreis]

LGTM

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

Will land in about 24h if CI is ok and there are no objections.

[blobor]

Looks like CI failed again on win 2008 VS2013 machine.

[joaocgreis]

Looks like a problem with Windows SDK on that machine, I've seen it on others. I can't do it right now, but I'll fix it and re-run CI. Everything else passed, so I don't expect a problem there.

[blobor]

@joaocgreis, great! Thank you so much for your help!

[joaocgreis]

CI passed this time: https://ci.nodejs.org/job/node-test-commit/2723/

Landed in: c7138e9

Thank you @blobor !