On Thu, Jul 28, 2016 at 11:47 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote: > On Thu, Jul 28, 2016 at 11:39 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >> Stefan Beller <sbeller@xxxxxxxxxx> 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.) > >> >> 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@xxxxxxxxxx> >>> --- >>> 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 >>> ) >>> ) >>> ' -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html