test: skip chmod-based permission tests when running as root#2171
Conversation
Two tests provoke EACCES via chmod (saveAliases backup double failure, appendSessionContent on a read-only file) and already skip on win32, but root ignores file modes so both fail when the suite runs as root (for example in a default Docker container). Every other chmod-based test in the repo already guards with process.getuid?.() === 0; these two were the only ones missing the guard. Apply the same skip condition and message.
|
Ready to act? Review this PR in Change Stack to turn feedback into patch suggestions you can inspect and refine. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTwo permission-sensitive test cases in the session management test suite now skip execution when running as root, in addition to the existing Windows platform skip. The skip conditions and associated messages are updated to reflect the broader guard for filesystem permission behaviors that do not behave consistently under root privilege. ChangesRoot User Test Skip Conditions
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…m#2171) Two tests provoke EACCES via chmod (saveAliases backup double failure, appendSessionContent on a read-only file) and already skip on win32, but root ignores file modes so both fail when the suite runs as root (for example in a default Docker container). Every other chmod-based test in the repo already guards with process.getuid?.() === 0; these two were the only ones missing the guard. Apply the same skip condition and message.
What Changed
Two tests provoke EACCES via chmod and already skip on win32, but root ignores file modes, so both fail when the suite runs as root (e.g. in a default Docker container):
tests/lib/session-aliases.test.js: "saveAliases triggers inner restoreErr catch when both save and restore fail"tests/lib/session-manager.test.js: "appendSessionContent returns false when file is read-only (EACCES)"Every other chmod-based test in the repo already guards with
process.getuid?.() === 0; these two were the only ones missing the guard. This applies the same skip condition and message.Why This Change
node tests/run-all.jsshould be green in any environment. As root the chmod calls succeed but do not restrict anything, so the code under test never sees EACCES and the assertions fail - an environment artifact, not a product bug.Testing Done
node tests/run-all.js) - 2619/2619 verified BOTH as a non-root user (tests execute, pass) and as root (tests skip, suite green) in clean node:22-bookworm containersprocess.getuid?.()is undefined-safe on Windows where the API does not existType of Change
test:TestsSecurity & Quality Checklist
Documentation
Summary by cubic
Skip two chmod-based permission tests when running as root (in addition to Windows) to avoid false failures. Aligns with existing
process.getuid?.() === 0guards and keepsnode tests/run-all.jsgreen in Docker/root environments.tests/lib/session-aliases.test.js: Skip on Windows/root; chmod cannot trigger EACCES in the save/restore double-fault case.tests/lib/session-manager.test.js: Skip on Windows/root for the read-only file (EACCES) check.Written for commit ffc2920. Summary will update on new commits.
Summary by CodeRabbit