Re: [PATCH v3 4/4] bisect: consistent style for git bisect run tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 1 Aug 2020 at 20:03, Aaron Lipman <alipman88@xxxxxxxxx> wrote:
> Subject: bisect: consistent style for git bisect run tests

Nit: this could be 't6030: modernize "git bisect run" tests' to tighten
the area a bit and to begin with a verb.

> Enforce consistent styling for tests on "git bisect run":
> - Use "write_script" to abstract away platform-specific details.
> - Favor current whitespace conventions.
> ---

Missing sign-off.

I think this should come earlier in the series, probably already as
patch 1/4. Some reasons for that:

 * If this effort doesn't go any further (let's hope it does!), at
   least the initial cleanup can be merged, so that later endeavours can
   find a slightly cleaner table to start with.

 * Revisiting your other patches in `git log` after they've been merged,
   it will be obvious that they follow other tests, so one can focus on
   what they do and not on how they differ.

 * Let's first tidy up, then build shiny new things. ;-)

>  # We want to automatically find the commit that
> -# introduced "Another" into hello.
> -test_expect_success \
> -    '"git bisect run" simple case' \
> -    'echo "#"\!"/bin/sh" > test_script.sh &&
[...]
> +# added "Another" into hello.
> +test_expect_success '"git bisect run" simple case' '
> +       write_script test_script.sh <<-\EOF &&
[...]

All of these look like correct, worthwhile modifications. The
s/introduced/added/ changes aren't mentioned in the proposed log
message. They have been discussed up-thread, but it might still be
good to add something like

  While at it, change "introduced" to "added" in the comments to make
  them read better.

FWIW, I guess there's some minor added value in doing that change and
since you're doing other changes anyway, it might actually be worth the
churn.

You change two tests like this, but there are other "git bisect run"
tests that look like they're in need of some modernizing. Looking for
"chmod" I spotted the ones I'm tweaking below. Looking for "git bisect
run" you might find more that may contain some funny indentation. I'll
leave it to you where to stop going down that rabbit-hole but if you do
re-roll, it might make sense to squash in the diff below, maybe with
some modifications to the commit message. (Signed-off-by: Martin Ågren
<martin.agren@xxxxxxxxx>, FWIW.)

Martin

-- >8 --

diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 63661c5e3d..66c12bc665 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -293,24 +293,17 @@ HASH6=
 test_expect_success 'bisect run & skip: cannot tell between 2' '
 	add_line_into_file "6: Yet a line." hello &&
 	HASH6=$(git rev-parse --verify HEAD) &&
-	echo "#"\!"/bin/sh" > test_script.sh &&
-	echo "sed -ne \\\$p hello | grep Ciao > /dev/null && exit 125" >> test_script.sh &&
-	echo "grep line hello > /dev/null" >> test_script.sh &&
-	echo "test \$? -ne 0" >> test_script.sh &&
-	chmod +x test_script.sh &&
+	write_script test_script.sh <<-\EOF &&
+	sed -ne \$p hello | grep Ciao >/dev/null && exit 125
+	! grep line hello >/dev/null
+	EOF
 	git bisect start $HASH6 $HASH1 &&
-	if git bisect run ./test_script.sh > my_bisect_log.txt
-	then
-		echo Oops, should have failed.
-		false
-	else
-		test $? -eq 2 &&
-		grep "first bad commit could be any of" my_bisect_log.txt &&
-		! grep $HASH3 my_bisect_log.txt &&
-		! grep $HASH6 my_bisect_log.txt &&
-		grep $HASH4 my_bisect_log.txt &&
-		grep $HASH5 my_bisect_log.txt
-	fi
+	test_expect_code 2 git bisect run ./test_script.sh >my_bisect_log.txt &&
+	grep "first bad commit could be any of" my_bisect_log.txt &&
+	! grep $HASH3 my_bisect_log.txt &&
+	! grep $HASH6 my_bisect_log.txt &&
+	grep $HASH4 my_bisect_log.txt &&
+	grep $HASH5 my_bisect_log.txt
 '
 
 HASH7=
@@ -318,14 +311,13 @@ test_expect_success 'bisect run & skip: find first bad' '
 	git bisect reset &&
 	add_line_into_file "7: Should be the last line." hello &&
 	HASH7=$(git rev-parse --verify HEAD) &&
-	echo "#"\!"/bin/sh" > test_script.sh &&
-	echo "sed -ne \\\$p hello | grep Ciao > /dev/null && exit 125" >> test_script.sh &&
-	echo "sed -ne \\\$p hello | grep day > /dev/null && exit 125" >> test_script.sh &&
-	echo "grep Yet hello > /dev/null" >> test_script.sh &&
-	echo "test \$? -ne 0" >> test_script.sh &&
-	chmod +x test_script.sh &&
+	write_script test_script.sh <<-\EOF &&
+	sed -ne \$p hello | grep Ciao >/dev/null && exit 125
+	sed -ne \$p hello | grep day >/dev/null && exit 125
+	! grep Yet hello >/dev/null
+	EOF
 	git bisect start $HASH7 $HASH1 &&
-	git bisect run ./test_script.sh > my_bisect_log.txt &&
+	git bisect run ./test_script.sh >my_bisect_log.txt &&
 	grep "$HASH6 is the first bad commit" my_bisect_log.txt
 '
 
-- 
2.28.0.81.ge8ab941b67




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux