test,fs: add fs.rm() tests for .git directories by RaisinTen · Pull Request #42410 · nodejs/node · GitHub

Git for Windows creates read-only files inside the .git directory.
fs.rm() was built in a way, to work around any EPERM error that can
happen while deleting the .git directory by turning the directory
into a writable one. This change adds a regression test for that.

Refs: isaacs/rimraf#21
Refs: #38810 (comment)
Signed-off-by: Darshan Sen raisinten@gmail.com

Git for Windows creates read-only files inside the .git directory. fs.rm() was built in a way, to work around any EPERM error that can happen while deleting the .git directory by turning the directory into a writable one. This change adds a regression test for that. Refs: isaacs/rimraf#21 Refs: nodejs#38810 (comment) Signed-off-by: Darshan Sen <raisinten@gmail.com>

LGTM once @richardlau's comment is addressed.

Signed-off-by: Darshan Sen <raisinten@gmail.com>

Git in centos7-arm64-gcc8 doesn't support the -C option, so the test was failing with:

Unknown option: -C
usage: git [--version] [--help] [-c name=value]
           [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]
           [-p|--paginate|--no-pager] [--no-replace-objects] [--bare]
           [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]
           <command> [<args>]

I have updated the test to run git init inside the directory that it tries to turn into a git directory instead, PTAL.

… dir` Signed-off-by: Darshan Sen <raisinten@gmail.com>

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

Landed in 5a138b5

Git for Windows creates read-only files inside the .git directory. fs.rm() was built in a way, to work around any EPERM error that can happen while deleting the .git directory by turning the directory into a writable one. This change adds a regression test for that. Refs: isaacs/rimraf#21 Refs: #38810 (comment) Signed-off-by: Darshan Sen <raisinten@gmail.com> PR-URL: #42410 Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Git for Windows creates read-only files inside the .git directory. fs.rm() was built in a way, to work around any EPERM error that can happen while deleting the .git directory by turning the directory into a writable one. This change adds a regression test for that. Refs: isaacs/rimraf#21 Refs: nodejs#38810 (comment) Signed-off-by: Darshan Sen <raisinten@gmail.com> PR-URL: nodejs#42410 Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Git for Windows creates read-only files inside the .git directory. fs.rm() was built in a way, to work around any EPERM error that can happen while deleting the .git directory by turning the directory into a writable one. This change adds a regression test for that. Refs: isaacs/rimraf#21 Refs: #38810 (comment) Signed-off-by: Darshan Sen <raisinten@gmail.com> PR-URL: #42410 Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Git for Windows creates read-only files inside the .git directory. fs.rm() was built in a way, to work around any EPERM error that can happen while deleting the .git directory by turning the directory into a writable one. This change adds a regression test for that. Refs: isaacs/rimraf#21 Refs: nodejs#38810 (comment) Signed-off-by: Darshan Sen <raisinten@gmail.com> PR-URL: nodejs#42410 Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Git for Windows creates read-only files inside the .git directory. fs.rm() was built in a way, to work around any EPERM error that can happen while deleting the .git directory by turning the directory into a writable one. This change adds a regression test for that. Refs: isaacs/rimraf#21 Refs: #38810 (comment) Signed-off-by: Darshan Sen <raisinten@gmail.com> PR-URL: #42410 Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Git for Windows creates read-only files inside the .git directory. fs.rm() was built in a way, to work around any EPERM error that can happen while deleting the .git directory by turning the directory into a writable one. This change adds a regression test for that. Refs: isaacs/rimraf#21 Refs: nodejs/node#38810 (comment) Signed-off-by: Darshan Sen <raisinten@gmail.com> PR-URL: nodejs/node#42410 Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Mar 20, 2022

RaisinTen mentioned this pull request Mar 20, 2022

fs: stop fixing EPERM on Windows #38810

Closed

richardlau reviewed Mar 20, 2022

View reviewed changes

Comment thread test/parallel/test-fs-rm.js Outdated

bcoe approved these changes Mar 20, 2022

View reviewed changes

test: check if git is present before running the tests

b76ef18

Signed-off-by: Darshan Sen <raisinten@gmail.com>

RaisinTen requested a review from richardlau March 21, 2022 14:05

RaisinTen added request-ci Add this label to start a Jenkins CI on a PR. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Mar 21, 2022

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 21, 2022

This comment was marked as outdated.

Sign in to view

richardlau approved these changes Mar 21, 2022

View reviewed changes

RaisinTen added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Mar 21, 2022

lpinca approved these changes Mar 21, 2022

View reviewed changes

This comment was marked as outdated.

Sign in to view

jasnell approved these changes Mar 25, 2022

View reviewed changes

test: run cd dir && git init && cd - instead of git init -C dir

a2da073

Signed-off-by: Darshan Sen <raisinten@gmail.com>

This comment was marked as outdated.

Sign in to view

RaisinTen removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Mar 26, 2022

fixup! test: run cd dir && git init && cd - instead of `git init -C…

bc950fa

… dir` Signed-off-by: Darshan Sen <raisinten@gmail.com>

This comment was marked as outdated.

Sign in to view

github-actions Bot mentioned this pull request Mar 27, 2022

CI Reliability 2022-03-27 nodejs/reliability#233

Open

26 tasks

RaisinTen requested review from aduh95, bcoe, jasnell and lpinca March 27, 2022 07:09

RaisinTen requested a review from richardlau March 27, 2022 07:09

lpinca approved these changes Mar 27, 2022

View reviewed changes

github-actions Bot mentioned this pull request Mar 28, 2022

CI Reliability 2022-03-28 nodejs/reliability#234

Open

24 tasks

RaisinTen added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Mar 28, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 28, 2022

nodejs-github-bot merged commit 5a138b5 into nodejs:master Mar 28, 2022

RaisinTen deleted the test,fs/add-fs.rm-tests-for-git-directories branch March 28, 2022 03:58

This was referenced Apr 5, 2022

V17.8.1 proposal #42612

Closed

v17.9.0 proposal #42613

Merged

juanarbol mentioned this pull request May 31, 2022

2022-05-XX, Version 16.16.0 'Gallium' (LTS) #43268

Closed

targos mentioned this pull request Aug 3, 2022

v16.17.0 release proposal #44098

Merged

test,fs: add fs.rm() tests for .git directories by RaisinTen · Pull Request #42410 · nodejs/node · GitHub

Uh oh!

Uh oh!

test,fs: add fs.rm() tests for .git directories - #42410

test,fs: add fs.rm() tests for .git directories#42410
nodejs-github-bot merged 4 commits into
nodejs:masterfrom
RaisinTen:test,fs/add-fs.rm-tests-for-git-directories

RaisinTen commented Mar 20, 2022

Uh oh!

Uh oh!

bcoe left a comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

RaisinTen commented Mar 26, 2022 •
edited
Loading

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented Mar 27, 2022

Uh oh!

nodejs-github-bot commented Mar 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

test,fs: add fs.rm() tests for .git directories by RaisinTen · Pull Request #42410 · nodejs/node · GitHub

Uh oh!

Uh oh!

test,fs: add fs.rm() tests for .git directories - #42410

test,fs: add fs.rm() tests for .git directories#42410nodejs-github-bot merged 4 commits intonodejs:masternodejs/node:masterfrom RaisinTen:test,fs/add-fs.rm-tests-for-git-directoriesRaisinTen/node:test,fs/add-fs.rm-tests-for-git-directoriesCopy head branch name to clipboard

Conversation

RaisinTen commented Mar 20, 2022

Uh oh!

Uh oh!

bcoe left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

RaisinTen commented Mar 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented Mar 27, 2022

Uh oh!

nodejs-github-bot commented Mar 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

test,fs: add fs.rm() tests for .git directories#42410
nodejs-github-bot merged 4 commits into
nodejs:masterfrom
RaisinTen:test,fs/add-fs.rm-tests-for-git-directories

RaisinTen commented Mar 26, 2022 •
edited
Loading