| Message ID | 20240113042254.38602-2-shyamthakkar001@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | t7501: add tests for --include, --only, --signoff | expand |
Thanks. "git am" auto-fixed these whitespace breakage. .git/rebase-apply/patch:25: trailing whitespace. .git/rebase-apply/patch:38: trailing whitespace. # .git/rebase-apply/patch:40: trailing whitespace. # and is not an endorsement to the current behavior, and we may .git/rebase-apply/patch:56: trailing whitespace. do .git/rebase-apply/patch:82: trailing whitespace. warning: 5 lines applied after fixing whitespace errors. Applying: t7501: add tests for --include and --only
Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes: > +test_expect_success 'fail to commit untracked pathspec (even with --include/--only)' ' "untracked pathspec" is a nonsense word. We do not track pathspec; you may use pathspec to specify which path(s) to work on. I think this is more about untracked files being ignored for the purpose of "-i" and "-o". You used to call this "untracked file", which probably makes more sense. > + # TODO: as for --include, the below command will fail because nothing is > + # staged. If something was staged, it would not fail even if the > + # pathspec was untracked (however, pathspec will remain untracked and > + # staged changes would be committed.) In either cases, no error is Both uses of "pathspec" here are nonsense. Saying "even though the pathspec does not match any tracked path" is OK. "(however, the untracked path that match the pathspec are not added and only the changes in the index gets commit)" is also OK. > + # returned to stderr like in (-o and without -o/-i) cases. In a > + # similar manner, "git add -u baz" also does not error out. > + # > + # Therefore, the below test is just to document the current behavior > + # and is not an endorsement to the current behavior, and we may > + # want to fix this. And when that happens, this test should be > + # updated accordingly. OK. > @@ -117,6 +143,55 @@ test_expect_success '--long with stuff to commit returns ok' ' > git commit -m next -a --long > ' > > +for opt in "" "-o" "--only" > +do > + test_expect_success 'exclude additional staged changes when given pathspec' ' > + echo content >>file && > + echo content >>baz && > + git add baz && > + git commit $opt -m "file" file && > + > + git diff --name-only >actual && > + test_must_be_empty actual && > + > + git diff --name-only --staged >actual && > + test_cmp - actual <<-EOF && > + baz > + EOF It is probably more common to say test_write_lines baz >expect && git diff --name-only --cached >actual && test_cmp expect actual especially when the expected pattern is a file with short lines, instead of here-text. It makes it easier to debug tests, too, because after "sh t7501-*.sh -i" fails, you can go to the trash directory and the expectation as well as the actual output are there in files. > + git diff --name-only HEAD^ HEAD >actual && > + test_cmp - actual <<-EOF > + file > + EOF Ditto. This tests the most important aspect of "-o"; even when the index has other changes relative to HEAD already, they are skipped over and only the changes to paths that match the given pathspec are committed, and this test demonstrates it. Well done. > +test_expect_success '-i/--include includes staged changes' ' > + echo content >>file && > + echo content >>baz && > + git add file && > + > + # baz is in the index, therefore, it will be committed > + git commit --include -m "file and baz" baz && > + > + git diff --name-only HEAD >remaining && > + test_must_be_empty remaining && > + > + git diff --name-only HEAD^ HEAD >changes && > + test_cmp - changes <<-EOF > + baz > + file > + EOF Again with "test_write_lines baz file >expect", this test becomes a bit shorter. This tests the most important aspect of "-i"; changes to the paths that match the given pathspec are added to the index, and recorded together with changes added to the index already to the commit. Well done. > +' > + > +test_expect_success '--include and --only do not mix' ' > + test_when_finished "git reset --hard" && > + echo content >>file && > + echo content >>baz && > + test_must_fail git commit --include --only -m "file baz" file baz 2>actual && > + test_grep -e "fatal: options .-i/--include. and .-o/--only. cannot be used together" actual > +' > + > test_expect_success 'commit message from non-existing file' ' > echo more bongo: bongo bongo bongo bongo >file && > test_must_fail git commit -F gah -a Very nicely done. Thanks.
diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh index 3d8500a52e..3e18b879b5 100755 --- a/t/t7501-commit-basic-functionality.sh +++ b/t/t7501-commit-basic-functionality.sh @@ -3,7 +3,7 @@ # Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com> # -# FIXME: Test the various index usages, -i and -o, test reflog, +# FIXME: Test the various index usages, test reflog, # signoff test_description='git commit' @@ -92,6 +92,32 @@ test_expect_success '--long fails with nothing to commit' ' test_must_fail git commit -m initial --long ' +test_expect_success 'fail to commit untracked pathspec (even with --include/--only)' ' + echo content >baz && + error="error: pathspec .baz. did not match any file(s) known to git" && + + test_must_fail git commit -m "baz" baz 2>err && + test_grep -e "$error" err && + + test_must_fail git commit --only -m "baz" baz 2>err && + test_grep -e "$error" err && + + # TODO: as for --include, the below command will fail because nothing is + # staged. If something was staged, it would not fail even if the + # pathspec was untracked (however, pathspec will remain untracked and + # staged changes would be committed.) In either cases, no error is + # returned to stderr like in (-o and without -o/-i) cases. In a + # similar manner, "git add -u baz" also does not error out. + # + # Therefore, the below test is just to document the current behavior + # and is not an endorsement to the current behavior, and we may + # want to fix this. And when that happens, this test should be + # updated accordingly. + + test_must_fail git commit --include -m "baz" baz 2>err && + test_must_be_empty err +' + test_expect_success 'setup: non-initial commit' ' echo bongo bongo bongo >file && git commit -m next -a @@ -117,6 +143,55 @@ test_expect_success '--long with stuff to commit returns ok' ' git commit -m next -a --long ' +for opt in "" "-o" "--only" +do + test_expect_success 'exclude additional staged changes when given pathspec' ' + echo content >>file && + echo content >>baz && + git add baz && + git commit $opt -m "file" file && + + git diff --name-only >actual && + test_must_be_empty actual && + + git diff --name-only --staged >actual && + test_cmp - actual <<-EOF && + baz + EOF + + git diff --name-only HEAD^ HEAD >actual && + test_cmp - actual <<-EOF + file + EOF + ' +done + +test_expect_success '-i/--include includes staged changes' ' + echo content >>file && + echo content >>baz && + git add file && + + # baz is in the index, therefore, it will be committed + git commit --include -m "file and baz" baz && + + git diff --name-only HEAD >remaining && + test_must_be_empty remaining && + + git diff --name-only HEAD^ HEAD >changes && + test_cmp - changes <<-EOF + baz + file + EOF +' + +test_expect_success '--include and --only do not mix' ' + test_when_finished "git reset --hard" && + echo content >>file && + echo content >>baz && + test_must_fail git commit --include --only -m "file baz" file baz 2>actual && + test_grep -e "fatal: options .-i/--include. and .-o/--only. cannot be used together" actual +' + test_expect_success 'commit message from non-existing file' ' echo more bongo: bongo bongo bongo bongo >file && test_must_fail git commit -F gah -a
Add tests for --only (-o) and --include (-i). This include testing with or without staged changes for both -i and -o. Also to test for committing untracked files with -i, -o and without -i/-o. Some tests already exist in t7501 for testing --only, however, it is tested in combination with --amend and --allow-empty and on to-be-born branch. The addition of these tests check, when the pathspec is provided, that only the files matching the pathspec get committed. This behavior is same when we provide --only and checked by the tests. (as --only is the default mode of operation when pathspec is provided.) As for --include, there is no prior test for checking if --include also commits staged changes, thus add test for that. Along with the test also document a potential bug, in which, when provided with -i and an untracked pathspec, commit does not fail if there are staged changes. And when there are no staged changes commit fails. However, no error is returned to stderr in either of the cases. Thus, these tests belong in t7501 with other similar existing tests, as described in the case of --only. And also add a test for checking incompatibilty when using -o and -i together. Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> --- t/t7501-commit-basic-functionality.sh | 77 ++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-)