Skip to content

test: skip tests failing when run under root #58610

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

Merged
merged 2 commits into from
Jun 10, 2025

Conversation

LiviaMedeiros
Copy link
Member

No description provided.

@LiviaMedeiros LiviaMedeiros added test Issues and PRs related to the tests. needs-ci PRs that need a full CI run. labels Jun 6, 2025
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/config
  • @nodejs/sqlite

Copy link

codecov bot commented Jun 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.16%. Comparing base (c1f090d) to head (b84f971).
Report is 33 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #58610      +/-   ##
==========================================
- Coverage   90.20%   90.16%   -0.05%     
==========================================
  Files         636      636              
  Lines      187698   188030     +332     
  Branches    36852    36899      +47     
==========================================
+ Hits       169316   169529     +213     
- Misses      11165    11271     +106     
- Partials     7217     7230      +13     

see 49 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@geeksilva97
Copy link
Contributor

geeksilva97 commented Jun 7, 2025

out of curiosity, what does that isRoot mean, and why do we wanna skip these tests specifically?

@LiviaMedeiros
Copy link
Member Author

root is superuser, with UID === 0.
These two tests have to be skipped because they test inability of process to perform read/write operations on write/read-only file, and therefore fail because root ignores file permissions.

@LiviaMedeiros LiviaMedeiros added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 7, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 7, 2025
@nodejs-github-bot

This comment was marked as outdated.

@geeksilva97
Copy link
Contributor

root is superuser, with UID === 0. These two tests have to be skipped because they test inability of process to perform read/write operations on write/read-only file, and therefore fail because root ignores file permissions.

Thank you, @LiviaMedeiros

@nodejs-github-bot
Copy link
Collaborator

@lpinca
Copy link
Member

lpinca commented Jun 10, 2025

Windows failures are related

---
duration_ms: 299.999
exitcode: 1
severity: fail
stack: |-
  d:\workspace\node-test-binary-windows-js-suites\node\test\parallel\test-config-file.js:11
  const isRoot = process.getuid() === 0;
                         ^

  TypeError: process.getuid is not a function
      at Object.<anonymous> (d:\workspace\node-test-binary-windows-js-suites\node\test\parallel\test-config-file.js:11:24)
      at Module._compile (node:internal/modules/cjs/loader:1733:14)
      at Module._extensions..js (node:internal/modules/cjs/loader:1898:10)
      at Module.load (node:internal/modules/cjs/loader:1468:32)
      at Module._load (node:internal/modules/cjs/loader:1285:12)
      at TracingChannel.traceSync (node:diagnostics_channel:322:14)
      at wrapModuleLoad (node:internal/modules/cjs/loader:235:24)
      at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:152:5)
      at node:internal/main/run_main_module:33:47

  Node.js v25.0.0-pre
...

@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jun 10, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 10, 2025
@nodejs-github-bot nodejs-github-bot merged commit be2120f into nodejs:main Jun 10, 2025
60 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in be2120f

seriousme pushed a commit to seriousme/node that referenced this pull request Jun 10, 2025
PR-URL: nodejs#58610
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
targos pushed a commit that referenced this pull request Jun 16, 2025
PR-URL: #58610
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants