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.

Sign up for GitHub

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

Jump to bottom

test: add cwd ENOENT known issue test #12343

Merged
merged 1 commit into from
Apr 18, 2017
Merged

test: add cwd ENOENT known issue test #12343

merged 1 commit into from
Apr 18, 2017

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Apr 11, 2017

If the current working directory is removed, Node cannot start normally because the module system calls uv_cwd().

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 11, 2017
@cjihrig cjihrig mentioned this pull request Apr 11, 2017
Closed
4 tasks
@mscdex mscdex added the process Issues and PRs related to the process subsystem. label Apr 11, 2017
richardlau
richardlau reviewed Apr 11, 2017
View reviewed changes
test/known_issues/test-cwd-enoent-file.js
common.refreshTmpDir();
const dir = fs.mkdtempSync(common.tmpDir + '/');
process.chdir(dir);
fs.rmdirSync(dir);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this work on Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will probably have to skip on Windows like the other cwd enoent test does.

Trott
Trott approved these changes Apr 11, 2017
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if CI passes

@cjihrig
Copy link
Contributor Author

cjihrig commented Apr 18, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/7486/

@cjihrig
test: add cwd ENOENT known issue test
4fc1199 
If the current working directory is removed, Node cannot
start normally because the module system calls uv_cwd().

Refs: nodejs#12022
PR-URL: nodejs#12343
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig cjihrig merged commit 4fc1199 into nodejs:master Apr 18, 2017
@cjihrig cjihrig deleted the cwd branch April 18, 2017 17:48
@evanlucas
Copy link
Contributor

This is failing lint on v7.x. Mind submitting a backport PR?

@cjihrig
Copy link
Contributor Author

cjihrig commented Apr 25, 2017

@evanlucas is this failing via the test runner, or failing if you run the test on its own? It's a known issue test, so it should fail.

@evanlucas
Copy link
Contributor

It is failing make lint in v7.x

Running JS linter...
./node tools/eslint/bin/eslint.js --cache --rulesdir=tools/eslint-rules \
	  benchmark lib test tools

/Volumes/code/forks/WORK-node/test/known_issues/test-cwd-enoent-file.js
  11:3  error  assert.fail() message should be third argument  assert-fail-single-argument

✖ 1 problem (1 error, 0 warnings)

make[2]: *** [jslint] Error 1
Running C++ linter...
Total errors found: 0
make[1]: *** [lint] Error 2
make: *** [test] Error 2

@cjihrig
Copy link
Contributor Author

cjihrig commented Apr 25, 2017

Ah, this probably depends on #12293 then.

@jasnell jasnell mentioned this pull request May 11, 2017
Closed
@sam-github
Copy link
Contributor

Don't need backport to 7.x, and its already in 8.x

@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
@gibfahn
Copy link
Member

gibfahn commented Jun 18, 2017

Should land with #12293 if that lands.

@gibfahn gibfahn mentioned this pull request Jun 18, 2017
Closed
4 tasks
@gibfahn gibfahn mentioned this pull request Sep 23, 2017
Closed
4 tasks
Trott pushed a commit to Trott/io.js that referenced this pull request Sep 29, 2017
@cjihrig @Trott
test: add cwd ENOENT known issue test
1f4b70b 
If the current working directory is removed, Node cannot
start normally because the module system calls uv_cwd().

Refs: nodejs#12022
PR-URL: nodejs#12343
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2017
@cjihrig @MylesBorins
test: add cwd ENOENT known issue test
0597d3f 
If the current working directory is removed, Node cannot
start normally because the module system calls uv_cwd().

Backport-PR-URL: #15479
Refs: #12022
PR-URL: #12343
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Oct 17, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
@cjihrig @MylesBorins
test: add cwd ENOENT known issue test
d8f5637 
If the current working directory is removed, Node cannot
start normally because the module system calls uv_cwd().

Backport-PR-URL: #15479
Refs: #12022
PR-URL: #12343
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 3, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau left review comments

@addaleax addaleax addaleax approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
process Issues and PRs related to the process subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.