First about the commit subject: "t7501: Add tests for various index usages, -i and -o, of commit command." it should be shorter, shouldn't end with a "." and "Add" should be "add". On Tue, Jan 9, 2024 at 7:10 AM Ghanshyam Thakkar <shyamthakkar001@xxxxxxxxx> wrote: > > This commit adds tests for -i and -o flags of the commit command. It > includes testing -i with and without staged changes, testing -o with and > without staged changes, and testing -i and -o together. A few suggestions to make things a bit more clear: - tell that -i is a synonymous for --include and -o for --only - tell if there are already tests for these options - tell why the tests you add are worth it if tests for an option already exist > Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@xxxxxxxxx> > --- > t/t7501-commit-basic-functionality.sh | 90 +++++++++++++++++++++++++++ > 1 file changed, 90 insertions(+) > > diff --git a/t/t7501-commit-basic-functionality.sh b/t/t7501-commit-basic-functionality.sh > index 3d8500a52e..71dc52ce3a 100755 > --- a/t/t7501-commit-basic-functionality.sh > +++ b/t/t7501-commit-basic-functionality.sh > @@ -76,6 +76,96 @@ test_expect_success 'nothing to commit' ' > test_must_fail git commit -m initial > ' > > +test_expect_success 'commit with -i fails with untracked files' ' > + test_when_finished "rm -rf testdir" && > + git init testdir && > + echo content >testdir/file.txt && > + test_must_fail git -C testdir commit -i file.txt -m initial > +' Existing tests didn't need a repo, so I am not sure it's worth creating a testdir repo just for this test. Also I am not sure this is the best place in the test script to add -i and -o related tests. Here we are between a 'nothing to commit' test and a '--dry-run fails with nothing to commit' and then more 'nothing to commit' related tests. I think it would be better if all those 'nothing to commit' related tests could stay together. > +test_expect_success 'commit with -i' ' > + echo content >bar && > + git add bar && > + git commit -m "bar" && Why is the above needed for this test? > + echo content2 >bar && > + git commit -i bar -m "bar second" > +' > + > +test_expect_success 'commit with -i multiple files' ' > + test_when_finished "git reset --hard" && > + echo content >bar && > + echo content >baz && > + echo content >saz && > + git add bar baz saz && > + git commit -m "bar baz saz" && Not sure why the above is needed here too. > + echo content2 >bar && > + echo content2 >baz && > + echo content2 >saz && > + git commit -i bar saz -m "bar" && > + git diff --name-only >remaining && > + test_grep "baz" remaining > +' > + > +test_expect_success 'commit with -i includes staged changes' ' > + test_when_finished "git reset --hard" && > + echo content >bar && > + git add bar && > + git commit -m "bar" && > + > + echo content2 >bar && > + echo content2 >baz && > + git add baz && > + git commit -i bar -m "bar baz" && > + git diff --name-only >remaining && > + test_must_be_empty remaining && > + git diff --name-only --staged >remaining && > + test_must_be_empty remaining > +' > + > +test_expect_success 'commit with -o' ' > + echo content >bar && > + git add bar && > + git commit -m "bar" && > + echo content2 >bar && > + git commit -o bar -m "bar again" > +' > + > +test_expect_success 'commit with -o fails with untracked files' ' > + test_when_finished "rm -rf testdir" && > + git init testdir && > + echo content >testdir/bar && > + test_must_fail git -C testdir commit -o bar -m "bar" > +' > + > +test_expect_success 'commit with -o exludes staged changes' ' s/exludes/excludes/ > + test_when_finished "git reset --hard" && > + echo content >bar && > + echo content >baz && > + git add . && > + git commit -m "bar baz" && > + > + echo content2 >bar && > + echo content2 >baz && > + git add baz && > + git commit -o bar -m "bar" && > + git diff --name-only --staged >actual && > + test_grep "baz" actual > +' Thanks.
