diff mbox series

[v2,4/6] tests: use `git submodule add` and fix expected status

Message ID 20230228185642.2357806-4-calvinwan@google.com (mailing list archive)
State New, archived
Headers show
Series add: block invalid submodules | expand

Commit Message

Calvin Wan Feb. 28, 2023, 6:56 p.m. UTC 
From: Josh Steadmon <steadmon@google.com>

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.

In this commit, we update test cases where the expected status output
must change due to the presence of a .gitmodules file. We use the
pre-existing expected output as a template for cases where .gitmodules
has been modified but not committed.

Signed-off-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Calvin Wan <calvinwan@google.com>
---
 t/t4027-diff-submodule.sh |  17 +++--
 t/t7508-status.sh         | 134 ++++++++++++++++++++++++++++++++------
 2 files changed, 128 insertions(+), 23 deletions(-)

Comments

Glen Choo March 7, 2023, 12:15 a.m. UTC | #1 
Calvin Wan <calvinwan@google.com> writes:

> @@ -122,25 +123,30 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)'
>  '
>  
>  test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.gitmodules]' '
> +	git branch pristine-gitmodules &&
>  	git config diff.ignoreSubmodules dirty &&
>  	git diff HEAD >actual &&
>  	test_must_be_empty actual &&
>  	git config --add -f .gitmodules submodule.subname.ignore none &&
>  	git config --add -f .gitmodules submodule.subname.path sub &&
> +	git commit -m "Update .gitmodules" .gitmodules &&
>  	git diff HEAD >actual &&
>  	sed -e "1,/^@@/d" actual >actual.body &&
>  	expect_from_to >expect.body $subprev $subprev-dirty &&
>  	test_cmp expect.body actual.body &&
>  	git config -f .gitmodules submodule.subname.ignore all &&
>  	git config -f .gitmodules submodule.subname.path sub &&
> +	git commit -m "Update .gitmodules" .gitmodules &&
>  	git diff HEAD >actual2 &&
>  	test_must_be_empty actual2 &&
>  	git config -f .gitmodules submodule.subname.ignore untracked &&
> +	git commit -m "Update .gitmodules" .gitmodules &&
>  	git diff HEAD >actual3 &&
>  	sed -e "1,/^@@/d" actual3 >actual3.body &&
>  	expect_from_to >expect.body $subprev $subprev-dirty &&
>  	test_cmp expect.body actual3.body &&
>  	git config -f .gitmodules submodule.subname.ignore dirty &&
> +	git commit -m "Update .gitmodules" .gitmodules &&
>  	git diff HEAD >actual4 &&
>  	test_must_be_empty actual4 &&
>  	git config submodule.subname.ignore none &&
> @@ -152,7 +158,7 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)
>  	git config --remove-section submodule.subname &&
>  	git config --remove-section -f .gitmodules submodule.subname &&
>  	git config --unset diff.ignoreSubmodules &&
> -	rm .gitmodules
> +	git reset --hard pristine-gitmodules
>  '

This looks like the perfect use case for test_when_finished :)

> @@ -190,12 +196,15 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)'
>  test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match) [.gitmodules]' '
>  	git config --add -f .gitmodules submodule.subname.ignore all &&
>  	git config --add -f .gitmodules submodule.subname.path sub &&
> +	git commit -m "Update .gitmodules" .gitmodules &&
>  	git diff HEAD >actual2 &&
>  	test_must_be_empty actual2 &&
>  	git config -f .gitmodules submodule.subname.ignore untracked &&
> +	git commit -m "Update .gitmodules" .gitmodules &&
>  	git diff HEAD >actual3 &&
>  	test_must_be_empty actual3 &&
>  	git config -f .gitmodules submodule.subname.ignore dirty &&
> +	git commit -m "Update .gitmodules" .gitmodules &&
>  	git diff HEAD >actual4 &&
>  	test_must_be_empty actual4 &&
>  	git config submodule.subname.ignore none &&

Like the previous patch, I wonder a little whether we should be diffing
with :!.gitmodules, but at least here we are focused on diffing with
submodules in general (and not the specific "git diff --submodule="
behavior), so I thnk this is okay to keep.

> @@ -206,7 +215,7 @@ test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)
>  	test_cmp expect.body actual.body &&
>  	git config --remove-section submodule.subname &&
>  	git config --remove-section -f .gitmodules submodule.subname &&
> -	rm .gitmodules
> +	git reset --hard pristine-gitmodules

Ditto about test_when_finished.

>  '
>  
>  test_expect_success 'git diff between submodule commits' '
> @@ -243,7 +252,7 @@ test_expect_success 'git diff between submodule commits [.gitmodules]' '
>  	expect_from_to >expect.body $subtip $subprev &&
>  	git config --remove-section submodule.subname &&
>  	git config --remove-section -f .gitmodules submodule.subname &&
> -	rm .gitmodules
> +	git reset --hard pristine-gitmodules
>  '
>  

Ditto

> @@ -1152,8 +1156,37 @@ test_expect_success '.gitmodules ignore=untracked suppresses submodules with unt
>  	test_cmp expect output &&
>  	git config --add -f .gitmodules submodule.subname.ignore untracked &&
>  	git config --add -f .gitmodules submodule.subname.path sm &&
> +	cat > expect-modified-gitmodules << EOF &&
> +On branch main
> +Your branch and '\''upstream'\'' have diverged,
> +and have 2 and 2 different commits each, respectively.
> +  (use "git pull" to merge the remote branch into yours)
> +
> +Changes to be committed:
> +  (use "git restore --staged <file>..." to unstage)
> +	modified:   sm
> +
> +Changes not staged for commit:
> +  (use "git add <file>..." to update what will be committed)
> +  (use "git restore <file>..." to discard changes in working directory)
> +	modified:   .gitmodules
> +	modified:   dir1/modified
> +
> +Submodule changes to be committed:
> +
> +* sm $head...$new_head (1):
> +  > Add bar
> +
> +Untracked files:
> +  (use "git add <file>..." to include in what will be committed)
> +	dir1/untracked
> +	dir2/modified
> +	dir2/untracked
> +	untracked
> +
> +EOF
>  	git status >output &&
> -	test_cmp expect output &&
> +	test_cmp expect-modified-gitmodules output &&
>  	git config -f .gitmodules  --remove-section submodule.subname
>  '

That another giant snapshot makes me a bit wary, since it's harder to
tell whether the "modifed .gitmodules" and "unmodified .gitmodules" are
really checking the same things, but there might not be a way around it.
The following tests check various combinations of values (dirty,
untracked, etc) and sources "--ignore-submodules", ".git/config ignore="
and ".gitmodules ignore=". For the .gitmodules tests, we really do have
to modify .gitmodules to test that it gives us the behavior we want.

As a hack, we could preemptively modify .gitmodules, so that it's
modified in all of the snapshots we're diffing. That feels too hacky to
me, but maybe others think it's fine.

(Side note: I recall a previous conversation with Junio about how we
shouldn't be changing behavior based on .gitmodules. If we had that, we
wouldn't need to worry about this right now.)
diff mbox series

Patch

diff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh
index 40164ae07d..2ee9f18b38 100755
--- a/t/t4027-diff-submodule.sh
+++ b/t/t4027-diff-submodule.sh
@@ -18,7 +18,8 @@  test_expect_success setup '
 
 	test_tick &&
 	echo frotz >nitfol &&
-	git add nitfol sub &&
+	git add nitfol &&
+	git submodule add ./sub &&
 	git commit -m superproject &&
 
 	(
@@ -122,25 +123,30 @@  test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)'
 '
 
 test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.gitmodules]' '
+	git branch pristine-gitmodules &&
 	git config diff.ignoreSubmodules dirty &&
 	git diff HEAD >actual &&
 	test_must_be_empty actual &&
 	git config --add -f .gitmodules submodule.subname.ignore none &&
 	git config --add -f .gitmodules submodule.subname.path sub &&
+	git commit -m "Update .gitmodules" .gitmodules &&
 	git diff HEAD >actual &&
 	sed -e "1,/^@@/d" actual >actual.body &&
 	expect_from_to >expect.body $subprev $subprev-dirty &&
 	test_cmp expect.body actual.body &&
 	git config -f .gitmodules submodule.subname.ignore all &&
 	git config -f .gitmodules submodule.subname.path sub &&
+	git commit -m "Update .gitmodules" .gitmodules &&
 	git diff HEAD >actual2 &&
 	test_must_be_empty actual2 &&
 	git config -f .gitmodules submodule.subname.ignore untracked &&
+	git commit -m "Update .gitmodules" .gitmodules &&
 	git diff HEAD >actual3 &&
 	sed -e "1,/^@@/d" actual3 >actual3.body &&
 	expect_from_to >expect.body $subprev $subprev-dirty &&
 	test_cmp expect.body actual3.body &&
 	git config -f .gitmodules submodule.subname.ignore dirty &&
+	git commit -m "Update .gitmodules" .gitmodules &&
 	git diff HEAD >actual4 &&
 	test_must_be_empty actual4 &&
 	git config submodule.subname.ignore none &&
@@ -152,7 +158,7 @@  test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)
 	git config --remove-section submodule.subname &&
 	git config --remove-section -f .gitmodules submodule.subname &&
 	git config --unset diff.ignoreSubmodules &&
-	rm .gitmodules
+	git reset --hard pristine-gitmodules
 '
 
 test_expect_success 'git diff HEAD with dirty submodule (index, refs match)' '
@@ -190,12 +196,15 @@  test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)'
 test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match) [.gitmodules]' '
 	git config --add -f .gitmodules submodule.subname.ignore all &&
 	git config --add -f .gitmodules submodule.subname.path sub &&
+	git commit -m "Update .gitmodules" .gitmodules &&
 	git diff HEAD >actual2 &&
 	test_must_be_empty actual2 &&
 	git config -f .gitmodules submodule.subname.ignore untracked &&
+	git commit -m "Update .gitmodules" .gitmodules &&
 	git diff HEAD >actual3 &&
 	test_must_be_empty actual3 &&
 	git config -f .gitmodules submodule.subname.ignore dirty &&
+	git commit -m "Update .gitmodules" .gitmodules &&
 	git diff HEAD >actual4 &&
 	test_must_be_empty actual4 &&
 	git config submodule.subname.ignore none &&
@@ -206,7 +215,7 @@  test_expect_success 'git diff HEAD with dirty submodule (untracked, refs match)
 	test_cmp expect.body actual.body &&
 	git config --remove-section submodule.subname &&
 	git config --remove-section -f .gitmodules submodule.subname &&
-	rm .gitmodules
+	git reset --hard pristine-gitmodules
 '
 
 test_expect_success 'git diff between submodule commits' '
@@ -243,7 +252,7 @@  test_expect_success 'git diff between submodule commits [.gitmodules]' '
 	expect_from_to >expect.body $subtip $subprev &&
 	git config --remove-section submodule.subname &&
 	git config --remove-section -f .gitmodules submodule.subname &&
-	rm .gitmodules
+	git reset --hard pristine-gitmodules
 '
 
 test_expect_success 'git diff (empty submodule dir)' '
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 2b7ef6c41a..5808339997 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -892,7 +892,7 @@  test_expect_success 'setup status submodule summary' '
 		git add foo &&
 		git commit -m "Add foo"
 	) &&
-	git add sm
+	git submodule add ./sm
 '
 
 test_expect_success 'status submodule summary is disabled by default' '
@@ -904,6 +904,7 @@  and have 1 and 2 different commits each, respectively.
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
+	new file:   .gitmodules
 	new file:   dir2/added
 	new file:   sm
 
@@ -931,6 +932,7 @@  test_expect_success 'status --untracked-files=all does not show submodule' '
 '
 
 cat >expect <<EOF
+A  .gitmodules
  M dir1/modified
 A  dir2/added
 A  sm
@@ -961,6 +963,7 @@  and have 1 and 2 different commits each, respectively.
 
 Changes to be committed:
   (use "git restore --staged <file>..." to unstage)
+	new file:   .gitmodules
 	new file:   dir2/added
 	new file:   sm
 
@@ -998,6 +1001,7 @@  test_expect_success 'commit with submodule summary ignores status.displayComment
 '
 
 cat >expect <<EOF
+A  .gitmodules
  M dir1/modified
 A  dir2/added
 A  sm
@@ -1068,6 +1072,7 @@  and have 2 and 2 different commits each, respectively.
 
 Changes to be committed:
   (use "git restore --source=HEAD^1 --staged <file>..." to unstage)
+	new file:   .gitmodules
 	new file:   dir2/added
 	new file:   sm
 
@@ -1134,7 +1139,6 @@  Submodule changes to be committed:
 
 Untracked files:
   (use "git add <file>..." to include in what will be committed)
-	.gitmodules
 	dir1/untracked
 	dir2/modified
 	dir2/untracked
@@ -1152,8 +1156,37 @@  test_expect_success '.gitmodules ignore=untracked suppresses submodules with unt
 	test_cmp expect output &&
 	git config --add -f .gitmodules submodule.subname.ignore untracked &&
 	git config --add -f .gitmodules submodule.subname.path sm &&
+	cat > expect-modified-gitmodules << EOF &&
+On branch main
+Your branch and '\''upstream'\'' have diverged,
+and have 2 and 2 different commits each, respectively.
+  (use "git pull" to merge the remote branch into yours)
+
+Changes to be committed:
+  (use "git restore --staged <file>..." to unstage)
+	modified:   sm
+
+Changes not staged for commit:
+  (use "git add <file>..." to update what will be committed)
+  (use "git restore <file>..." to discard changes in working directory)
+	modified:   .gitmodules
+	modified:   dir1/modified
+
+Submodule changes to be committed:
+
+* sm $head...$new_head (1):
+  > Add bar
+
+Untracked files:
+  (use "git add <file>..." to include in what will be committed)
+	dir1/untracked
+	dir2/modified
+	dir2/untracked
+	untracked
+
+EOF
 	git status >output &&
-	test_cmp expect output &&
+	test_cmp expect-modified-gitmodules output &&
 	git config -f .gitmodules  --remove-section submodule.subname
 '
 
@@ -1163,7 +1196,7 @@  test_expect_success '.git/config ignore=untracked suppresses submodules with unt
 	git config --add submodule.subname.ignore untracked &&
 	git config --add submodule.subname.path sm &&
 	git status >output &&
-	test_cmp expect output &&
+	test_cmp expect-modified-gitmodules output &&
 	git config --remove-section submodule.subname &&
 	git config --remove-section -f .gitmodules submodule.subname
 '
@@ -1180,7 +1213,7 @@  test_expect_success '.gitmodules ignore=dirty suppresses submodules with untrack
 	git config --add -f .gitmodules submodule.subname.ignore dirty &&
 	git config --add -f .gitmodules submodule.subname.path sm &&
 	git status >output &&
-	test_cmp expect output &&
+	test_cmp expect-modified-gitmodules output &&
 	git config -f .gitmodules  --remove-section submodule.subname
 '
 
@@ -1190,7 +1223,7 @@  test_expect_success '.git/config ignore=dirty suppresses submodules with untrack
 	git config --add submodule.subname.ignore dirty &&
 	git config --add submodule.subname.path sm &&
 	git status >output &&
-	test_cmp expect output &&
+	test_cmp expect-modified-gitmodules output &&
 	git config --remove-section submodule.subname &&
 	git config -f .gitmodules  --remove-section submodule.subname
 '
@@ -1205,7 +1238,7 @@  test_expect_success '.gitmodules ignore=dirty suppresses submodules with modifie
 	git config --add -f .gitmodules submodule.subname.ignore dirty &&
 	git config --add -f .gitmodules submodule.subname.path sm &&
 	git status >output &&
-	test_cmp expect output &&
+	test_cmp expect-modified-gitmodules output &&
 	git config -f .gitmodules  --remove-section submodule.subname
 '
 
@@ -1215,7 +1248,7 @@  test_expect_success '.git/config ignore=dirty suppresses submodules with modifie
 	git config --add submodule.subname.ignore dirty &&
 	git config --add submodule.subname.path sm &&
 	git status >output &&
-	test_cmp expect output &&
+	test_cmp expect-modified-gitmodules output &&
 	git config --remove-section submodule.subname &&
 	git config -f .gitmodules  --remove-section submodule.subname
 '
@@ -1245,7 +1278,6 @@  Submodule changes to be committed:
 
 Untracked files:
   (use "git add <file>..." to include in what will be committed)
-	.gitmodules
 	dir1/untracked
 	dir2/modified
 	dir2/untracked
@@ -1259,8 +1291,39 @@  EOF
 test_expect_success ".gitmodules ignore=untracked doesn't suppress submodules with modified content" '
 	git config --add -f .gitmodules submodule.subname.ignore untracked &&
 	git config --add -f .gitmodules submodule.subname.path sm &&
+	cat > expect-modified-gitmodules << EOF &&
+On branch main
+Your branch and '\''upstream'\'' have diverged,
+and have 2 and 2 different commits each, respectively.
+  (use "git pull" to merge the remote branch into yours)
+
+Changes to be committed:
+  (use "git restore --staged <file>..." to unstage)
+	modified:   sm
+
+Changes not staged for commit:
+  (use "git add <file>..." to update what will be committed)
+  (use "git restore <file>..." to discard changes in working directory)
+  (commit or discard the untracked or modified content in submodules)
+	modified:   .gitmodules
+	modified:   dir1/modified
+	modified:   sm (modified content)
+
+Submodule changes to be committed:
+
+* sm $head...$new_head (1):
+  > Add bar
+
+Untracked files:
+  (use "git add <file>..." to include in what will be committed)
+	dir1/untracked
+	dir2/modified
+	dir2/untracked
+	untracked
+
+EOF
 	git status >output &&
-	test_cmp expect output &&
+	test_cmp expect-modified-gitmodules output &&
 	git config -f .gitmodules  --remove-section submodule.subname
 '
 
@@ -1270,7 +1333,7 @@  test_expect_success ".git/config ignore=untracked doesn't suppress submodules wi
 	git config --add submodule.subname.ignore untracked &&
 	git config --add submodule.subname.path sm &&
 	git status >output &&
-	test_cmp expect output &&
+	test_cmp expect-modified-gitmodules output &&
 	git config --remove-section submodule.subname &&
 	git config -f .gitmodules  --remove-section submodule.subname
 '
@@ -1306,7 +1369,6 @@  Submodules changed but not updated:
 
 Untracked files:
   (use "git add <file>..." to include in what will be committed)
-	.gitmodules
 	dir1/untracked
 	dir2/modified
 	dir2/untracked
@@ -1318,10 +1380,45 @@  EOF
 '
 
 test_expect_success ".gitmodules ignore=untracked doesn't suppress submodule summary" '
+	cat > expect-modified-gitmodules << EOF &&
+On branch main
+Your branch and '\''upstream'\'' have diverged,
+and have 2 and 2 different commits each, respectively.
+  (use "git pull" to merge the remote branch into yours)
+
+Changes to be committed:
+  (use "git restore --staged <file>..." to unstage)
+	modified:   sm
+
+Changes not staged for commit:
+  (use "git add <file>..." to update what will be committed)
+  (use "git restore <file>..." to discard changes in working directory)
+	modified:   .gitmodules
+	modified:   dir1/modified
+	modified:   sm (new commits)
+
+Submodule changes to be committed:
+
+* sm $head...$new_head (1):
+  > Add bar
+
+Submodules changed but not updated:
+
+* sm $new_head...$head2 (1):
+  > 2nd commit
+
+Untracked files:
+  (use "git add <file>..." to include in what will be committed)
+	dir1/untracked
+	dir2/modified
+	dir2/untracked
+	untracked
+
+EOF
 	git config --add -f .gitmodules submodule.subname.ignore untracked &&
 	git config --add -f .gitmodules submodule.subname.path sm &&
 	git status >output &&
-	test_cmp expect output &&
+	test_cmp expect-modified-gitmodules output &&
 	git config -f .gitmodules  --remove-section submodule.subname
 '
 
@@ -1331,7 +1428,7 @@  test_expect_success ".git/config ignore=untracked doesn't suppress submodule sum
 	git config --add submodule.subname.ignore untracked &&
 	git config --add submodule.subname.path sm &&
 	git status >output &&
-	test_cmp expect output &&
+	test_cmp expect-modified-gitmodules output &&
 	git config --remove-section submodule.subname &&
 	git config -f .gitmodules  --remove-section submodule.subname
 '
@@ -1344,7 +1441,7 @@  test_expect_success ".gitmodules ignore=dirty doesn't suppress submodule summary
 	git config --add -f .gitmodules submodule.subname.ignore dirty &&
 	git config --add -f .gitmodules submodule.subname.path sm &&
 	git status >output &&
-	test_cmp expect output &&
+	test_cmp expect-modified-gitmodules output &&
 	git config -f .gitmodules  --remove-section submodule.subname
 '
 
@@ -1354,7 +1451,7 @@  test_expect_success ".git/config ignore=dirty doesn't suppress submodule summary
 	git config --add submodule.subname.ignore dirty &&
 	git config --add submodule.subname.path sm &&
 	git status >output &&
-	test_cmp expect output &&
+	test_cmp expect-modified-gitmodules output &&
 	git config --remove-section submodule.subname &&
 	git config -f .gitmodules  --remove-section submodule.subname
 '
@@ -1387,7 +1484,6 @@  cat > expect << EOF
 ;
 ; Untracked files:
 ;   (use "git add <file>..." to include in what will be committed)
-;	.gitmodules
 ;	dir1/untracked
 ;	dir2/modified
 ;	dir2/untracked
@@ -1420,7 +1516,6 @@  Changes not staged for commit:
 
 Untracked files:
   (use "git add <file>..." to include in what will be committed)
-	.gitmodules
 	dir1/untracked
 	dir2/modified
 	dir2/untracked
@@ -1446,11 +1541,11 @@  Changes to be committed:
 Changes not staged for commit:
   (use "git add <file>..." to update what will be committed)
   (use "git restore <file>..." to discard changes in working directory)
+	modified:   .gitmodules
 	modified:   dir1/modified
 
 Untracked files:
   (use "git add <file>..." to include in what will be committed)
-	.gitmodules
 	dir1/untracked
 	dir2/modified
 	dir2/untracked
@@ -1566,6 +1661,7 @@  Changes to be committed:
 Changes not staged for commit:
   (use "git add <file>..." to update what will be committed)
   (use "git restore <file>..." to discard changes in working directory)
+	modified:   .gitmodules
 	modified:   dir1/modified
 
 Untracked files not listed (use -u option to show untracked files)