Subject: Re: [PATCH] t0050: remove the unused $test_case variable

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 07 Feb 2018 20:08:08 +0100

On Wed, Feb 07 2018, Johannes Sixt jotted:

> Am 07.02.2018 um 09:07 schrieb Ævar Arnfjörð Bjarmason:
>>
>> On Wed, Feb 07 2018, Johannes Sixt jotted:
>>
>>> Am 07.02.2018 um 00:13 schrieb Ævar Arnfjörð Bjarmason:
>>>> The $test_case variable hasn't been used since
>>>> decd3c0c28 ("t0050-*.sh: mark the rename (case change) test as
>>>> passing", 2014-11-28) when its last user went away.
>>>>
>>>> Let's remove the "say" as well, since it's obvious from subsequent
>>>> output that we're testing on a case sensitive filesystem.
>>>
>>> Am I misunderstanding the message? I think it reports properties of
>>> the test environment. And the tests do run on case-insensitive
>>> filesystems. IMO, the message should be kept.
>>
>> It's obvious from subsequent output whether the FS is case sensitive or
>> not, so I thought it was redundant to keep this report at the top since
>> we didn't have the variable setting anymore.
>
> There are test cases that do different things depending on whether the
> CASE_INSENSITIVE_FS prerequisite is set. I think it was the intent to
> report whether it is set and not whether one or the other value of the
> (now unused) variable is used somewhere.
>
> BTW, the message texts do not show which variant is taken (these are
> without your patch):
>
> On Windows:
>
> t>sh t0050-filesystem.sh
> will test on a case insensitive filesystem
> will test on a filesystem lacking symbolic links
> ok 1 - detection of case insensitive filesystem during repo init
> ok 2 - detection of filesystem w/o symlink support during repo init
> ok 3 - setup case tests
> ok 4 - rename (case change)
> ok 5 - merge (case change)
> not ok 6 - add (with different case) # TODO known breakage
> ok 7 - setup unicode normalization tests
> ok 8 - rename (silent unicode normalization)
> ok 9 - merge (silent unicode normalization)
> # still have 1 known breakage(s)
> # passed all remaining 8 test(s)
> 1..9
>
> On Linux:
>
> t@master:1002> ./t0050-filesystem.sh
> ok 1 - detection of case insensitive filesystem during repo init
> ok 2 - detection of filesystem w/o symlink support during repo init
> ok 3 - setup case tests
> ok 4 - rename (case change)
> ok 5 - merge (case change)
> ok 6 # skip add (with different case) (missing CASE_INSENSITIVE_FS)
> ok 7 - setup unicode normalization tests
> ok 8 - rename (silent unicode normalization)
> ok 9 - merge (silent unicode normalization)
> # passed all 9 test(s)
> 1..9
>
> I'd even argue that there should be messages on Linux, too.

Thanks. Let's just drop this patch. I thought it would still print out
something similar to that "missing CASE_INSENSITIVE_FS" at a quick
glance last night, but was obviously wrong.