Skip to contentSkip to conversation

t7501-commit: drop silly command substitution

PATCH In review SZEDER Gábor 2 participants 2 messages Aug 21, 2018 – Aug 22, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 t7501-commit: drop silly command substitution

The test '--dry-run with conflicts fixed from a merge' in 't7501-commit.sh', added in 8dc874b2ee (wt-status.c: set commitable bit if there is a meaningful merge., 2016-02-15), runs the following unnecessary and downright bogus command substitution:

  ! $(git merge --no-commit commit-1) &&

I.e. after 'git merge ...' is executed and expectedly fails, the test attempts to execute its output:

Merging:
80f2ea2 commit 2
virtual commit-1
found 1 common ancestor:
e60d113 Initial commit
Auto-merging test-file
CONFLICT (content): Merge conflict in test-file Automatic merge failed; fix conflicts and then commit the result.

as a command, which most likely fails, because there is no such command as "Merging:". Then '!' negates the failed exit status, the test continues, and eventually succeeds.

Remove this command substitution and use 'test_must_fail' to ensure that 'git merge' fails.

---
 t/t7501-commit.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
t/t7501-commit.sh +1 -1
index 51646d8019..d766bf34c4 100755
@@ -677,7 +677,7 @@ test_expect_success '--dry-run with conflicts fixed from a merge' '
 	git checkout -b branch-2 HEAD^1 &&
 	echo "commit-2-state" >test-file &&
 	git commit -m "commit 2" -i test-file &&
-	! $(git merge --no-commit commit-1) &&
+	test_must_fail git merge --no-commit commit-1 &&
 	echo "commit-2-state" >test-file &&
 	git add test-file &&
 	git commit --dry-run &&
Signature · 1 line
2.19.0.rc0.136.gd2dd172e64
#1 lore raw
Quoted 6 lines · SZEDER Gábor

The test '--dry-run with conflicts fixed from a merge' in 't7501-commit.sh', added in 8dc874b2ee (wt-status.c: set commitable bit if there is a meaningful merge., 2016-02-15), runs the following unnecessary and downright bogus command substitution:

  ! $(git merge --no-commit commit-1) &&

I vaguely recall discussing (or noticing myself and then forgetting to mention it? I dunno) this exact one on another thread of your test fixes and drawing the same conclusion as you have here.

Will apply. Thanks for following it through.

Quoted 38 lines

I.e. after 'git merge ...' is executed and expectedly fails, the test attempts to execute its output:

Merging:
80f2ea2 commit 2
virtual commit-1
found 1 common ancestor:
e60d113 Initial commit
Auto-merging test-file
CONFLICT (content): Merge conflict in test-file Automatic merge failed; fix conflicts and then commit the result.

as a command, which most likely fails, because there is no such command as "Merging:". Then '!' negates the failed exit status, the test continues, and eventually succeeds.

Remove this command substitution and use 'test_must_fail' to ensure that 'git merge' fails.

---
 t/t7501-commit.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
t/t7501-commit.sh +1 -1
index 51646d8019..d766bf34c4 100755
@@ -677,7 +677,7 @@ test_expect_success '--dry-run with conflicts fixed from a merge' '
 	git checkout -b branch-2 HEAD^1 &&
 	echo "commit-2-state" >test-file &&
 	git commit -m "commit 2" -i test-file &&
-	! $(git merge --no-commit commit-1) &&
+	test_must_fail git merge --no-commit commit-1 &&
 	echo "commit-2-state" >test-file &&
 	git add test-file &&
 	git commit --dry-run &&