t4018: drop "debugging" cat from hunk-header tests
diff mbox series

Message ID 20200116183423.GA3011203@coredump.intra.peff.net
State New
Headers show
Series
  • t4018: drop "debugging" cat from hunk-header tests
Related show

Commit Message

Jeff King Jan. 16, 2020, 6:34 p.m. UTC
We run a series of hunk-header tests in a loop, and each one does this:

  test_when_finished 'cat actual' &&      # for debugging only

This is pretty pointless. When the test succeeds, we waste time running
a useless cat process. If you're debugging a failure with "-i", then we
won't run the when-finished part at all. So it helps only if you're
running with something like "--verbose-log".

Since we expect the tests to succeed most of the time, a better way to
do this would be a helper that checks the output and dumps "actual" only
when it fails. But it's probably not even worth the effort, as anyone
debugging a failure could just run with "-i" and investigate the
"actual" file themselves.

Signed-off-by: Jeff King <peff@peff.net>
---
Just noticed this when working with t4018 on an unrelated problem.

I could be convinced otherwise on the final paragraph, but I think it
would only be worth it if we added a general test_grep() helper and used
it in more places.

 t/t4018-diff-funcname.sh | 1 -
 1 file changed, 1 deletion(-)

Comments

Jonathan Nieder Jan. 16, 2020, 10:56 p.m. UTC | #1
Jeff King wrote:

> We run a series of hunk-header tests in a loop, and each one does this:
>
>   test_when_finished 'cat actual' &&      # for debugging only
>
> This is pretty pointless. When the test succeeds, we waste time running
> a useless cat process. If you're debugging a failure with "-i", then we
> won't run the when-finished part at all. So it helps only if you're
> running with something like "--verbose-log".
>
> Since we expect the tests to succeed most of the time, a better way to
> do this would be a helper that checks the output and dumps "actual" only
> when it fails. But it's probably not even worth the effort, as anyone
> debugging a failure could just run with "-i" and investigate the
> "actual" file themselves.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Just noticed this when working with t4018 on an unrelated problem.
>
> I could be convinced otherwise on the final paragraph, but I think it
> would only be worth it if we added a general test_grep() helper and used
> it in more places.
>
>  t/t4018-diff-funcname.sh | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

Sure, a test_grep would be nice for CI cases (especially for
Heisenbugs), but in its absence I agree that this patch is the right
thing to do.

Patch
diff mbox series

diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index c0f4839543..02255a08bf 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -106,7 +106,6 @@  do
 		result=success
 	fi
 	test_expect_$result "hunk header: $i" "
-		test_when_finished 'cat actual' &&	# for debugging only
 		git diff -U1 $i >actual &&
 		grep '@@ .* @@.*RIGHT' actual
 	"