Skip to content
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: don't use cwd for relative path #4477

Merged

Conversation

jbergstroem
Copy link
Member

With the introduction of temporary paths in the test runner realpath tests would bail in scenarios where the temporary folder wasn't in the same directory as the source code.

/R=@Trott?

@jbergstroem jbergstroem added the test Issues and PRs related to the tests. label Dec 30, 2015
@jbergstroem
Copy link
Member Author

CI: https://ci.nodejs.org/job/node-test-commit/1571/

Will likely get a fail on raspberry pi because the path to common pipe will be too long. A fix for that would either be making it flaky for a while ("fix") or changing the temporary path which we explore through #4476.

@jbergstroem
Copy link
Member Author

@Trott mentions that this actually doesn't properly address relative tests. Will address shortly.

@Trott
Copy link
Member

Trott commented Dec 30, 2015

LGTM if CI is happy (or happier)

@jbergstroem
Copy link
Member Author

@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Dec 30, 2015
@Trott
Copy link
Member

Trott commented Dec 30, 2015

Bonus benefit is that you can run this with CWD of test/parallel where the previous version fails in that case.

With the introduction of temporary paths in the test runner
realpath tests would bail in scenarios where the temporary folder
wasn't in the same directory as the source code.

PR-URL: nodejs#4477
Reviewed-By: Rich Trott <[email protected]>
@jbergstroem jbergstroem force-pushed the fix/realpath-relative-baseurl branch from ccc802d to 6efa031 Compare December 30, 2015 06:51
@jbergstroem jbergstroem merged commit 6efa031 into nodejs:master Dec 30, 2015
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Jan 6, 2016
With the introduction of temporary paths in the test runner
realpath tests would bail in scenarios where the temporary folder
wasn't in the same directory as the source code.

PR-URL: nodejs#4477
Reviewed-By: Rich Trott <[email protected]>
@richardlau
Copy link
Member

@thealphanerd This will need land on v4.x-staging since #3325 has landed and made the temp dir customizable via NODE_TEST_DIR while removing NODE_COMMON_PIPE.

jasnell pushed a commit that referenced this pull request Jan 15, 2016
With the introduction of temporary paths in the test runner
realpath tests would bail in scenarios where the temporary folder
wasn't in the same directory as the source code.

PR-URL: #4477
Reviewed-By: Rich Trott <[email protected]>
@jasnell
Copy link
Member

jasnell commented Jan 15, 2016

Landed in v4.x-staging in bb2e2d0

MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
With the introduction of temporary paths in the test runner
realpath tests would bail in scenarios where the temporary folder
wasn't in the same directory as the source code.

PR-URL: #4477
Reviewed-By: Rich Trott <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
With the introduction of temporary paths in the test runner
realpath tests would bail in scenarios where the temporary folder
wasn't in the same directory as the source code.

PR-URL: nodejs#4477
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants