Re: [PATCH v2 4/6] t1450: don't create unused files

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Tue, Apr 04 2023, Andrei Rybak wrote:

> Test 'fsck error and recovery on invalid object type' in file
> t1450-fsck.sh redirects output of a failing "git fsck" invocation to
> files "out" and "err" to assert presence of error messages in the output
> of the command.  Commit 31deb28f5e (fsck: don't hard die on invalid
> object types, 2021-10-01) changed the way assertions in this test are
> performed.  The test doesn't compare the whole standard error with
> prepared file "err.expect" and it doesn't assert that standard output is
> empty.

That's my commit, and thanks for catching this...

> Don't create unused files "err.expect" and "out" in test 'fsck error and
> recovery on invalid object type'.
>
> Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
> ---
>  t/t1450-fsck.sh | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> index bca46378b2..8c442adb1a 100755
> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -989,10 +989,7 @@ test_expect_success 'fsck error and recovery on invalid object type' '
>  
>  		garbage_blob=$(git hash-object --stdin -w -t garbage --literally </dev/null) &&
>  
> -		cat >err.expect <<-\EOF &&
> -		fatal: invalid object type
> -		EOF
> -		test_must_fail git fsck >out 2>err &&
> +		test_must_fail git fsck 2>err &&
>  		grep -e "^error" -e "^fatal" err >errors &&
>  		test_line_count = 1 errors &&
>  		grep "$garbage_blob: object is of unknown type '"'"'garbage'"'"':" err

...but ditto my review on other patches, this just seems like a mistake
of mine, i.e. if I add the "test_must_be_empty out" the test passes.

So isn't the answer here that my 31deb28f5e had an unintentional
regression, and we should bring the assertion back? Its commit message
says nothing about wanting to stop asserting stdout.

Maybe there was a reason I'm missing for why I remved it, it's since
been paged out of my wetware, but looking at it briefly now it just
seems like an unintentional bug / loss of test coverage that we should
fix.