Skip to content

test: fix flaky test for symlinks#3418

Closed
Trott wants to merge 3 commits intonodejs:masterfrom
Trott:win10-fs-symlink
Closed

test: fix flaky test for symlinks#3418
Trott wants to merge 3 commits intonodejs:masterfrom
Trott:win10-fs-symlink

Conversation

@Trott
Copy link
Member

@Trott Trott commented Oct 17, 2015 

If the symlink portion of the test was being skipped due to a
combination of OS support and user privileges, then an assertion would
always fail. This fixes that problem, improves assertion error reporting
and renames the test to make it clear that it is a test for links and
not just symlinks.

Fixes: #3311

@Trott Trott added the test Issues and PRs related to the tests. label Oct 17, 2015
@Trott Trott mentioned this pull request Oct 17, 2015
Closed
@Trott
Copy link
Member Author

Trott commented Oct 17, 2015

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

@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Oct 17, 2015
@jbergstroem
Copy link
Member

jbergstroem commented Oct 18, 2015

how about returning SKIP for this case?

@Trott
Copy link
Member Author

Trott commented Oct 18, 2015

It only skips the fs.symlink() test, but it still performs the fs.link() test. So if we wanted to do that, we would have to split this test into two different test files. Which, actually, makes a lot of sense...

@Trott
test: fix flaky test for symlinks
193c6b2 
If the symlink portion of the test was being skipped due to a
combination of OS support and user privileges, then an assertion would
always fail. This fixes that problem, improves assertion error reporting
and renames the test to make it clear that it is a test for links and
not just symlinks.

Fixes: nodejs#3311
@Trott
Copy link
Member Author

Trott commented Oct 18, 2015

@jbergstroem Test is now split into two files, one for fs.link() and one for fs.symlink(). When the symlink test is skipped it now outputs a TAP skip. PTAL

@Trott
Copy link
Member Author

Trott commented Oct 18, 2015

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

@jbergstroem
Copy link
Member

jbergstroem commented Oct 20, 2015

LGTM

Trott added a commit that referenced this pull request Oct 20, 2015
@Trott
test: fix flaky test for symlinks
1c57845 
If the symlink portion of the test was being skipped due to a
combination of OS support and user privileges, then an assertion would
always fail. This fixes that problem, improves assertion error reporting
and splits the test to make it clear that it is a test for links and
symlinks.

Fixes: #3311
PR-URL: #3418
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@Trott
Copy link
Member Author

Trott commented Oct 20, 2015

Landed in 1c57845

@Trott Trott closed this Oct 20, 2015
Trott added a commit that referenced this pull request Oct 21, 2015
@Trott @rvagg
test: fix flaky test for symlinks
f78c8e7 
If the symlink portion of the test was being skipped due to a
combination of OS support and user privileges, then an assertion would
always fail. This fixes that problem, improves assertion error reporting
and splits the test to make it clear that it is a test for links and
symlinks.

Fixes: #3311
PR-URL: #3418
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@rvagg rvagg mentioned this pull request Oct 21, 2015
Merged
Trott added a commit that referenced this pull request Oct 26, 2015
@Trott @jasnell
test: fix flaky test for symlinks
d62d158 
If the symlink portion of the test was being skipped due to a
combination of OS support and user privileges, then an assertion would
always fail. This fixes that problem, improves assertion error reporting
and splits the test to make it clear that it is a test for links and
symlinks.

Fixes: #3311
PR-URL: #3418
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@jasnell
Copy link
Member

jasnell commented Oct 26, 2015

landed in v4.x-staging in d62d158

@rvagg rvagg mentioned this pull request Oct 28, 2015
Closed
Trott added a commit that referenced this pull request Oct 29, 2015
@Trott @jasnell
test: fix flaky test for symlinks
155c778 
If the symlink portion of the test was being skipped due to a
combination of OS support and user privileges, then an assertion would
always fail. This fixes that problem, improves assertion error reporting
and splits the test to make it clear that it is a test for links and
symlinks.

Fixes: #3311
PR-URL: #3418
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@snyk-bot snyk-bot mentioned this pull request Aug 10, 2020
Closed
@Trott Trott deleted the win10-fs-symlink branch January 13, 2022 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

fs Issues and PRs related to the fs subsystem / file system. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Investigate flaky test test-fs-symlink

4 participants

@Trott @jbergstroem @jasnell @mscdex