Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Windows CI job update changes #30724

Closed
wants to merge 2 commits into from
Closed

Windows CI job update changes #30724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6977a61
build,win: add test-ci-native and test-ci-js
joaocgreis Nov 29, 2019
5fbd4cf
build,win: propagate error codes in vcbuild
joaocgreis Nov 30, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Expand Up @@ -489,6 +489,7 @@ test-all-valgrind: test-build
test-all-suites: | clear-stalled test-build bench-addons-build doc-only ## Run all test suites.
$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) test/*

# CI_* variables should be kept synchronized with the ones in vcbuild.bat
CI_NATIVE_SUITES ?= addons js-native-api node-api
CI_JS_SUITES ?= default
ifeq ($(node_use_openssl), false)
Expand Down
18 changes: 13 additions & 5 deletions vcbuild.bat
View file
Open in desktop
Expand Up @@ -15,6 +15,13 @@ if /i "%1"=="/?" goto help

cd %~dp0

@rem CI_* variables should be kept synchronized with the ones in Makefile
set CI_NATIVE_SUITES=addons js-native-api node-api
set CI_JS_SUITES=default
set CI_DOC=doctool
@rem Same as the test-ci target in Makefile
set "common_test_suites=%CI_JS_SUITES% %CI_NATIVE_SUITES% %CI_DOC%&set build_addons=1&set build_js_native_api_tests=1&set build_node_api_tests=1"

@rem Process arguments.
set config=Release
set target=Build
Expand Down Expand Up @@ -51,10 +58,8 @@ set build_js_native_api_tests=
set build_node_api_tests=
set test_node_inspect=
set test_check_deopts=
set js_test_suites=default
set v8_test_options=
set v8_build_options=
set "common_test_suites=%js_test_suites% doctool addons js-native-api node-api&set build_addons=1&set build_js_native_api_tests=1&set build_node_api_tests=1"
set http2_debug=
set nghttp2_debug=
set link_module=
Expand All @@ -63,6 +68,7 @@ set cctest=
set openssl_no_asm=
set doc=
set extra_msbuild_args=
set exit_code=0

:next-arg
if "%1"=="" goto args-done
Expand All @@ -86,8 +92,8 @@ if /i "%1"=="noetw" set noetw=1&goto arg-ok
if /i "%1"=="ltcg" set ltcg=1&goto arg-ok
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if /i "%1"=="test-ci-native" set test_args=%test_args% %test_ci_args% -J -p tap --logfile test.tap %CI_NATIVE_SUITES% %CI_DOC%&set build_addons=1&set build_js_native_api_tests=1&set build_node_api_tests=1&set cctest_args=%cctest_args% --gtest_output=xml:cctest.junit.xml&goto arg-ok
if /i "%1"=="test-ci-js" set test_args=%test_args% %test_ci_args% -J -p tap --logfile test.tap %CI_JS_SUITES%&set no_cctest=1&goto arg-ok
if /i "%1"=="build-addons" set build_addons=1&goto arg-ok
if /i "%1"=="build-js-native-api-tests" set build_js_native_api_tests=1&goto arg-ok
if /i "%1"=="build-node-api-tests" set build_node_api_tests=1&goto arg-ok
Expand Down Expand Up @@ -628,9 +634,11 @@ if defined no_cctest echo Skipping cctest because no-cctest was specified && got
if not exist "%config%\cctest.exe" echo cctest.exe not found. Run "vcbuild test" or "vcbuild cctest" to build it. && goto run-test-py
echo running 'cctest %cctest_args%'
"%config%\cctest" %cctest_args%
if %errorlevel% neq 0 set exit_code=%errorlevel%
:run-test-py
echo running 'python tools\test.py %test_args%'
python tools\test.py %test_args%
if %errorlevel% neq 0 set exit_code=%errorlevel%
goto test-v8

:test-v8
Expand Down Expand Up @@ -710,7 +718,7 @@ echo vcbuild.bat no-cctest : skip building cctest.exe
goto exit

:exit
goto :EOF
exit /b %exit_code%


rem ***************
Expand Down