-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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: disallow running the test suite as root #2585
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’d merge and if there is a need we can add the escape hatch back maybe? |
Yes, that seems fine. |
I added back the escape hatch. If |
New CI: https://ci.nodejs.org/view/libuv/job/libuv-test-commit/1667/ (Looks OK) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm with the escape hatch.
The test suite is known to have issues when run as root. This commit prevents the test suite from running when the euid is 0. Fixes: libuv#2549 PR-URL: libuv#2585 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
9f05a92
to
af45b6b
Compare
The test suite is known to have issues when run as root. This commit prevents the test suite from running when the euid is 0.
I'm not sure how the other maintainers will feel about this change, but people clearly don't read the GitHub issue template, so I thought this was worth a shot to prevent issues like #2549 (and many more).
CI: https://ci.nodejs.org/view/libuv/job/libuv-test-commit/1666/ (looks like a normal libuv CI run)