Calvin Wan <calvinwan@xxxxxxxxxx> writes:
> This commit continues the previous work of updating the test suite to
> use `git submodule add` to create submodules instead of using `git add`
> to include embedded repositories. Specifically, in this commit we update
> test cases where expected diffs must change due to the presence of a
> .gitmodules file.
Adjusting the diff makes sense when the .gitmodules file is relevant to
the diff being tested.
> diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh
> index 61da7e3b94..a0f14db3d2 100755
> --- a/t/t3040-subprojects-basic.sh
> +++ b/t/t3040-subprojects-basic.sh
> @@ -19,11 +19,12 @@ test_expect_success 'setup: create subprojects' '
> ( cd sub2 && git init && : >Makefile && git add * &&
> git commit -q -m "subproject 2" ) &&
> git update-index --add sub1 &&
> - git add sub2 &&
> + git submodule add ./sub2 &&
> git commit -q -m "subprojects added" &&
> GIT_PRINT_SHA1_ELLIPSIS="yes" git diff-tree --abbrev=5 HEAD^ HEAD |cut -d" " -f-3,5- >current &&
> git branch save HEAD &&
> cat >expected <<-\EOF &&
> + :000000 100644 00000... A .gitmodules
> :000000 160000 00000... A sub1
> :000000 160000 00000... A sub2
> EOF
e.g. this change makes sense
> diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> index 2aa12243bd..f5074071a4 100755
> --- a/t/t4041-diff-submodule-option.sh
> +++ b/t/t4041-diff-submodule-option.sh
> @@ -50,9 +50,19 @@ test_expect_success 'setup' '
> '
>
> test_expect_success 'added submodule' '
> - git add sm1 &&
> + git submodule add ./sm1 &&
> + gitmodules_hash1=$(git rev-parse --short $(git hash-object .gitmodules)) &&
> git diff-index -p --submodule=log HEAD >actual &&
> cat >expected <<-EOF &&
> + diff --git a/.gitmodules b/.gitmodules
> + new file mode 100644
> + index 0000000..$gitmodules_hash1
> + --- /dev/null
> + +++ b/.gitmodules
> + @@ -0,0 +1,3 @@
> + +[submodule "sm1"]
> + + path = sm1
> + + url = ./sm1
> Submodule sm1 0000000...$head1 (new submodule)
> EOF
> test_cmp expected actual
But in this file and the next (t4041 and t4060), we are checking
submodule diffing behavior, so wouldn't it make sense to ignore
non-submodule changes in the diff?
E.g. we could have ignored .gitmodules during the diff like so
test_expect_success 'added submodule' '
git submodule add ./sm1 &&
gitmodules_hash1=$(git rev-parse --short $(git hash-object .gitmodules)) &&
- git diff-index -p --submodule=log HEAD >actual &&
+ git diff-index -p --submodule=log HEAD -- :!.gitmodules >actual &&
and then we wouldn't have to adjust the diff. That would be my preferred
approach, since it keeps the irrelevant details out of the test.
To play devil's advocate, there's a small integration test-style benefit
to testing both a regular file diff and a submodule diff together. I
haven't checked if these are the only files that are testing this, but
even if not, checking .gitmodules repeatedly seems like a suboptimal way
to do this.
> @@ -243,7 +244,7 @@ test_expect_success 'status -a clean (empty submodule dir)' '
> '
>
> cat >status_expect <<\EOF
> -AA .gitmodules
> +UU .gitmodules
> A sub1
> EOF
_Maybe_ this is worth modernizing too as a 'while we're at it' kind of
change, though it's far less important than the earlier patch (where the
setup actually touches the Git repo and submodules). Idk.
