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

test-tls-env-extra-ca-file-load fails to test assertions #32072

Closed
ebickle opened this issue Mar 3, 2020 · 0 comments
Closed

test-tls-env-extra-ca-file-load fails to test assertions #32072

ebickle opened this issue Mar 3, 2020 · 0 comments

Comments

@ebickle
Copy link
Contributor

ebickle commented Mar 3, 2020

  • Version: master (v14.0.0-pre)
  • Platform: Windows 10 Version 1903 64-bit (OS Build 18362.657)
  • Subsystem: crypto / tls

What steps will reproduce the bug?

Bug identified in a code review and cannot be directly reproduced: a unit test silently succeeds without executing it's assertions or running any tests.

The unit test test-tls-env-extra-ca-file-load.js attempts to fork two child processes with the NODE_EXTRA_CA_CERTS environment variable set and unset. The unit test logic of the parent process and child process are gated inside of a if (process.argv[2] !== 'child') statement.

The code blocks for the parent and child processes are swapped. During execution the child process argument is not set and the code falls into the assertion block. The CHILD_USE_EXTRA_CA_CERTS isn't set to yes or no, causing all assertions to be bypassed and the unit test to exit without doing anything. The process is never forked.

How often does it reproduce? Is there a required condition?

Reproduces 100% of the time.

What is the expected behavior?

The unit test should fork two child processes and execute the appropriate assertions.

What do you see instead?

Unit test silently finishes without forking any child processes or running assertions.

ebickle added a commit to ebickle/node that referenced this issue Mar 3, 2020
Fixes broken unit test for the NODE_EXTRA_CA_CERTS environment variable. Unit test was exiting without evaluating any assertions or running any tests.

Fixes: nodejs#32072
ebickle added a commit to ebickle/node that referenced this issue Mar 5, 2020
Fixes broken unit test for the NODE_EXTRA_CA_CERTS environment variable. Unit test was exiting without evaluating any assertions or running any tests.

Fixes: nodejs#32072
BridgeAR pushed a commit that referenced this issue Mar 17, 2020
Fixes broken unit test for the NODE_EXTRA_CA_CERTS environment
variable. Unit test was exiting without evaluating any assertions
or running any tests.

Fixes: #32072

PR-URL: #32073
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this issue Mar 24, 2020
Fixes broken unit test for the NODE_EXTRA_CA_CERTS environment
variable. Unit test was exiting without evaluating any assertions
or running any tests.

Fixes: #32072

PR-URL: #32073
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this issue Apr 22, 2020
Fixes broken unit test for the NODE_EXTRA_CA_CERTS environment
variable. Unit test was exiting without evaluating any assertions
or running any tests.

Fixes: #32072

PR-URL: #32073
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant