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/parallel/test-fs-error-messages fails if tmpdir on different mount than tests #21669

Closed
drewfish opened this issue Jul 5, 2018 · 8 comments
Closed

test/parallel/test-fs-error-messages fails if tmpdir on different mount than tests #21669

drewfish opened this issue Jul 5, 2018 · 8 comments

Comments

@drewfish
Copy link
Contributor

drewfish commented Jul 5, 2018

  • Version: v10.6.0
  • Platform:
  • Subsystem: test/parallel/test-fs-error-messages

When the NODE_TEST_DIR env var is set to a temporary directory which is on a different filesystem, test/parallel/test-fs-error-messages "rename non-empty directory" subtest will fail with err.code EXDEV. However the test code is written to assume this is EPERM so the assertion fails.
@drewfish
Copy link
Contributor Author

drewfish commented Jul 5, 2018

If it's helpful I can attempt a patch for this (should be pretty easy).

@richardlau
Copy link
Member

Does #21552 address this for you?

@drewfish
Copy link
Contributor Author

drewfish commented Jul 5, 2018

Looking at the diff I think it will.

@aduh95
Copy link
Contributor

aduh95 commented Jul 18, 2018

My PR landed in b75bde3, has it resolved your issue?

@drewfish
Copy link
Contributor Author

Yep works well. I cherry-picked b75bde3 onto the v10.6.0 tag and my build & tests worked fine.

@richardlau
Copy link
Member

Fixed in v10.7.0.

@drewfish
Copy link
Contributor Author

Unfortunately I'm seeing this again, starting with v11.2.0 (v11.1.0 is OK):

AssertionError [ERR_ASSERTION]: Expect EXDEV: cross-device link not permitted, rename '/tmp/screwdriver/.tmp.14/non-existent' -> 'foo' to match /ENOENT: no such file or directory, rename '\/tmp\/screwdriver\/\.tmp\.14\/non\-existent' \-\> '.*foo'/

I think it's because of the fs.rename(nonexistentFile, 'foo', ...) call:
https://github.com/nodejs/node/blob/v11.x/test/parallel/test-fs-error-messages.js#L302

I suspect that fs.rename(nonexistentFile, path.join(tmpdir.path, 'foo'), ...) might work better. I can open a PR if that helps.

@richardlau
Copy link
Member

I suspect that fs.rename(nonexistentFile, path.join(tmpdir.path, 'foo'), ...) might work better. I can open a PR if that helps.

Please do.

@drewfish drewfish mentioned this issue Nov 28, 2018
Closed
3 tasks
Trott pushed a commit to Trott/io.js that referenced this issue Nov 28, 2018
@drewfish @Trott
test: validate fs.rename() when NODE_TEST_DIR on separate mount
8344f24 
When testing fs.rename() of an non-existent file, use a destination path
which is in the same directory. Otherwise we might trigger an `EXDEV`
error if NODE_TEST_DIR is a separate device than the current working
directory.

Fixes: nodejs#21669

PR-URL: nodejs#24707
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this issue Nov 29, 2018
@drewfish @targos
test: validate fs.rename() when NODE_TEST_DIR on separate mount
053f3d6 
When testing fs.rename() of an non-existent file, use a destination path
which is in the same directory. Otherwise we might trigger an `EXDEV`
error if NODE_TEST_DIR is a separate device than the current working
directory.

Fixes: #21669

PR-URL: #24707
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
@drewfish @refack
test: validate fs.rename() when NODE_TEST_DIR on separate mount
89ec30e 
When testing fs.rename() of an non-existent file, use a destination path
which is in the same directory. Otherwise we might trigger an `EXDEV`
error if NODE_TEST_DIR is a separate device than the current working
directory.

Fixes: nodejs#21669

PR-URL: nodejs#24707
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BethGriggs pushed a commit that referenced this issue Feb 12, 2019
@drewfish @BethGriggs
test: validate fs.rename() when NODE_TEST_DIR on separate mount
85e9487 
When testing fs.rename() of an non-existent file, use a destination path
which is in the same directory. Otherwise we might trigger an `EXDEV`
error if NODE_TEST_DIR is a separate device than the current working
directory.

Fixes: #21669

PR-URL: #24707
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
rvagg pushed a commit that referenced this issue Feb 28, 2019
@drewfish @rvagg
test: validate fs.rename() when NODE_TEST_DIR on separate mount
6a80380 
When testing fs.rename() of an non-existent file, use a destination path
which is in the same directory. Otherwise we might trigger an `EXDEV`
error if NODE_TEST_DIR is a separate device than the current working
directory.

Fixes: #21669

PR-URL: #24707
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test: validate fs.rename() when NODE_TEST_DIR on separate mount
3 participants
@drewfish @richardlau @aduh95