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.

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

win,build: set exit_code on configure failure #32205

Closed
wants to merge 1 commit into from

Conversation

bartbrzo
Copy link

Fixes: #31573

Changed vcbuild.bat to exit with an error in case of exception in the configure script.

In case of failure, the error was not propagated because errorlevel was overwritten with the result of del command.

This should land in all LTS branches.

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

@nodejs-github-bot 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 Mar 11, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@bzoz
Copy link
Contributor

bzoz commented Mar 27, 2020

The one failing test is:

18:03:38 not ok 556 parallel/test-tls-root-certificates
18:03:38   ---
18:03:38   duration_ms: 0.412
18:03:38   severity: fail
18:03:38   exitcode: 1
18:03:38   stack: |-
18:03:38     assert.js:384
18:03:38         throw err;
18:03:38         ^
18:03:38     
18:03:38     AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
18:03:38     
18:03:38       assert(tls.rootCertificates.includes(extraCert))
18:03:38     
18:03:38         at Object.<anonymous> (C:\workspace\node-test-binary-windows-js-suites\node\test\parallel\test-tls-root-certificates.js:49:3)
18:03:38         at Module._compile (internal/modules/cjs/loader.js:1202:30)
18:03:38         at Object.Module._extensions..js (internal/modules/cjs/loader.js:1222:10)
18:03:38         at Module.load (internal/modules/cjs/loader.js:1051:32)
18:03:38         at Function.Module._load (internal/modules/cjs/loader.js:947:14)
18:03:38         at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
18:03:38         at internal/main/run_main_module.js:17:47 {
18:03:38       generatedMessage: true,
18:03:38       code: 'ERR_ASSERTION',
18:03:38       actual: false,
18:03:38       expected: true,
18:03:38       operator: '=='
18:03:38     }
18:03:38     assert.js:102
18:03:38       throw new AssertionError(obj);
18:03:38       ^
18:03:38     
18:03:38     AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
18:03:38     
18:03:38     1 !== 0
18:03:38     
18:03:38         at ChildProcess.<anonymous> (C:\workspace\node-test-binary-windows-js-suites\node\test\parallel\test-tls-root-certificates.js:19:12)
18:03:38         at ChildProcess.<anonymous> (C:\workspace\node-test-binary-windows-js-suites\node\test\common\index.js:362:15)
18:03:38         at ChildProcess.emit (events.js:315:20)
18:03:38         at Process.ChildProcess._handle.onexit (internal/child_process.js:276:12) {
18:03:38       generatedMessage: true,
18:03:38       code: 'ERR_ASSERTION',
18:03:38       actual: 1,
18:03:38       expected: 0,
18:03:38       operator: 'strictEqual'
18:03:38     }

This is unrelated and tracked in nodejs/build#2254

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 29, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@addaleax
Copy link
Member

Landed in 5e05f8a

@addaleax addaleax closed this Mar 30, 2020
addaleax pushed a commit that referenced this pull request Mar 30, 2020
PR-URL: #32205
Fixes: #31573
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this pull request Mar 30, 2020
PR-URL: #32205
Fixes: #31573
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request Apr 22, 2020
PR-URL: #32205
Fixes: #31573
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, 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
None yet
Development

Successfully merging this pull request may close these issues.

False positive build result in vcbuild.bat
6 participants