Re: [PATCH] Tests: document test_submodule_{,forced_}switch()

[Stefan Beller <sbeller@google.com>]

On Mon, Nov 6, 2017 at 5:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> Factor out the commonalities from test_submodule_switch() and
>> test_submodule_forced_switch() in lib-submodule-update.sh, and document
>> their usage.
>>
>> This also makes explicit (through the KNOWN_FAILURE_FORCED_SWITCH_TESTS
>> variable) the fact that, currently, all functionality tested using
>> test_submodule_forced_switch() do not correctly handle the situation in
>> which a submodule is replaced with an ordinary directory.
>>
>> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
>> ---
>> I find tests that use lib-submodule-update.sh difficult to understand
>> due to the lack of clarity of what test_submodule_switch() and others do
>> with their argument - I hope this will make things easier for future
>> readers.
>> ---
>>  t/lib-submodule-update.sh | 250 +++++++++-------------------------------------
>>  1 file changed, 46 insertions(+), 204 deletions(-)
>
> I suspect that the benefit of this is a lot larger than "document" a
> test helper function or two ;-)  "document & clean-up", perhaps?

It is.

>
> I didn't compare the before-and-after with fine toothed comb, but
> a cursory look didn't find anything glaringly questionable other
> than the above.
>

I have reviewed the patch and thinks it withstands the test of a fine comb.