FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Skip the long path tests when not on Windows by rsp · Pull Request #4116 · nodejs/node · GitHub

/ node Public

Skip the long path tests when not on Windows - #4116

Closed
rsp wants to merge 1 commit into
nodejs:masterfrom
rsp:fix-long-path-tests-4
ConversationCommits (1)ChecksFiles changed
Closed

Skip the long path tests when not on Windows#4116
rsp wants to merge 1 commit into
nodejs:masterfrom
rsp:fix-long-path-tests-4

Conversation

rsp commented Dec 2, 2015

Copy link
Copy Markdown
Contributor

This is another attempt to fix the tests failing on Ubuntu (see issue #2255)
This time by skipping the tests when not on Windows (as advised by @bnoordhuis)

Other PRs that solve the same issue:

See also comments to PRs #3925 and #3929 for more context.

rsp mentioned this pull request Dec 2, 2015
Closed
bnoordhuis reviewed Dec 2, 2015
View reviewed changes
Comment thread test/parallel/test-fs-long-path.js Outdated

bnoordhuis Dec 2, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you use the TAP-compatible approach we use e.g. here?

(I forgot about common.isWindows, it's fairly new.)

rsp Dec 2, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I changed the test skipping in 5502b9b - is that what you mean?

bnoordhuis commented Dec 2, 2015

Copy link
Copy Markdown
Member

LGTM, thanks. Can you squash and write a commit log in the style that CONTRIBUTING.md wants?

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

mscdex added fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform. test Issues and PRs related to the tests. labels Dec 2, 2015
test: Skip long path tests on non-Windows
e07b145 
If not running on Windows it skips the long path tests in:
* test-fs-long-path.js
* test-require-long-path.js

It fixes issue #2255 that is currently: wontfix

For context see comments to PRs:
#3925
#3929
#4116

rsp commented Dec 2, 2015

Copy link
Copy Markdown
Contributor Author

@bnoordhuis I rebased it on fresh master, squashed into e07b145 with new message and force pushed here. I can't access the CI link ("rsp is missing the Overall/Read permission") so I don't know if it tests the forced update automatically or not. Thanks for your help.

jasnell commented Dec 3, 2015

Copy link
Copy Markdown
Member

LGTM

JungMinu commented Dec 5, 2015

Copy link
Copy Markdown
Member

As @rsp updated, new CI : https://ci.nodejs.org/job/node-test-pull-request/932/

JungMinu commented Dec 5, 2015

Copy link
Copy Markdown
Member

LGTM if new CI is happy :)

bnoordhuis pushed a commit that referenced this pull request Dec 7, 2015
test: skip long path tests on non-Windows
19e06d7 
If not running on Windows it skips the long path tests in:

* test-fs-long-path.js
* test-require-long-path.js

Fixes: #2255
PR-URL: #4116
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>

bnoordhuis commented Dec 7, 2015

Copy link
Copy Markdown
Member

Thanks, landed in 19e06d7.

bnoordhuis closed this Dec 7, 2015
rvagg pushed a commit that referenced this pull request Dec 8, 2015
test: skip long path tests on non-Windows
0c2a0dc 
If not running on Windows it skips the long path tests in:

* test-fs-long-path.js
* test-require-long-path.js

Fixes: #2255
PR-URL: #4116
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
This was referenced Dec 8, 2015
Closed
Merged
rsp mentioned this pull request Dec 8, 2015
Closed

rsp commented Dec 8, 2015

Copy link
Copy Markdown
Contributor Author

@jasnell @JungMinu @bnoordhuis @rvagg Thanks. My pleasure. I think the wontfix label can be removed from issue #2255 now.

rvagg mentioned this pull request Dec 17, 2015
Closed
MylesBorins pushed a commit that referenced this pull request Dec 29, 2015
test: skip long path tests on non-Windows
2d18d54 
If not running on Windows it skips the long path tests in:

* test-fs-long-path.js
* test-require-long-path.js

Fixes: #2255
PR-URL: #4116
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
test: skip long path tests on non-Windows
90498e2 
If not running on Windows it skips the long path tests in:

* test-fs-long-path.js
* test-require-long-path.js

Fixes: #2255
PR-URL: #4116
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
MylesBorins mentioned this pull request Jan 19, 2016
Merged
rsp mentioned this pull request Mar 22, 2016
Closed
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
test: skip long path tests on non-Windows
704fecb 
If not running on Windows it skips the long path tests in:

* test-fs-long-path.js
* test-require-long-path.js

Fixes: nodejs#2255
PR-URL: nodejs#4116
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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. windows Issues and PRs related to the Windows platform.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL