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

Re: [PATCH 1/2] t7406: correct depth test in shallow test

From
Stefan Beller <sbeller@google.com>
Date
Jul 28, 2016, 19:24 UTC
Message-ID
<CAGZ79kZfXKC66uokqOXeWND=V+53YfAkfd1NUK7A4jM2UAea7A@mail.gmail.com>
In-Reply-To
<CAGZ79ka5n1_5CM3DOGHcd3W=4KJb9d-sb=E1pyvzP=XWGVXE3g@mail.gmail.com>
On Thu, Jul 28, 2016 at 11:47 AM, Stefan Beller <sbeller@google.com> wrote:
Show 33 quoted lines
> On Thu, Jul 28, 2016 at 11:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> We used to ask for 3 changes and tested for having 1, so the test
>>> seems broken.
>>
>> I am not sure what to think of "seems broken".
>
> When asking for depth 3, I would expect a result of 1,2, or 3 commits.
>
> But when testing the depth argument with a history less than 3, and then
> implying: "I got 1, which is less than 3, so the depth works", seems
> to be a logical shortcut to me.
>
> I would have expected a history of >3, then ask for 3 and confirm we did not
> get 4 or 5 or 6, but 3 only.
>
>>
>> Asking for 3 and having 1 is broken in what way?  Should we be
>> expecting for 3 because we asked for that many?  Should we expect
>> less than three even though we asked for three because the upstream
>> side does not even have that many?  If the current test that asks
>> for 3 and gets only 1 is not failing, why should we expect that
>> asking for 2 would get 2?  In other words, why is it sane that
>> asking for fewer number of commits gives us more?
>
> I think there is a subtle thing going on, that I did not examine properly but
> it is hidden in the modernization from
>
>     test 1 = $(something)
>  to test_line_count = 2
>
> I'll investigate the actual reason.

So when I place a test_pause just before that check for the lines we have the upstream submodule: $ git log --oneline 6355002 upstream line4 820877d upstream line3 4301fd3 Commit 2 0c90624 upstream

which then allows fetching 6355002 upstream line4 820877d upstream line3 4301fd3 Commit 2

and "Commit 2" is recorded as the sha1, i.e. when checking out "Commit 2" and running (git log --oneline | wc -l) we get 1 as it cuts off after that.

When adding a test (in the next patch) that adds more commits to the submodule upstream, we will fetch with depth 3 but will no longer see the sha1, so we try a different approach. Current approach: try fetching again with no depth, and then try again with sha1 given.

So one could say there is a bug as the fetching should use the depth argument as well.

The depth of 2 I chose originally turns out to be a lucky accident too, as the depth from "Commit 2" is 2, so that we would observe the same depth no matter if a --depth 2 was given and working or not.

I'll redo this test (as 2 tests, one is testing the situation as we have now, i.e. the desired tip is reachable from within the depth argument, the second test will test if it is not reachable.)

Show 34 quoted lines
>
>>
>> Also most of the lines in this subshell seem to be breaking
>> &&-chain.
>
> Thanks for pointing that out, will fix while at it.
>
>>
>>
>>
>>> Correct the test by using test_line_count that exists in the test suite.
>>>
>>> Signed-off-by: Stefan Beller <sbeller@google.com>
>>> ---
>>>  t/t7406-submodule-update.sh | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
>>> index 88e9750..bd261ac 100755
>>> --- a/t/t7406-submodule-update.sh
>>> +++ b/t/t7406-submodule-update.sh
>>> @@ -846,9 +846,10 @@ test_expect_success 'submodule update clone shallow submodule' '
>>>       (cd super3 &&
>>>        sed -e "s#url = ../#url = file://$pwd/#" <.gitmodules >.gitmodules.tmp &&
>>>        mv -f .gitmodules.tmp .gitmodules &&
>>> -      git submodule update --init --depth=3
>>> +      git submodule update --init --depth=2
>>>        (cd submodule &&
>>> -       test 1 = $(git log --oneline | wc -l)
>>> +       git log --oneline >lines
>>> +       test_line_count = 2 lines
>>>        )
>>>  )
>>>  '
Previous: Stefan BellerNext: Junio C Hamano
Message 5 of 11 in “submodule update: allow '.' for branch value”
  1. 0/2 submodule update: allow '.' for branch valueStefan Beller, Jul 28, 2016
  2. 1/2 t7406: correct depth test in shallow testStefan Beller, Jul 28, 2016
  3. Junio C HamanoJul 28, 2016
  4. Stefan BellerJul 28, 2016
  5. Stefan BellerJul 28, 2016
  6. Junio C HamanoJul 28, 2016
  7. 2/2 submodule update: allow '.' for branch valueStefan Beller, Jul 28, 2016
  8. 2/2 submodule update: allow '.' for branch valueStefan Beller, Jul 28, 2016
  9. Junio C HamanoJul 28, 2016
  10. Stefan BellerJul 28, 2016
  11. Junio C HamanoJul 28, 2016

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.