diff mbox series

[08/30] subtree: t7900: fix 'verify one file change per commit'

Message ID 20210423194230.1388945-9-lukeshu@lukeshu.com (mailing list archive)
State Superseded
Headers show
Series subtree: clean up, improve UX | expand

Commit Message

Luke Shumaker April 23, 2021, 7:42 p.m. UTC
From: Luke Shumaker <lukeshu@datawire.io>

As far as I can tell, this test isn't actually testing anything, because
someone forgot to tack on `--name-only` to `git log`.  This seems to
have been the case since the test was first written, back in fa16ab36ad
("test.sh: make sure no commit changes more than one file at a time.",
2009-04-26), unless `git log` used to do that by default and didn't need
the flag back then?

Convincing myself that it's not actually testing anything was tricky,
the code is a little hard to reason about.  It can be made a lot simpler
if instead of trying to parse all of the info from a single `git log`,
we're OK calling `git log` from inside of a loop.  And it's my opinion
that tests are not the place for clever optimized code.

So, fix and simplify the test, so that it's actually testing something
and is simpler to reason about.

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
---
 contrib/subtree/t/t7900-subtree.sh | 41 +++++-------------------------
 1 file changed, 6 insertions(+), 35 deletions(-)
diff mbox series

Patch

diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 96acd4f8be..70be18246b 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -22,29 +22,6 @@  subtree_test_create_repo () {
 	git -C "$1" config log.date relative
 }
 
-# Make sure no patch changes more than one file.
-# The original set of commits changed only one file each.
-# A multi-file change would imply that we pruned commits
-# too aggressively.
-join_commits () {
-	commit=
-	all=
-	while read x y; do
-		if [ -z "$x" ]; then
-			continue
-		elif [ "$x" = "commit:" ]; then
-			if [ -n "$commit" ]; then
-				echo "$commit $all"
-				all=
-			fi
-			commit="$y"
-		else
-			all="$all $y"
-		fi
-	done
-	echo "$commit $all"
-}
-
 test_create_commit () (
 	repo=$1 &&
 	commit=$2 &&
@@ -865,18 +842,12 @@  test_expect_success 'verify one file change per commit' '
 		cd "$test_count" &&
 		git subtree split --prefix="sub dir2" --branch subproj2-br &&
 
-		x= &&
-		git log --pretty=format:"commit: %H" | join_commits |
-		(
-			while read commit a b; do
-				test_debug "echo Verifying commit $commit"
-				test_debug "echo a: $a"
-				test_debug "echo b: $b"
-				test "$b" = ""
-				x=1
-			done
-			test "$x" = 1
-		)
+		git log --format="%H" > commit-list &&
+		while read commit
+		do
+			git log -n1 --format="" --name-only "$commit" >file-list &&
+			test_line_count -le 1 file-list || return 1
+		done <commit-list
 	)
 '