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

parallel/test-dotenv-node-options skipped #50346

Closed
laijonathan opened this issue Oct 23, 2023 · 7 comments · Fixed by #53545
Closed

parallel/test-dotenv-node-options skipped #50346

laijonathan opened this issue Oct 23, 2023 · 7 comments · Fixed by #53545

Comments

@laijonathan
Copy link

laijonathan commented Oct 23, 2023

I noticed that parallel/test-dotenv-node-options is skipped in the Linux CI/CD test results on Jenkins

eg: https://ci.nodejs.org/job/node-test-commit-linux/nodes=rhel8-x64/lastCompletedBuild/testReport/(root)/parallel/test_dotenv_node_options_/

I also checked test/parallel/parallel.status and it doesn't look like this testcase was ever marked as skipped.

Does anyone know why this is skipped? Is there an issue with the testcase or the dotenv feature?

Thanks

@richardlau
Copy link
Member

It's not obvious to me either. In https://ci.nodejs.org/job/node-test-commit-linux/nodes=rhel8-x64/54511/consoleFull we see:

19:59:23 ok 927 parallel/test-dotenv-edge-cases # skip 0
19:59:23   ---
19:59:23   duration_ms: 276.99400
19:59:23   ...
19:59:23 ok 928 parallel/test-dotenv-node-options # skip 0
19:59:23   ---
19:59:23   duration_ms: 311.21900
19:59:23   ...

Both of which are marked skipped:

Maybe it's a testcase issue, or testrunner bug?
test/parallel/test-dotenv-node-options.js has a couple of possible skips (but neither of them look like they should be triggering):

if (process.config.variables.node_without_node_options) {
common.skip('missing NODE_OPTIONS support');
}

it('TZ environment variable', { skip: !common.hasIntl || process.config.variables.icu_small }, async () => {

However test/parallel/test-dotenv-edge-cases.js does not contain any skip statements.
cc @nodejs/test_runner

@laijonathan
Copy link
Author

FWIW, I have tried running this testcase manually with a combination of flags from https://nodejs.org/api/permissions.html#process-based-permissions but I have never managed to get it to pass.
I just get a combination of Error: Access to this API has been restricted and TypeError: Cannot read properties of undefined (reading 'has') even with the correct flags.

eg: https://stackoverflow.com/questions/76454106/i-get-typeerror-cannot-read-properties-of-undefined-reading-has-when-checki

@cjihrig
Copy link
Contributor

cjihrig commented Oct 23, 2023

skip 0

It looks like #48929 may have introduced this behavior in the test runner. skip is documented to skip the test if a truthy value is provided. However, that change removes the coercion to a boolean.

@anonrig
Copy link
Member

anonrig commented Jan 26, 2024

Does this issue still persist?

@richardlau
Copy link
Member

Does this issue still persist?

Yes. From today's daily build, https://ci.nodejs.org/job/node-test-commit-linux/55932/nodes=rhel8-x64/testReport/(root)/parallel/test_dotenv_node_options_/ still shows the test as being skipped and https://ci.nodejs.org/job/node-test-commit-linux/55932/nodes=rhel8-x64/consoleFull

08:08:48 ok 928 parallel/test-dotenv-edge-cases # skip 0
08:08:48   ---
08:08:48   duration_ms: 269.52700
08:08:48   ...
08:08:48 ok 929 parallel/test-dotenv-node-options # skip 0
08:08:48   ---
08:08:48   duration_ms: 269.23600
08:08:48   ...

@richardlau
Copy link
Member

This also appears to affect test/parallel/test-node-output-errors.mjs
e.g. https://ci.nodejs.org/job/node-test-commit-linux/58512/nodes=ubuntu2204-64/testReport/(root)/parallel/test_node_output_errors_/

@richardlau
Copy link
Member

This is the Python test harness (mis)interpreting the # skipped line in the summary from the built-in test runner.
Fix (and more detailed explanation): #53545

nodejs-github-bot pushed a commit that referenced this issue Jun 24, 2024
Fix the Python test harness so that it no longer treats the `# skipped`
part of the summary at the end of the built-in test runner output as
marking the test as skipped.

PR-URL: #53545
Fixes: #50346
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
targos pushed a commit that referenced this issue Jun 25, 2024
Fix the Python test harness so that it no longer treats the `# skipped`
part of the summary at the end of the built-in test runner output as
marking the test as skipped.

PR-URL: #53545
Fixes: #50346
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
targos pushed a commit that referenced this issue Jun 25, 2024
Fix the Python test harness so that it no longer treats the `# skipped`
part of the summary at the end of the built-in test runner output as
marking the test as skipped.

PR-URL: #53545
Fixes: #50346
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
marco-ippolito pushed a commit that referenced this issue Jul 19, 2024
Fix the Python test harness so that it no longer treats the `# skipped`
part of the summary at the end of the built-in test runner output as
marking the test as skipped.

PR-URL: #53545
Fixes: #50346
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
marco-ippolito pushed a commit that referenced this issue Jul 19, 2024
Fix the Python test harness so that it no longer treats the `# skipped`
part of the summary at the end of the built-in test runner output as
marking the test as skipped.

PR-URL: #53545
Fixes: #50346
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
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.

4 participants