[PATCH] tests: remove leftover untracked files

[PATCH] tests: remove leftover untracked files

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

Remove untracked files that are unwanted after they are done being used.

While the set of cases in this patch is certainly far from
comprehensive, it was motivated by some work to see what the fallout
would be if we were to make the removal of untracked files as a side
effect of other commands into an error.  Some cases were a bit more
involved than the testcase changes included in this patch, but the ones
included here represent the simple cases.  While this patch is not that
important since we are not changing the behavior of those other commands
into an error in the near term, I thought these changes were useful
anyway as an explicit documentation of the intent that these untracked
files are no longer useful.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
    tests: remove leftover untracked files
    
    Remove untracked files that are unwanted after they are done being used.
    
    While the set of cases in this patch is certainly far from
    comprehensive, it was motivated by some work to see what the fallout
    would be if we were to make the removal of untracked files as a side
    effect of other commands into an error. Some cases were a bit more
    involved than the testcase changes included in this patch, but the ones
    included here represent the simple cases. While this patch is not that
    important since we are not changing the behavior of those other commands
    into an error in the near term, I thought these changes were useful
    anyway as an explicit documentation of the intent that these untracked
    files are no longer useful.
    
    Signed-off-by: Elijah Newren newren@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1088%2Fnewren%2Fremove-leftover-test-crud-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1088/newren/remove-leftover-test-crud-v1
Pull-Request: https://github.com/git/git/pull/1088

 t/t0090-cache-tree.sh                    | 1 +
 t/t2021-checkout-overwrite.sh            | 1 +
 t/t3404-rebase-interactive.sh            | 1 +
 t/t3435-rebase-gpg-sign.sh               | 1 +
 t/t3510-cherry-pick-sequence.sh          | 1 +
 t/t5304-prune.sh                         | 1 +
 t/t6415-merge-dir-to-symlink.sh          | 6 ++++--
 t/t6424-merge-unrelated-index-changes.sh | 1 +
 t/t6430-merge-recursive.sh               | 4 +++-
 t/t6436-merge-overwrite.sh               | 3 ++-
 t/t7201-co.sh                            | 1 +
 t/t7600-merge.sh                         | 1 +
 12 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 9bf66c9e68d..90675726484 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -195,6 +195,7 @@ test_expect_success 'reset --hard gives cache-tree' '
 
 test_expect_success 'reset --hard without index gives cache-tree' '
 	rm -f .git/index &&
+	git clean -fd &&
 	git reset --hard &&
 	test_cache_tree
 '
diff --git a/t/t2021-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh
index 70d69263e68..660132ff8d5 100755
--- a/t/t2021-checkout-overwrite.sh
+++ b/t/t2021-checkout-overwrite.sh
@@ -48,6 +48,7 @@ test_expect_success 'checkout commit with dir must not remove untracked a/b' '
 
 test_expect_success SYMLINKS 'the symlink remained' '
 
+	test_when_finished "rm a/b" &&
 	test -h a/b
 '
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index d877872e8f4..972ce026bbc 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -297,6 +297,7 @@ test_expect_success 'abort with error when new base cannot be checked out' '
 		output &&
 	test_i18ngrep "file1" output &&
 	test_path_is_missing .git/rebase-merge &&
+	rm file1 &&
 	git reset --hard HEAD^
 '
 
diff --git a/t/t3435-rebase-gpg-sign.sh b/t/t3435-rebase-gpg-sign.sh
index ec107668580..5f8ba2c7399 100755
--- a/t/t3435-rebase-gpg-sign.sh
+++ b/t/t3435-rebase-gpg-sign.sh
@@ -65,6 +65,7 @@ test_rebase_gpg_sign ! true  -i --gpg-sign --no-gpg-sign
 test_rebase_gpg_sign   false -i --no-gpg-sign --gpg-sign
 
 test_expect_failure 'rebase -p --no-gpg-sign override commit.gpgsign' '
+	test_when_finished "git clean -f" &&
 	git reset --hard merged &&
 	git config commit.gpgsign true &&
 	git rebase -p --no-gpg-sign --onto=one fork-point main &&
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 49010aa9469..3b0fa66c33d 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -238,6 +238,7 @@ test_expect_success 'allow skipping commit but not abort for a new history' '
 '
 
 test_expect_success 'allow skipping stopped cherry-pick because of untracked file modifications' '
+	test_when_finished "rm unrelated" &&
 	pristine_detach initial &&
 	git rm --cached unrelated &&
 	git commit -m "untrack unrelated" &&
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 7cabb85ca6e..8ae314af585 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -291,6 +291,7 @@ test_expect_success 'prune: handle HEAD reflog in multiple worktrees' '
 		cat ../expected >blob &&
 		git add blob &&
 		git commit -m "second commit in third" &&
+		git clean -f && # Remove untracked left behind by deleting index
 		git reset --hard HEAD^
 	) &&
 	git prune --expire=now &&
diff --git a/t/t6415-merge-dir-to-symlink.sh b/t/t6415-merge-dir-to-symlink.sh
index 2ce104aca7c..2655e295f5a 100755
--- a/t/t6415-merge-dir-to-symlink.sh
+++ b/t/t6415-merge-dir-to-symlink.sh
@@ -25,7 +25,8 @@ test_expect_success 'checkout does not clobber untracked symlink' '
 	git reset --hard main &&
 	git rm --cached a/b &&
 	git commit -m "untracked symlink remains" &&
-	test_must_fail git checkout start^0
+	test_must_fail git checkout start^0 &&
+	git clean -fd    # Do not leave the untracked symlink in the way
 '
 
 test_expect_success 'a/b-2/c/d is kept when clobbering symlink b' '
@@ -34,7 +35,8 @@ test_expect_success 'a/b-2/c/d is kept when clobbering symlink b' '
 	git rm --cached a/b &&
 	git commit -m "untracked symlink remains" &&
 	git checkout -f start^0 &&
-	test_path_is_file a/b-2/c/d
+	test_path_is_file a/b-2/c/d &&
+	git clean -fd    # Do not leave the untracked symlink in the way
 '
 
 test_expect_success 'checkout should not have deleted a/b-2/c/d' '
diff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh
index 5e3779ebc93..89dd544f388 100755
--- a/t/t6424-merge-unrelated-index-changes.sh
+++ b/t/t6424-merge-unrelated-index-changes.sh
@@ -132,6 +132,7 @@ test_expect_success 'merge-recursive, when index==head but head!=HEAD' '
 
 	# Make index match B
 	git diff C B -- | git apply --cached &&
+	test_when_finished "git clean -fd" &&  # Do not leave untracked around
 	# Merge B & F, with B as "head"
 	git merge-recursive A -- B F > out &&
 	test_i18ngrep "Already up to date" out
diff --git a/t/t6430-merge-recursive.sh b/t/t6430-merge-recursive.sh
index ffcc01fe65d..a0efe7cb6db 100755
--- a/t/t6430-merge-recursive.sh
+++ b/t/t6430-merge-recursive.sh
@@ -718,7 +718,9 @@ test_expect_success 'merge-recursive remembers the names of all base trees' '
 	# merge-recursive prints in reverse order, but we do not care
 	sort <trees >expect &&
 	sed -n "s/^virtual //p" out | sort >actual &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+
+	git clean -fd
 '
 
 test_expect_success 'merge-recursive internal merge resolves to the sameness' '
diff --git a/t/t6436-merge-overwrite.sh b/t/t6436-merge-overwrite.sh
index 84b4aacf496..c0b7bd7c3fe 100755
--- a/t/t6436-merge-overwrite.sh
+++ b/t/t6436-merge-overwrite.sh
@@ -68,7 +68,8 @@ test_expect_success 'will not overwrite removed file' '
 	git commit -m "rm c1.c" &&
 	cp important c1.c &&
 	test_must_fail git merge c1a &&
-	test_cmp important c1.c
+	test_cmp important c1.c &&
+	rm c1.c  # Do not leave untracked file in way of future tests
 '
 
 test_expect_success 'will not overwrite re-added file' '
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 7f6e23a4bb9..b7ba1c3268e 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -585,6 +585,7 @@ test_expect_success 'checkout --conflict=diff3' '
 '
 
 test_expect_success 'failing checkout -b should not break working tree' '
+	git clean -fd &&  # Remove untracked files in the way
 	git reset --hard main &&
 	git symbolic-ref HEAD refs/heads/main &&
 	test_must_fail git checkout -b renamer side^ &&
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 2ef39d3088e..c773e30b3fa 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -717,6 +717,7 @@ test_expect_success 'failed fast-forward merge with --autostash' '
 	git reset --hard c0 &&
 	git merge-file file file.orig file.5 &&
 	cp file.5 other &&
+	test_when_finished "rm other" &&
 	test_must_fail git merge --autostash c1 2>err &&
 	test_i18ngrep "Applied autostash." err &&
 	test_cmp file.5 file

base-commit: e0a2f5cbc585657e757385ad918f167f519cfb96
-- 
gitgitgadget

Re: [PATCH] tests: remove leftover untracked files

[Johannes Schindelin]

Hi Elijah,

On Wed, 8 Sep 2021, Elijah Newren via GitGitGadget wrote:

> From: Elijah Newren <newren@gmail.com>
>
> Remove untracked files that are unwanted after they are done being used.
>
> While the set of cases in this patch is certainly far from
> comprehensive, it was motivated by some work to see what the fallout
> would be if we were to make the removal of untracked files as a side
> effect of other commands into an error.  Some cases were a bit more
> involved than the testcase changes included in this patch, but the ones
> included here represent the simple cases.  While this patch is not that
> important since we are not changing the behavior of those other commands
> into an error in the near term, I thought these changes were useful
> anyway as an explicit documentation of the intent that these untracked
> files are no longer useful.

While I find it a bit distracting when things are getting fixed that are
not exactly broken (there is enough that is broken and needs to be
fixed!), at least this patch is not disruptive and since you already did
the work, I am in favor of integrating the patch.

Thanks,
Dscho

Re: [PATCH] tests: remove leftover untracked files

[Derrick Stolee]

On 9/8/2021 12:47 AM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> Remove untracked files that are unwanted after they are done being used.
> 
> While the set of cases in this patch is certainly far from
> comprehensive, it was motivated by some work to see what the fallout
> would be if we were to make the removal of untracked files as a side
> effect of other commands into an error.  Some cases were a bit more
> involved than the testcase changes included in this patch, but the ones
> included here represent the simple cases.  While this patch is not that
> important since we are not changing the behavior of those other commands
> into an error in the near term, I thought these changes were useful
> anyway as an explicit documentation of the intent that these untracked
> files are no longer useful.

I think your stated goal is a good one. These minor test edits are
good to fit in while they are not conflicting with any other work
in flight. As long as it is easy to fit in with other ongoing topics,
then this should be taken and merged quickly.

Thanks,
-Stolee

[PATCH v2] tests: remove leftover untracked files

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

Remove untracked files that are unwanted after they are done being used.

While the set of cases in this patch is certainly far from
comprehensive, it was motivated by some work to see what the fallout
would be if we were to make the removal of untracked files as a side
effect of other commands into an error.  Some cases were a bit more
involved than the testcase changes included in this patch, but the ones
included here represent the simple cases.  While this patch is not that
important since we are not changing the behavior of those other commands
into an error in the near term, I thought these changes were useful
anyway as an explicit documentation of the intent that these untracked
files are no longer useful.

Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Acked-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
    tests: remove leftover untracked files
    
    Changes since v1:
    
     * Added Acked-by's from Stolee and Dscho

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1088%2Fnewren%2Fremove-leftover-test-crud-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1088/newren/remove-leftover-test-crud-v2
Pull-Request: https://github.com/git/git/pull/1088

Range-diff vs v1:

 1:  85d326a52d9 ! 1:  8812157be16 tests: remove leftover untracked files
     @@ Commit message
          anyway as an explicit documentation of the intent that these untracked
          files are no longer useful.
      
     +    Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
     +    Acked-by: Derrick Stolee <stolee@gmail.com>
          Signed-off-by: Elijah Newren <newren@gmail.com>
      
       ## t/t0090-cache-tree.sh ##


 t/t0090-cache-tree.sh                    | 1 +
 t/t2021-checkout-overwrite.sh            | 1 +
 t/t3404-rebase-interactive.sh            | 1 +
 t/t3435-rebase-gpg-sign.sh               | 1 +
 t/t3510-cherry-pick-sequence.sh          | 1 +
 t/t5304-prune.sh                         | 1 +
 t/t6415-merge-dir-to-symlink.sh          | 6 ++++--
 t/t6424-merge-unrelated-index-changes.sh | 1 +
 t/t6430-merge-recursive.sh               | 4 +++-
 t/t6436-merge-overwrite.sh               | 3 ++-
 t/t7201-co.sh                            | 1 +
 t/t7600-merge.sh                         | 1 +
 12 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 9bf66c9e68d..90675726484 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -195,6 +195,7 @@ test_expect_success 'reset --hard gives cache-tree' '
 
 test_expect_success 'reset --hard without index gives cache-tree' '
 	rm -f .git/index &&
+	git clean -fd &&
 	git reset --hard &&
 	test_cache_tree
 '
diff --git a/t/t2021-checkout-overwrite.sh b/t/t2021-checkout-overwrite.sh
index 70d69263e68..660132ff8d5 100755
--- a/t/t2021-checkout-overwrite.sh
+++ b/t/t2021-checkout-overwrite.sh
@@ -48,6 +48,7 @@ test_expect_success 'checkout commit with dir must not remove untracked a/b' '
 
 test_expect_success SYMLINKS 'the symlink remained' '
 
+	test_when_finished "rm a/b" &&
 	test -h a/b
 '
 
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index d877872e8f4..972ce026bbc 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -297,6 +297,7 @@ test_expect_success 'abort with error when new base cannot be checked out' '
 		output &&
 	test_i18ngrep "file1" output &&
 	test_path_is_missing .git/rebase-merge &&
+	rm file1 &&
 	git reset --hard HEAD^
 '
 
diff --git a/t/t3435-rebase-gpg-sign.sh b/t/t3435-rebase-gpg-sign.sh
index ec107668580..5f8ba2c7399 100755
--- a/t/t3435-rebase-gpg-sign.sh
+++ b/t/t3435-rebase-gpg-sign.sh
@@ -65,6 +65,7 @@ test_rebase_gpg_sign ! true  -i --gpg-sign --no-gpg-sign
 test_rebase_gpg_sign   false -i --no-gpg-sign --gpg-sign
 
 test_expect_failure 'rebase -p --no-gpg-sign override commit.gpgsign' '
+	test_when_finished "git clean -f" &&
 	git reset --hard merged &&
 	git config commit.gpgsign true &&
 	git rebase -p --no-gpg-sign --onto=one fork-point main &&
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 49010aa9469..3b0fa66c33d 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -238,6 +238,7 @@ test_expect_success 'allow skipping commit but not abort for a new history' '
 '
 
 test_expect_success 'allow skipping stopped cherry-pick because of untracked file modifications' '
+	test_when_finished "rm unrelated" &&
 	pristine_detach initial &&
 	git rm --cached unrelated &&
 	git commit -m "untrack unrelated" &&
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 7cabb85ca6e..8ae314af585 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -291,6 +291,7 @@ test_expect_success 'prune: handle HEAD reflog in multiple worktrees' '
 		cat ../expected >blob &&
 		git add blob &&
 		git commit -m "second commit in third" &&
+		git clean -f && # Remove untracked left behind by deleting index
 		git reset --hard HEAD^
 	) &&
 	git prune --expire=now &&
diff --git a/t/t6415-merge-dir-to-symlink.sh b/t/t6415-merge-dir-to-symlink.sh
index 2ce104aca7c..2655e295f5a 100755
--- a/t/t6415-merge-dir-to-symlink.sh
+++ b/t/t6415-merge-dir-to-symlink.sh
@@ -25,7 +25,8 @@ test_expect_success 'checkout does not clobber untracked symlink' '
 	git reset --hard main &&
 	git rm --cached a/b &&
 	git commit -m "untracked symlink remains" &&
-	test_must_fail git checkout start^0
+	test_must_fail git checkout start^0 &&
+	git clean -fd    # Do not leave the untracked symlink in the way
 '
 
 test_expect_success 'a/b-2/c/d is kept when clobbering symlink b' '
@@ -34,7 +35,8 @@ test_expect_success 'a/b-2/c/d is kept when clobbering symlink b' '
 	git rm --cached a/b &&
 	git commit -m "untracked symlink remains" &&
 	git checkout -f start^0 &&
-	test_path_is_file a/b-2/c/d
+	test_path_is_file a/b-2/c/d &&
+	git clean -fd    # Do not leave the untracked symlink in the way
 '
 
 test_expect_success 'checkout should not have deleted a/b-2/c/d' '
diff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh
index 5e3779ebc93..89dd544f388 100755
--- a/t/t6424-merge-unrelated-index-changes.sh
+++ b/t/t6424-merge-unrelated-index-changes.sh
@@ -132,6 +132,7 @@ test_expect_success 'merge-recursive, when index==head but head!=HEAD' '
 
 	# Make index match B
 	git diff C B -- | git apply --cached &&
+	test_when_finished "git clean -fd" &&  # Do not leave untracked around
 	# Merge B & F, with B as "head"
 	git merge-recursive A -- B F > out &&
 	test_i18ngrep "Already up to date" out
diff --git a/t/t6430-merge-recursive.sh b/t/t6430-merge-recursive.sh
index ffcc01fe65d..a0efe7cb6db 100755
--- a/t/t6430-merge-recursive.sh
+++ b/t/t6430-merge-recursive.sh
@@ -718,7 +718,9 @@ test_expect_success 'merge-recursive remembers the names of all base trees' '
 	# merge-recursive prints in reverse order, but we do not care
 	sort <trees >expect &&
 	sed -n "s/^virtual //p" out | sort >actual &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+
+	git clean -fd
 '
 
 test_expect_success 'merge-recursive internal merge resolves to the sameness' '
diff --git a/t/t6436-merge-overwrite.sh b/t/t6436-merge-overwrite.sh
index 84b4aacf496..c0b7bd7c3fe 100755
--- a/t/t6436-merge-overwrite.sh
+++ b/t/t6436-merge-overwrite.sh
@@ -68,7 +68,8 @@ test_expect_success 'will not overwrite removed file' '
 	git commit -m "rm c1.c" &&
 	cp important c1.c &&
 	test_must_fail git merge c1a &&
-	test_cmp important c1.c
+	test_cmp important c1.c &&
+	rm c1.c  # Do not leave untracked file in way of future tests
 '
 
 test_expect_success 'will not overwrite re-added file' '
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 7f6e23a4bb9..b7ba1c3268e 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -585,6 +585,7 @@ test_expect_success 'checkout --conflict=diff3' '
 '
 
 test_expect_success 'failing checkout -b should not break working tree' '
+	git clean -fd &&  # Remove untracked files in the way
 	git reset --hard main &&
 	git symbolic-ref HEAD refs/heads/main &&
 	test_must_fail git checkout -b renamer side^ &&
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 2ef39d3088e..c773e30b3fa 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -717,6 +717,7 @@ test_expect_success 'failed fast-forward merge with --autostash' '
 	git reset --hard c0 &&
 	git merge-file file file.orig file.5 &&
 	cp file.5 other &&
+	test_when_finished "rm other" &&
 	test_must_fail git merge --autostash c1 2>err &&
 	test_i18ngrep "Applied autostash." err &&
 	test_cmp file.5 file

base-commit: e0a2f5cbc585657e757385ad918f167f519cfb96
-- 
gitgitgadget