git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCHv6 1/2] submodule tests: don't use itself as a submodule

From
Stefan Beller <sbeller@google.com>
Date
Jan 9, 2017, 20:10 UTC
Message-ID
<CAGZ79kak8qzUG5G1mM8uDqxW8tVBNmuLcvMhopby_U8PvUOjJg@mail.gmail.com>
In-Reply-To
<xmqqh959ynb4.fsf@gitster.mtv.corp.google.com>
On Sun, Jan 8, 2017 at 6:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 19 quoted lines
> Stefan Beller <sbeller@google.com> writes:
>
>> This provides an easier way to have submodules in tests, by just setting
>> TEST_CREATE_SUBMODULE to a non empty string, similar to
>> TEST_NO_CREATE_REPO.
>
> Yuck.
>
> I find it doubtful that it is a good idea to create two submodule
> repositories by merely dot-including the test-lib.sh; I find it
> doubly doubtful that it is a good idea to make test_create_repo
> pay attention to the special variable to implement that.
>
> I am OK with a solution where callers that set TEST_CREATE_SUBMODULE
> variable in this patch to instead have an explicit call
>
>         test_create_repo --submodule pretzel
>
> That would be a lot more obvious.
agreed.
Show 8 quoted lines
>
> The primary reason why I hate the implementation in this patch is
> that it is very easy for a test that says TEST_CREATE_SUBMODULE
> upfront, only to get the initial test repository (which everybody
> else gets) with two test submodules, to later gain a test that wants
> to use a separate repository and call "test_create_repo".  It will
> always get the pretzel submodules, which may or may not match what
> the test writer who adds a new test needs.

I agree. At the time of writing the patch series, I was anticipating writing way more submodule tests, but now I have these future tests integrated into lib-submodule-update.sh.

Show 13 quoted lines
>
>> Make use of it in those tests that add a submodule from ./. except for
>> the occurrence in create_lib_submodule_repo as there it seems we craft
>> a repository deliberately for both inside as well as outside use.
>
> But isn't the point of this change that use of ./. cannot be
> mimicking any real-world use, hence pointless for the purpose of
> really testing the components of the system?  If "we craft
> deliberately for both inside and outside use" indeed _IS_ a good
> thing, then perhaps use of ./. has practical real-world use---if
> not, wouldn't we want to fix the scripts that include the
> lib-submodule-repo helper not to test such an unrealistic layout?
>

Makes sense; I tried to fix it up to avoid the ./. clone in create_lib_submodule_repo, but the issue is there are too many implicit assumption of these two repos, such that a faithful conversion would just duplicate code for the submodule, (e.g. create the same amount of commits, containing the same diffs, etc.), which then can be argued to just slow down the test suite as the clone from ./. is actually reducing the needed work by a factor of 2.

Previous: Junio C Hamano
Message 7 of 7 in “[PATCHv6 0/2] pathspec: give better message for submodule related pathspec error”
  1. Stefan BellerJan 5, 2017
  2. 2/2 pathspec: give better message for submodule related pathspec errorStefan Beller, Jan 5, 2017
  3. Junio C HamanoJan 9, 2017
  4. Stefan BellerJan 9, 2017
  5. 1/2 submodule tests: don't use itself as a submoduleStefan Beller, Jan 5, 2017
  6. Junio C HamanoJan 9, 2017
  7. Stefan BellerJan 9, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.