Subject: Re: [PATCH 07/15] remote-mediawiki tests: guard test_cmp with test_path_is_file

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 21 Sep 2020 12:42:04 +0200

On Mon, Sep 21 2020, Ævar Arnfjörð Bjarmason wrote:

> On Wed, Sep 16 2020, Eric Sunshine wrote:
>
>> On Wed, Sep 16, 2020 at 8:17 AM Ævar Arnfjörð Bjarmason
>> <[email-masked]> wrote:
>>> Change a test that used a plain test_cmp to first check the file(s)
>>> using test_path_is_file. If some of these file(s) don't exist (as
>>> happened to me during debugging), test_cmp will emit a way less useful
>>> message about the failure.
>>
>> An alternative would be to update test_cmp() to present a more helpful
>> error message so that all test scripts can benefit rather than just
>> this script. By the way, were you testing with a reasonably recent
>> version of Git? I ask because test_cmp() was updated not long ago to
>> provide better diagnostics when one of the files is missing.
>>
>> [1]: d572f52a64 (test_cmp: diagnose incorrect arguments, 2020-08-09)
>
> Thanks (and also to Đoàn Trần Công Danh in a side-thread). I've dropped
> this patch. It's indeed better to leave this to a more general facility
> as in your now-integrated test_cmp patch.
>
> The reason I came up with this now-useless patch is because I originally
> started hacking this series on a slightly older version of git, which
> didn't have that patch.

Correction: It was the other way around, but I ran into the case with
your patch + converting a test to test_expect_failure, where before that
would be an "ok" failure since a file was missing, but now hard errosr
with a BUG.

I think that behavior is OK, but in going back&forth and rebasing
managed to miss it the first time around. The v2 of this series has a
more narrow fix for that.