On Tue Jan 9, 2024 at 2:50 PM IST, Christian Couder wrote: > 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". Updated in v2. > 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 I have updated the commit messages in v2 to address these points. > > 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. Yes, I just wanted to make sure the files were not tracked. However, I have updated these instaces to use the existing repo and use unique non-generic names for generating untracked files. > 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. I have moved these tests above the "--amend" related tests, which do not break the flow of the tests. > > +test_expect_success 'commit with -i' ' > > + echo content >bar && > > + git add bar && > > + git commit -m "bar" && > > Why is the above needed for this test? This was to make sure that the file is tracked, however, I realised that committing is not necessary, so I have updated this test to use existing files in repo and to not generate a new one. > > > + 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. Similar to above, I have updated this test to use existing files in repo and to not generate a new one. > > > + 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/ Done. > > > + 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. Thanks for the review!
