Subject: Re: [PATCH 2/2] diff tests: rewrite flakyness-causing test "aid"

From: Junio C Hamano <[email-masked]>
Date: Fri, 16 Apr 2021 11:32:55 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

> Are you thinking of the case where someone wants to add N new lines to
> the "while read magic cmd" loop?

No.  I think stopping with BUG is fine.  Adding a new test is a
serious event and I'd prefer to make it stop to give it undistracted
attention by the developer, instead of letting many to be handled at
once.

Of course, we _could_ instead remember the fact that we deposted
newly proposed expectation pattern that the user must audit, still
keep going and list the new files at the end, but I do not think it
is necessary.  Your newly added BUG is perfectly fine. 

>>> In any case, I think it's best to just drop this series.
>>
>> That is a sad and wrong conclusion.  We should just realize what we
>> really wanted to fix in the first place and keep the improvement;
>> otherwise all the review time was wasted.
>
> To be clear that's not some "I can't have the change I want so I'm
> taking my toys and going home" tantrum. I don't mind making changes in
> response to reviews etc.
>
> As noted above I'm genuinely still unable to see how I'd selectively
> untangle the change I made in a way that both wouldn't either fix bugs
> in the test, or introduce new bugs while I was at it, and that I'd be
> comfortable putting my name behind.

I fail to see why the "rewrite flakyness-causing test aid" that you
set out to perform should need more than a two-liner change.  There
is no reason to touch '&& rm "$actual" actual expect' at the end.
It is the success case where the flakyness-causing "aid" does not
even kick in.

 t/t4013-diff-various.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git c/t/t4013-diff-various.sh w/t/t4013-diff-various.sh
index 6cca8b84a6..da6da13920 100755
--- c/t/t4013-diff-various.sh
+++ w/t/t4013-diff-various.sh
@@ -229,8 +229,8 @@ do
 			rm -f "$actual" actual expect
 		else
 			# this is to help developing new tests.
-			cp "$actual" "$expect"
-			false
+			mv "$actual" "$expect.new"
+			BUG "No $expect; check $expect.new and use it perhaps?"
 		fi
 	'
 done <<\EOF
n