test: set umask explicitly#25213
test: set umask explicitly #25213
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Some tests which create files and check file permissions assume the umask is compatible with 022, and break when set to something like 007. Explicitly set umask to 022
nodejs-github-bot added test 
Trott commented Dec 25, 2018
The issue here is that the temp directory is being created with more restrictive permissions than that being used by the test. Rather than fixing in the Python test runner, might it be better to change the
Counterarguments I can think of would be that this solution is simpler, and there may be edge cases where it's more robust too although I can't think of any. |
i8-pi commented Dec 26, 2018
Sorry, I didn't list all failing tests. Here are the rest: so this isn't purely a tmpdir issue I would consider the umask part of the test environment that the test framework should sanitize before each run, just like with environment variables. A common entry point to all test would be the best place to set it, if the Python test runner is not such a place If there are tests that should be run with a different umask, I think they should be separate tests where we explicitly override the default, so a single test run covers all cases |
Trott commented Dec 26, 2018
All of the listed failures are indeed the same tmpdir issue (which makes sense because tests generally shouldn't be creating files outside of tmpdir).
That all makes a lot of sense to me. Perhaps both things should be done. |
Trott commented Dec 26, 2018
Trott added the author ready Trott commented Dec 26, 2018
Belated welcome @i8-pi and thanks for the pull request! As you might guess, reviews can be a bit slow to happen this time of year, even for simple changes, so thanks in advance for your patience. |
bnoordhuis left a comment
bnoordhuis left a comment There was a problem hiding this comment.
Thanks @i8-pi, LGTM. CI: https://ci.nodejs.org/job/node-test-pull-request/19853/
addaleax commented Dec 31, 2018
Windows re-build: https://ci.nodejs.org/job/node-test-commit-windows-fanned/23668/ |
addaleax commented Jan 2, 2019
Landed in ee592c3 |
Some tests which create files and check file permissions assume the umask is compatible with 022, and break when set to something like 007. Explicitly set umask to 022 PR-URL: #25213 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
Some tests which create files and check file permissions assume the umask is compatible with 022, and break when set to something like 007. Explicitly set umask to 022 PR-URL: #25213 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
nodejs#25213 proposes setting umask in the Python test runner to avoid spurious test failures when running from a shell with a restrictive umask. This is a good idea, but will only fix the issue for tests run with the Python runner. Set it in `common/index.js` as well so that it fixes it even when tests are run directly with a `node` binary, bypassing the Python test runner.
nodejs#25213 proposes setting umask in the Python test runner to avoid spurious test failures when running from a shell with a restrictive umask. This is a good idea, but will only fix the issue for tests run with the Python runner. Set it in `common/index.js` as well so that it fixes it even when tests are run directly with a `node` binary, bypassing the Python test runner. PR-URL: nodejs#25229 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
Some tests which create files and check file permissions assume the umask is compatible with 022, and break when set to something like 007. Explicitly set umask to 022 PR-URL: nodejs#25213 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
#25213 proposes setting umask in the Python test runner to avoid spurious test failures when running from a shell with a restrictive umask. This is a good idea, but will only fix the issue for tests run with the Python runner. Set it in `common/index.js` as well so that it fixes it even when tests are run directly with a `node` binary, bypassing the Python test runner. PR-URL: #25229 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
Some tests which create files and check file permissions assume the umask is compatible with 022, and break when set to something like 007. Explicitly set umask to 022 PR-URL: nodejs#25213 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
nodejs#25213 proposes setting umask in the Python test runner to avoid spurious test failures when running from a shell with a restrictive umask. This is a good idea, but will only fix the issue for tests run with the Python runner. Set it in `common/index.js` as well so that it fixes it even when tests are run directly with a `node` binary, bypassing the Python test runner. PR-URL: nodejs#25229 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
Some tests which create files and check file permissions assume the umask is compatible with 022, and break when set to something like 007. Explicitly set umask to 022 PR-URL: #25213 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
Some tests which create files and check file permissions assume the umask is compatible with 022, and break when set to something like 007. Explicitly set umask to 022 PR-URL: #25213 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
Some tests which create files and check file permissions assume the umask is compatible with 022, and break when set to something like 007. Explicitly set umask to 022 PR-URL: #25213 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
5 participants
Some tests which create files and check file permissions assume the
umask is compatible with 022, and break when set to something like 007.
Explicitly set umask to 022
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesI'm seeing test errors
when running
umask 007; make -j4 test