test: use tmp directory in chdir test #2589
Conversation
|
LGTM if CI is happy |
|
cc @nodejs/build this moves another test to |
|
I think the filename of this test should be renamed to |
This patch - makes chdir test to use the tmp directory - moves the test to parallel - renames the file to test-process-chdir as chdir is in process module
thefourtheye
force-pushed
the
improve-test-chdir
branch
from
24b70cf
to
263bdcb
Compare
|
@mscdex Good call. Thanks :-) Renamed the file. |
|
New CI Run: https://ci.nodejs.org/job/node-test-commit/581/ |
| 'weird \uc3a4\uc3ab\uc3af characters \u00e1\u00e2\u00e3'); | ||
|
|
||
| // Make sure that the tmp directory is clean | ||
| common.refreshTmpDir(); |
There was a problem hiding this comment.
I think this is effectively unsafe after using common.tmpDir?
Isn't it possible that your tmpDir is different then?
There was a problem hiding this comment.
@Fishrock123 refresh just removes the directory and creates it
exports.refreshTmpDir = function() {
rimrafSync(exports.tmpDir);
fs.mkdirSync(exports.tmpDir);
};The name is actually decided when we load the module itself,
if (process.env.TEST_THREAD_ID) {
// Distribute ports in parallel tests
if (!process.env.NODE_COMMON_PORT)
exports.PORT += +process.env.TEST_THREAD_ID * 100;
exports.tmpDirName += '.' + process.env.TEST_THREAD_ID;
}
exports.tmpDir = path.join(exports.testDir, exports.tmpDirName);Since we are just resolving the path before that, we are safe here.
|
Test is passing in all the environments. @Fishrock123 @mscdex LGTY? |
|
LGTM if CI is happy |
This patch - makes chdir test to use the tmp directory - moves the test to parallel - renames the file to test-process-chdir as chdir is in process module PR-URL: #2589 Reviewed-By: Brendan Ashworth <[email protected]> Reviewed-By: Rich Trott <[email protected]>
|
Thanks for the review people. Landed in 9aa6a43 |
This patch - makes chdir test to use the tmp directory - moves the test to parallel - renames the file to test-process-chdir as chdir is in process module PR-URL: nodejs#2589 Reviewed-By: Brendan Ashworth <[email protected]> Reviewed-By: Rich Trott <[email protected]>
This patch makes chdir test to use the tmp directory and moves the test
to parallel.
CI Run: https://jenkins-iojs.nodesource.com/job/node-test-pull-request/198/