Re: [RFC PATCH 4/6] tests: use `git submodule add` and fix expected diffs

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 13, 2023, 23:19 UTC
Message-ID: <xmqqttzp887h.fsf@gitster.g>
In-Reply-To: <xmqqy1p188rj.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 26 quoted lines

> Calvin Wan <calvinwan@google.com> writes:
>
>> From: Josh Steadmon <steadmon@google.com>
>>
>> This commit continues the previous work of updating the test suite to
>> use `git submodule add` to create submodules instead of using `git add`
>> to include embedded repositories. Specifically, in this commit we update
>> test cases where expected diffs must change due to the presence of a
>> .gitmodules file.
>
> Good that these "same" tasks are split into two steps, ones that
> don't notice the additional presence of .gitmodules (i.e. the
> previous step) and the result that do.
>
> Strictly speaking, gitlink support in the index and trees does not
> have to be used to record submodules, and changing all "git add" to
> "git submodule add" is somewhat a sad change that closes the door to
> those folks who want to use the mechanism for implementing "better
> submodules" that do not depend on a separate .gitmodules files, for
> example.  To leave the door open, "git add" may need to learn an
> overriding mechanism allow adding a gitlink to the index without
> anything else.  And then this step could use "git add -f" (or
> whategver the overriding mechanism) instead of "git submodule add",
> without changing the expected diff output.
>
> Would such a tweak make this step better?  I dunno.

The other side of the coin is that the tests that were touched by the previous step were not testing half of what "submodule add" is doing. Not all the "let's add submodule here" test may not have to verify the resulting .gitmodules file (e.g. ones that want to see the low-level diff machinery works correctly by taking output from "git diff --cached --raw" would be primarily interested in what happens to the index and not to .gitmodules at all), but some of them should. A rough rule of thumb might be that the tests that are about "submodules" should care what ".gitmodules" has, while the tests tare are about "gitlink" should not. It might even make sense to use "add -f" (or whatever override to allow adding a gitlink only to the index without touching .gitmodules and other things), not "submodule add", for the latter class of tests.

Thanks.