On Thu, Mar 24, 2016 at 5:25 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Thu, Mar 24, 2016 at 7:34 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
>> This adds a test for "submodule update", wich calls "submodule update"
>
> s/wich/which/
>
>> from an untracked repository in the superproject. When doing creating
>
> Grammo: "doing creating"
>
>> 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!
>
>> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
>> ---
>> 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
>
>> + mkdir untracked &&
>> + cd untracked &&
>> + git submodule update --recursive >actual &&
>> + test_i18ngrep "Submodule path .../deeper/submodule/subsubmodule.: checked out" actual
>> + )
>> +'
--
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