-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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: set umask for tests #25229
test: set umask for tests #25229
Conversation
Before: $ umask 077 && ./node test/parallel/test-fs-mkdir-mode-mask.js
assert.js:86
throw new AssertionError(obj);
^
AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
384 !== 420
at test (/Users/trott/io.js/test/parallel/test-fs-mkdir-mode-mask.js:32:12)
at Object.<anonymous> (/Users/trott/io.js/test/parallel/test-fs-mkdir-mode-mask.js:44:1)
at Module._compile (internal/modules/cjs/loader.js:718:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:729:10)
at Module.load (internal/modules/cjs/loader.js:617:32)
at tryModuleLoad (internal/modules/cjs/loader.js:560:12)
at Function.Module._load (internal/modules/cjs/loader.js:552:3)
at Function.Module.runMain (internal/modules/cjs/loader.js:771:12)
at executeUserCode (internal/bootstrap/node.js:389:15)
at startExecution (internal/bootstrap/node.js:328:3)
$ After: $ umask 077 && ./node test/parallel/test-fs-mkdir-mode-mask.js
$ |
Lot of failures. |
Yes, I added the |
It's probably as simple as checking for existence first and doing nothing if it's not there (and assuming a parent process took care of it) but I want to check carefully to make sure there's nothing that requires anything more than that. |
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.
Landed in a53518d |
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 <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
#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 <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
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 <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
#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 rundirectly with a
node
binary, bypassing the Python test runner.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes