FazBrowse GitHub Viewer | Trending | | Home

Tools: [Download Repo ZIP] [Original HTTPS Page]

Windows CI job update changes by joaocgreis · Pull Request #30724 · nodejs/node · GitHub

Windows CI job update changes - #30724

Closed

joaocgreis wants to merge 2 commits into

nodejs:masterfrom

JaneaSystems:joaocgreis-JBR-vcbuild-CI-master

joaocgreis commented Nov 30, 2019

Copy link Copy Markdown

Member

The Windows CI job currently includes direct calls to cctest and test.py. Moving most of the logic back to vcbuild has been in our backlog since the change was made.

This became necessary now because I'm working on an update to the CI job that makes it work for all Node.js versions, if the vcbuild targets are there. I will open backport PRs to active branches.

The second commit fixes an issue where failures cctest make the job become yellow rather than red. Since this works with the new target, this will only apply to the new job.

Refs: nodejs/build#1996

cc @nodejs/build @nodejs/platform-windows

Checklist

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

joaocgreis added 2 commits November 29, 2019 02:36

build,win: add test-ci-native and test-ci-js

6977a61

build,win: propagate error codes in vcbuild

5fbd4cf

Don't exit vcbuild with error code 0 when cctest fails.

nodejs-github-bot added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Nov 30, 2019

nodejs-github-bot commented Nov 30, 2019

Copy link Copy Markdown

Collaborator

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

This was referenced Nov 30, 2019

[v12.x backport] Windows CI job update changes #30725

Closed

[v10.x backport] Windows CI job update changes #30726

Closed

[v8.x backport] Windows CI job update changes #30727

Closed

rvagg approved these changes Dec 2, 2019

View reviewed changes

rvagg left a comment

Copy link Copy Markdown

Member

I'm surprised that test-ci was deprecated and now removed, is this because we run fanned and don't need them together?

joaocgreis commented Dec 2, 2019

Copy link Copy Markdown

Member Author

I believe test-ci was deprecated for nodejs/build#1761, the logic moved to the Jenkins job. This brings it back to vcbuild.bat. There are two test-ci-* targets because we will have two test jobs (preview in https://ci.nodejs.org/job/reis-test-commit-windows-fanned/).

@nodejs/build @nodejs/platform-windows please take a look, we are not running CI at full capacity because some resources have already been re-organized to test, I need this to land to complete the work. Thanks!

sam-github approved these changes Dec 2, 2019

View reviewed changes

Trott added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Dec 2, 2019

Trott approved these changes Dec 2, 2019

View reviewed changes

Trott commented Dec 2, 2019

Copy link Copy Markdown

Member

Landed in 8c101de...3b484ed

Trott pushed a commit that referenced this pull request Dec 2, 2019

build,win: add test-ci-native and test-ci-js

001d250

PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

Trott pushed a commit that referenced this pull request Dec 2, 2019

build,win: propagate error codes in vcbuild

3b484ed

Don't exit vcbuild with error code 0 when cctest fails.

PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

Trott closed this Dec 2, 2019

targos pushed a commit that referenced this pull request Dec 3, 2019

build,win: add test-ci-native and test-ci-js

6a53152

PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

targos pushed a commit that referenced this pull request Dec 3, 2019

build,win: propagate error codes in vcbuild

0aae502

Don't exit vcbuild with error code 0 when cctest fails.

PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

BridgeAR mentioned this pull request Dec 3, 2019

v13.3.0 proposal #30774

Merged

BethGriggs pushed a commit that referenced this pull request Dec 3, 2019

build,win: add test-ci-native and test-ci-js

c3e3862

Backport-PR-URL: #30726
PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

BethGriggs pushed a commit that referenced this pull request Dec 3, 2019

build,win: propagate error codes in vcbuild

4d3ec1a

Don't exit vcbuild with error code 0 when cctest fails.

Backport-PR-URL: #30726
PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

BethGriggs pushed a commit that referenced this pull request Dec 3, 2019

build,win: add test-ci-native and test-ci-js

7770279

Backport-PR-URL: #30727
PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

BethGriggs pushed a commit that referenced this pull request Dec 3, 2019

build,win: propagate error codes in vcbuild

8ca151d

Don't exit vcbuild with error code 0 when cctest fails.

Backport-PR-URL: #30727
PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

BethGriggs mentioned this pull request Dec 4, 2019

v10.18.1 proposal #30796

Closed

targos pushed a commit that referenced this pull request Dec 5, 2019

build,win: add test-ci-native and test-ci-js

8dd5eec

PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

targos pushed a commit that referenced this pull request Dec 5, 2019

build,win: propagate error codes in vcbuild

b18c8d3

Don't exit vcbuild with error code 0 when cctest fails.

PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

BethGriggs mentioned this pull request Dec 9, 2019

v12.13.2 proposal #30865

Closed

MylesBorins pushed a commit that referenced this pull request Dec 16, 2019

build,win: add test-ci-native and test-ci-js

208b813

Backport-PR-URL: #30727
PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

MylesBorins pushed a commit that referenced this pull request Dec 16, 2019

build,win: add test-ci-native and test-ci-js

54a466a

Backport-PR-URL: #30726
PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

MylesBorins pushed a commit that referenced this pull request Dec 16, 2019

build,win: add test-ci-native and test-ci-js

f01959a

PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

MylesBorins pushed a commit that referenced this pull request Dec 17, 2019

build,win: propagate error codes in vcbuild

3b096c2

Don't exit vcbuild with error code 0 when cctest fails.

PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

MylesBorins pushed a commit that referenced this pull request Dec 17, 2019

build,win: propagate error codes in vcbuild

68b2b5c

Don't exit vcbuild with error code 0 when cctest fails.

Backport-PR-URL: #30726
PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

MylesBorins pushed a commit that referenced this pull request Dec 17, 2019

build,win: propagate error codes in vcbuild

81b22dd

Don't exit vcbuild with error code 0 when cctest fails.

Backport-PR-URL: #30727
PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

BethGriggs mentioned this pull request Dec 23, 2019

v12.14.1 proposal #31069

Merged

BethGriggs pushed a commit that referenced this pull request Dec 31, 2019

build,win: propagate error codes in vcbuild

dc52988

Don't exit vcbuild with error code 0 when cctest fails.

PR-URL: #30724
Refs: nodejs/build#1996
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

bnoordhuis reviewed Jan 2, 2020

View reviewed changes

Comment thread vcbuild.bat

    
              if /i "%1"=="licensertf"    set licensertf=1&goto arg-ok

              if /i "%1"=="test"          set test_args=%test_args% -J %common_test_suites%&set lint_cpp=1&set lint_js=1&set lint_md=1&goto arg-ok

              :: test-ci is deprecated

              if /i "%1"=="test-ci"       goto arg-ok

bnoordhuis Jan 2, 2020

Copy link Copy Markdown

Member

The removal of the test-ci target breaks some of the CI jobs, see https://ci.nodejs.org/job/node-test-commit-windows/976/nodes=win10/console:

C:\workspace\node-test-commit-windows\nodes\win10>vcbuild.bat release nosign x64 test-ci ignore-flaky NODE_TEST_DIR=C:\node-tmp  
Note: vcbuild no longer signs by default. "nosign" is redundant.
Error: invalid command line option `test-ci`.

(Apologies if it's already been reported. I searched nodejs/node and nodejs/build but didn't find an exact match.)

joaocgreis Jan 2, 2020

Copy link Copy Markdown

Member Author

@bnoordhuis any reason not to use node-test-commit-windows-fanned instead? node-test-commit-windows has been forgotten for a long time, I'll archive it if there's no objection.

Note that test-ci was a no-op as removed here, running with it only compiled, did not run tests.

bnoordhuis Jan 3, 2020

Copy link Copy Markdown

Member

Ah, if it's unmaintained then yes, please remove or archive it to avoid confusion. :-)

bnoordhuis mentioned this pull request Jan 2, 2020

v8: include right headers in torque output nodejs/node-v8#133

Closed

BethGriggs mentioned this pull request Jan 7, 2020

v10.18.1 proposal #31248

Merged

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters

Show hidden characters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

bnoordhuis left review comments

Trott approved these changes

+2 more reviewers

sam-github approved these changes

rvagg approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform.

Projects

Milestone

No milestone

Back | FazBrowse Home | New Git URL