Skip to content

Conversation

@lundibundi
Copy link
Member

Fixes: #21501

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes (waiting for CI)
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@targos could you please try this on the setup mentioned in the issue if possible?

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Sep 26, 2018
@lundibundilundibundi added the needs-ci PRs that need a full CI run. label Sep 26, 2018
@Trott
Copy link
Member

Can you confirm that the test still works with this change if the host is disconnected from the network?

@Trott
Copy link
Member

(Actually, if you could confirm that it works in its current form on the master branch when disconnected from the network, that would be helpful too!)

@lundibundi
Copy link
MemberAuthor

lundibundi commented Sep 29, 2018

@Trott, sorry for the delay. I have run this test with/without network successfully both on master and on this branch.

Edit: I ran them on Windows 10 (10.0.17134 Build 17134), just to clarify.

@lundibundi
Copy link
MemberAuthor

@Trott
Copy link
Member

This is a Windows-only test, I believe, so... @nodejs/platform-windows Can one or two folks review? @nodejs/testing too...

@addaleaxaddaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Sep 30, 2018
@addaleax
Copy link
Member

Copy link
Member

@jdaltonjdalton left a comment

Choose a reason for hiding this comment

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

Neat issue and workaround.

@lundibundi
Copy link
MemberAuthor

@danbev
Copy link
Contributor

Landed in 1be804d.

@danbevdanbev closed this Oct 1, 2018
danbev pushed a commit that referenced this pull request Oct 1, 2018
PR-URL: #23101Fixes: #21501 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: John-David Dalton <[email protected]>
@lundibundilundibundi deleted the fix-localhost-readfilesync branch October 1, 2018 09:20
targos pushed a commit that referenced this pull request Oct 1, 2018
PR-URL: #23101Fixes: #21501 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: John-David Dalton <[email protected]>
targos pushed a commit that referenced this pull request Oct 3, 2018
PR-URL: #23101Fixes: #21501 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: John-David Dalton <[email protected]>
@targostargos mentioned this pull request Oct 7, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test-fs-readfilesync-enoent failing locally with unknown error

6 participants

@lundibundi@nodejs-github-bot@Trott@addaleax@danbev@jdalton