RE: [Test] t1901 - sparse checkout file when lock is taken fails (subtest 19)

["Randall S. Becker" <rsbecker@nexbridge.com>]

On March 11, 2020 6:06 PM, Junio C Hamano wrote:
> "Randall S. Becker" <rsbecker@nexbridge.com> writes:
> 
> > This situation still occurs at 2.26.0-rc0. As above, this results from
> > a text compare to a platform-specific message that should not be used.
> > To hack around it, a possible fix (which I don't like) could be as follows:
> 
> Martin's fix 4605a730 (t1091: don't grep for `strerror()` string,
> 2020-03-08) was merged to 'next' on the 9th and then down to 'master'
> today, and will be in the final (unless there is some issues with it, which I do
> not think will be the case).
> 
> Thanks.
> 
> -- >8 --
> From: Martin Ågren <martin.agren@gmail.com>
> Subject: [PATCH] t1091: don't grep for `strerror()` string
> 
> We grep for "File exists" in stderr of the failing `git sparse-checkout` to make
> sure that it failed for the right reason. We expect the string to show up there
> since we call `strerror(errno)` in `unable_to_lock_message()` in lockfile.c.
> 
> On the NonStop platform, this fails because the error string is "File already
> exists", which doesn't match our grepping.
> 
> See 9042140097 ("test-dir-iterator: do not assume errno values",
> 2019-07-30) for a somewhat similar fix. There, we patched a test helper,
> which meant we had access to `errno` and could investigate it better in the
> test helper instead of just outputting the numerical value and evaluating it in
> the test script. The current situation is different, since (short of modifying the
> lockfile machinery, e.g., to be more
> verbose) we don't have more than the output from `strerror()` available.
> 
> Except we do: We prefix `strerror(errno)` with `_("Unable to create
> '%s.lock': ")`. Let's grep for that part instead. It verifies that we were indeed
> unable to create the lock file. (If that fails for some other reason than the file
> existing, we really really should expect other tests to fail as well.)

I'm hoping that that error message matches. Will test it as soon as it is final is out there.

> An alternative fix would be to loosen the expression a bit and grep for
> "File.* exists" instead. There would be no guarantee that some other
> implementation couldn't come up with another error string, That is, that
> could be the first move in an endless game of whack-a-mole. Of course, it
> could also take us from "99" to "100" percent of the platforms and we'd
> never have this problem again. But since we have another way of addressing
> this, let's not even try the "loosen it up a bit" strategy.
> 
> Reported-by: Randall S. Becker <rsbecker@nexbridge.com>
> Signed-off-by: Martin Ågren <martin.agren@gmail.com>
> Acked-by: Derrick Stolee <dstolee@microsoft.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  t/t1091-sparse-checkout-builtin.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-
> builtin.sh
> index b4c9c32a03..44a91205d6 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -305,7 +305,7 @@ test_expect_success 'fail when lock is taken' '
>  	test_when_finished rm -rf repo/.git/info/sparse-checkout.lock &&
>  	touch repo/.git/info/sparse-checkout.lock &&
>  	test_must_fail git -C repo sparse-checkout set deep 2>err &&
> -	test_i18ngrep "File exists" err
> +	test_i18ngrep "Unable to create .*\.lock" err
>  '
> 
>  test_expect_success '.gitignore should not warn about cone mode' '
> --
> 2.26.0-rc1-6-ga56d361f66

This still really helps stabilize things for the NonStop platform.

Thanks!
Randall