test: make temp path customizable #3325
Conversation
|
CI (without passing |
test/common.js
Outdated
|
I'm still catching up but did |
|
AFAIK it was never in tests, just passed to |
|
I'll leave it to someone more in tune with the entirety of the build process to |
|
Prior to removal, if |
|
@jbergstroem ... what's the status on this? Looks like it needs to be rebased at a minimum |
|
@jasnell waiting for feedback/review. I'll rebase. |
cc1749a to
3f1add0
Compare
|
@Trott to answer your question regarding "enabling" parallel builds -- that would be something we'd most likely be changing in the jenkins setup. |
|
LGTM Can you ensure that To run parallel tests on Windows, the pipe name should also depend on the thread number. What about adding something like lines 64 and 66 after 139? |
|
@joaocgreis does the pipe really have to depend on a thread number since it already lives in its own thread folder ( Regarding creating the directory, that's already part of |
|
@jbergstroem the pipe is defined differently in windows, it is a global name, not a file in the test folder (as created in line 139). The |
3f1add0 to
0740b42
Compare
|
Rebased and fixed an issue with two tests that assumed a specific folder structure. CI (which again shouldn't branch into this): https://ci.nodejs.org/job/node-test-commit/1324/ |
|
Oh, and @joaocgreis -- I now create the dir through refreshTmp. |
test/common.js
Outdated
There was a problem hiding this comment.
existsSync() is deprecated so maybe use one of the suggested alternatives?
|
ping! |
test/common.js
Outdated
|
@jbergstroem wouldn't checking the NODE_TEST_DIR directly in the test runner be better? |
00d9d98 to
f96e027
Compare
|
@joaocgreis checking in the runner now. Also, created an option called |
|
CI (should be unaffected): https://ci.nodejs.org/job/node-test-commit/1560/ |
In CI we previously passed `NODE_COMMON_PIPE` to the test runner to avoid long filenames. Add an option to the test runner that allows the user to change the temporary directory instead. This also allows us to run test suites in parallel since `NODE_COMMON_PIPE` otherwise would have been used from multiple tests at the same time. PR-URL: #3325 Reviewed-By: Joao Reis <reis@janeasystems.com>
A few tests assumed that temp dirs always lived in the same parent folder as fixtures. Make these use `common.tmpDir` instead. PR-URL: #3325 Reviewed-By: Joao Reis <reis@janeasystems.com>
|
Landed in v4.x-staging in d33279d and b3079ef |
In CI we previously passed `NODE_COMMON_PIPE` to the test runner to avoid long filenames. Add an option to the test runner that allows the user to change the temporary directory instead. This also allows us to run test suites in parallel since `NODE_COMMON_PIPE` otherwise would have been used from multiple tests at the same time. PR-URL: #3325 Reviewed-By: Joao Reis <reis@janeasystems.com>
A few tests assumed that temp dirs always lived in the same parent folder as fixtures. Make these use `common.tmpDir` instead. PR-URL: #3325 Reviewed-By: Joao Reis <reis@janeasystems.com>
In CI we previously passed `NODE_COMMON_PIPE` to the test runner to avoid long filenames. Add an option to the test runner that allows the user to change the temporary directory instead. This also allows us to run test suites in parallel since `NODE_COMMON_PIPE` otherwise would have been used from multiple tests at the same time. PR-URL: nodejs#3325 Reviewed-By: Joao Reis <reis@janeasystems.com>
A few tests assumed that temp dirs always lived in the same parent folder as fixtures. Make these use `common.tmpDir` instead. PR-URL: nodejs#3325 Reviewed-By: Joao Reis <reis@janeasystems.com>
In CI we previously passed
NODE_COMMON_PIPEto the test runner to avoid long filenames on Windows. Avoid long paths by changing the temporary directory instead. This also allows us to run test suites in parallel sinceNODE_COMMON_PIPEotherwise would have been used from multiple tests./R=@nodejs/build, @Trott ?