Junio C Hamano <gitster@xxxxxxxxx> 于2021年1月18日周一 上午7:34写道: > > "ZheNing Hu via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > > diff --git a/t/t3012-ls-files-dedup.sh b/t/t3012-ls-files-dedup.sh > > new file mode 100755 > > index 00000000000..75877255c2c > > --- /dev/null > > +++ b/t/t3012-ls-files-dedup.sh > > @@ -0,0 +1,57 @@ > > +#!/bin/sh > > + > > +test_description='git ls-files --deduplicate test' > > + > > +. ./test-lib.sh > > We should already have a ls-files test so that we can add a handful > new tests to it, instead of dedicating a whole new test script. > Fine,It might be easier for me to write a test file myself for the time being. But I will learn slowly. > Also, don't do everything in a single 'setup'. There are various > scenarios you want to make sure ls-files to work (grep for ls-files > in the following you added---I count 4 of them), and when a future > developer touches the code, he or she may break one but not other > three. The purpose you write tests is to protect your new feature > from such a developer *AND* help such a developer to debug and fix > his or her changes. For that, it would be a lot more sensible to > have one set-up that is common, and then four separate tests. > > > +test_expect_success 'setup' ' > > + >a.txt && > > + >b.txt && > > + >delete.txt && > > + git add a.txt b.txt delete.txt && > > + git commit -m master:1 && > > Needless use of the word "master". Observe what is going on in the > project around you and avoid stepping other peoples' toes. One of > the ongoing effort is to grep for the phrase master in t/ directory > and examine what happens when the default initial branch name > becomes something other than 'master', so adding a needless hit like > this is most unwelcome. > Well, I will try my best to use less "master". > > + echo a >a.txt && > > + echo b >b.txt && > > + echo delete >delete.txt && > > + git add a.txt b.txt delete.txt && > > + git commit -m master:2 && > > > + git checkout HEAD~ && > > + git switch -c dev && > > Needless mixture of checkout/switch. If you switch branches using > "git checkout", for example, consistently do so, i.e. > > git checkout -b dev HEAD~1 > > It's not like these new tests are to test checkout and switch; your > mission is to protect "ls-files --dedup" feature here. > > > + test_when_finished "git switch master" && > > + echo change >a.txt && > > + git add a.txt && > > + git commit -m dev:1 && > > I'd consider all of the above to be 'setup' that is common for > subsequent tests. It may make sense to actually do everything > on the initial branch, i.e. after creating two commits, do > I understand it now...setup is for serve as a basis for other tests. > git tag tip && > git reset --hard HEAD^ && > echo change >a.txt && > git commit -a -m side && > git tag side > > You are always on the initial branch without ever switching, so > there is no need for the when_finished stuff. > > Then the first of your test is to show the index with conflicts. > > > + test_must_fail git merge master && > > This will become "git merge tip" instead of 'master'. > use tag instead of use branch name... > > + git ls-files --deduplicate >actual && > > + cat >expect <<-\EOF && > > + a.txt > > + b.txt > > + delete.txt > > + EOF > > + test_cmp expect actual && > > And up to this point is the first test after 'setup'. > > The next test should begin with: > > git reset --hard side && > test_must_fail git merge tip && > > so that even when the first test is skipped, or left unmerged, > you'll begin with a known state. > Well,I understand now that the a test_success should allow other programmers to skip this test,so that we should reset to a known state at the beginning of each test. > > + rm delete.txt && > > + git ls-files -d -m --deduplicate >actual && > > + cat >expect <<-\EOF && > > + a.txt > > + delete.txt > > + EOF > > + test_cmp expect actual && > > + git ls-files -d -m -t --deduplicate >actual && > > + cat >expect <<-\EOF && > > + C a.txt > > + C a.txt > > + C a.txt > > + R delete.txt > > + C delete.txt > > + EOF > > + test_cmp expect actual && > > + git ls-files -d -m -c --deduplicate >actual && > > + cat >expect <<-\EOF && > > + a.txt > > + b.txt > > + delete.txt > > + EOF > > + test_cmp expect actual && > > These three can be kept in the same test_expect_success, as they are > exercising read-only operation on the same state but with different > display options. > indeed so. > But in this case, the preparation is not too tedious (just a failed > merge plus a deletion), so you probably would prefer to split it > into 3 independent tests---that may make it more helpful to future > developers. > Thanks:) > > + git merge --abort > > +' > > +test_done