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.

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

test: add test for fsPromises.lchmod #20584

Closed
wants to merge 3 commits into from

Conversation

shisama
Copy link
Contributor

@shisama shisama commented May 7, 2018

To increase test coverage for fs/promises,
add test for fsPromises.lchmod.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label May 7, 2018
@ChALkeR ChALkeR added experimental Issues and PRs related to experimental features. fs Issues and PRs related to the fs subsystem / file system. labels May 8, 2018
@shisama shisama force-pushed the test-fs-promises-lchmod branch 2 times, most recently from 70ef9c2 to 1d85905 Compare May 9, 2018 14:20
@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 18, 2018
@BridgeAR
Copy link
Member

@BridgeAR BridgeAR removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 18, 2018
@jasnell
Copy link
Member

jasnell commented May 23, 2018

CI completely failed on this one.

await symlink(newPath, newLink);
await lchmod(newLink, newMode);
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed it. Thank you for your review.

@shisama shisama force-pushed the test-fs-promises-lchmod branch from 2f802a0 to 4437388 Compare May 24, 2018 16:37
@richardlau
Copy link
Member

@shisama shisama force-pushed the test-fs-promises-lchmod branch from 4437388 to ea2a7c7 Compare May 25, 2018 12:25
@BridgeAR
Copy link
Member

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 29, 2018
@BridgeAR BridgeAR requested a review from joyeecheung May 29, 2018 19:26
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 30, 2018
To increase test coverage for fs.promises by adding a test for
lchmod.

PR-URL: nodejs#20584
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@BridgeAR
Copy link
Member

Landed in 6558dcb 🎉

@BridgeAR BridgeAR closed this May 30, 2018
addaleax pushed a commit that referenced this pull request May 31, 2018
To increase test coverage for fs.promises by adding a test for
lchmod.

PR-URL: #20584
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jun 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. experimental Issues and PRs related to experimental features. fs Issues and PRs related to the fs subsystem / file system. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants