diff mbox series

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

Message ID 20200801175840.1877-5-alipman88@gmail.com
State New, archived
Headers show
Series Introduce --first-parent flag for git bisect | expand

Commit Message

Aaron Lipman Aug. 1, 2020, 5:58 p.m. UTC
Enforce consistent styling for tests on "git bisect run":
- Use "write_script" to abstract away platform-specific details.
- Favor current whitespace conventions.
---
 t/t6030-bisect-porcelain.sh | 46 ++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

Comments

Martin Ågren Aug. 1, 2020, 7:27 p.m. UTC | #1
On Sat, 1 Aug 2020 at 20:03, Aaron Lipman <alipman88@gmail.com> 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@gmail.com>, 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
 '
diff mbox series

Patch

diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 51f5eb6ea3..63661c5e3d 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -243,32 +243,30 @@  test_expect_success 'bisect skip: with commit both bad and skipped' '
 '
 
 # 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 &&
-     echo "grep Another hello > /dev/null" >> test_script.sh &&
-     echo "test \$? -ne 0" >> test_script.sh &&
-     chmod +x test_script.sh &&
-     git bisect start &&
-     git bisect good $HASH1 &&
-     git bisect bad $HASH4 &&
-     git bisect run ./test_script.sh > my_bisect_log.txt &&
-     grep "$HASH3 is the first bad commit" my_bisect_log.txt &&
-     git bisect reset'
+# added "Another" into hello.
+test_expect_success '"git bisect run" simple case' '
+	write_script test_script.sh <<-\EOF &&
+	! grep Another hello >/dev/null
+	EOF
+	git bisect start &&
+	git bisect good $HASH1 &&
+	git bisect bad $HASH4 &&
+	git bisect run ./test_script.sh >my_bisect_log.txt &&
+	grep "$HASH3 is the first bad commit" my_bisect_log.txt &&
+	git bisect reset
+'
 
 # We want to automatically find the commit that
-# introduced "Ciao" into hello.
-test_expect_success \
-    '"git bisect run" with more complex "git bisect start"' \
-    'echo "#"\!"/bin/sh" > test_script.sh &&
-     echo "grep Ciao hello > /dev/null" >> test_script.sh &&
-     echo "test \$? -ne 0" >> test_script.sh &&
-     chmod +x test_script.sh &&
-     git bisect start $HASH4 $HASH1 &&
-     git bisect run ./test_script.sh > my_bisect_log.txt &&
-     grep "$HASH4 is the first bad commit" my_bisect_log.txt &&
-     git bisect reset'
+# added "Ciao" into hello.
+test_expect_success '"git bisect run" with more complex "git bisect start"' '
+	write_script test_script.sh <<-\EOF &&
+	! grep Ciao hello >/dev/null
+	EOF
+	git bisect start $HASH4 $HASH1 &&
+	git bisect run ./test_script.sh >my_bisect_log.txt &&
+	grep "$HASH4 is the first bad commit" my_bisect_log.txt &&
+	git bisect reset
+'
 
 # $HASH1 is good, $HASH5 is bad, we skip $HASH3
 # but $HASH4 is good,