Re: [PATCH 4/4] submodule: add more tests for recursive submodule behavior

On Thu, Mar 24, 2016 at 5:25 PM, Eric Sunshine [off-list ref] wrote:

On Thu, Mar 24, 2016 at 7:34 PM, Stefan Beller [off-list ref] wrote:

quoted

This adds a test for "submodule update", wich calls "submodule update"

s/wich/which/

quoted

from an untracked repository in the superproject. When doing creating

Grammo: "doing creating"

quoted

the parent patch a similar test failed for "submodule sync", but
all tests passed for "submodule update". It took me a long time
to figure out this was a difference in test coverage instead of
commands behaving differently. Let's improve the test coverage such
to make it a better place.

When trying to fix the issue in the parent patch I could get
the test suite passing when removing the $@ argument from module_list
in the sync command. This also indicates a low test coverage, so
fix that.

These two paragraphs are almost entirely commentary, thus probably
belong below the "---" line. I'm having a difficult time trying to
decipher from this commit message what this patch is actually about.
Perhaps the commit message could do a better job explaining exactly
what shortcoming(s) it's addressing.

The tests on submodule commands executed not from the top level are very sparse.
I had a hard time to developing patches 1 and 2.
And I felt

    "This patch adds more test coverage."

is not a sufficient commit message.

The current tests have found issues, which
lead to fixing them in patch 1,2, but I think only by accident, as one command
(sync) was testing from the non root. By having more test coverage it
is easier to
have a guess what is wrong with the code.

Any hint on how to write that into a commit message without being commentatory
would be great!

quoted

Signed-off-by: Stefan Beller <redacted>
---

diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh

@@ -774,4 +774,16 @@ test_expect_success 'submodule update --recursive drops module name before recur
+test_expect_success 'submodule update --recursive works from subdirectory' '
+       (cd super2 &&
+        (cd deeper/submodule/subsubmodule &&
+         git checkout HEAD^
+        ) &&

Maybe use -C and drop the sub-subshell:

    git -C deeper/submodule/subsubmodule checkout HEAD^

ok

quoted

+        mkdir untracked &&
+        cd untracked &&
+        git submodule update --recursive >actual &&
+        test_i18ngrep "Submodule path .../deeper/submodule/subsubmodule.: checked out" actual
+       )
+'