Re: [PATCH v1 6/7] t1404: don't create unused file

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 13, 2023, 21:56 UTC
Message-ID: <xmqqo7ows4bv.fsf@gitster.g>
In-Reply-To: <20230312201520.370234-8-rybak.a.v@gmail.com>

Andrei Rybak <rybak.a.v@gmail.com> writes:

Show 9 quoted lines

> Some tests in file t1404-update-ref-errors.sh create file "unchanged" as
> the expected side for a test_cmp assertion at the end of the test for
> output of "git for-each-ref".  The filename conveys the expectation that
> the output won't change between two invocations of "git for-each-ref".
>
> Test 'no bogus intermediate values during delete' also creates a file
> named "unchanged".  However, in this test the reference is being
> deleted, i.e. it _does change_.  The file itself isn't used for any
> assertions in the test.

I think the name "unchanged" is a reference to: the state recorded in this file is before all the interesting changes done in this test.

So another for-each-ref, after the "lock, start a process that waits for and then removes the ref" begins but while the other process is still waiting, whose output is compared with "unchanged" may have been another way to perform this test, but we have "it could be $D that is what we want, and two plausible 'wrong' answers are $C and undefined" that is sufficient. So I agree with removing the line that creates "unchanged".

The other test to the file added by the same commit 6a2a7736 (t1404: demonstrate two problems with reference transactions, 2017-09-08) creates the "unchanged" file in the same way, but it does get used after running "update-ref" that is tested. I would not be surprised if the one removed by this patch was created by a cut-and-paste by mistake.

Thanks.

Show 19 quoted lines

> Don't create the unused and slightly misleading file "unchanged".
>
> Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
> ---
>  t/t1404-update-ref-errors.sh | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
> index b5606d93b5..937ae0d733 100755
> --- a/t/t1404-update-ref-errors.sh
> +++ b/t/t1404-update-ref-errors.sh
> @@ -551,7 +551,6 @@ test_expect_success REFFILES 'no bogus intermediate values during delete' '
>  	git update-ref $prefix/foo $C &&
>  	git pack-refs --all &&
>  	git update-ref $prefix/foo $D &&
> -	git for-each-ref $prefix >unchanged &&
>  	# Now try to update the reference, but hold the `packed-refs` lock
>  	# for a while to see what happens while the process is blocked:
>  	: >.git/packed-refs.lock &&