Eric,Thanks! I have little confuse about I can use` test_when_finished "git switch master" `, but I can't use` test_when_finished "git switch -" `, why？ Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 于2021年1月16日周六 下午3:13写道： > > On Thu, Jan 14, 2021 at 7:22 AM 阿德烈 via GitGitGadget > <gitgitgadget@xxxxxxxxx> wrote: > > In order to provide users a better experience > > when viewing information about files in the index > > and the working tree, the `--dedup` option will suppress > > some duplicate options under some conditions. > > [...] > > I have a few very minor comments alongside Junio's review comments... > > > Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx> > > --- > > diff --git a/t/t3012-ls-files-dedup.sh b/t/t3012-ls-files-dedup.sh > > @@ -0,0 +1,54 @@ > > +test_description='git ls-files --dedup test. > > + > > +This test prepares the following in the cache: > > + > > + a.txt - a file(base) > > + a.txt - a file(master) > > + a.txt - a file(dev) > > + b.txt - a file > > + delete.txt - a file > > + expect1 - a file > > + expect2 - a file > > + > > +' > > This test script description is outdated now. Perhaps shorten it to: > > test_description='ls-files dedup tests' > > Or, it might be suitable to simply add the new test to the existing > t3004-ls-files-basic.sh instead. > > > +test_expect_success 'setup' ' > > + > a.txt && > > + > b.txt && > > + > delete.txt && > > + cat >expect1<<-\EOF && > > Style nits: no space after redirection operator and a space before > redirection operator: > > >a.txt && > >b.txt && > >delete.txt && > cat >expect1 <<-\EOF && > > > + cat >expect2<<-EOF && > > Nit: missing the backslash (and wrong spacing): > > cat >expect2 <<-\EOF && > > > + echo a>a.txt && > > + echo b>b.txt && > > Style: > > 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 && > > If someone adds a new test after this test, then that new test will > run in the "dev" branch, which might be unexpected or undesirable. It > often is a good idea to ensure that tests do certain types of cleanup > to avoid breaking subsequent tests. Here, it would be a good idea to > ensure that the test switches back to the original branch when it > finishes (regardless of whether it finishes successfully or > unsuccessfully). > > git switch -c dev && > test_when_finished "git switch master" && > > Or you could use `git switch -` if you don't want to hard-code the > name "master" in the test (since there has been effort lately to > remove that name from tests. > > > + echo change >a.txt && > > + git add a.txt && > > + git commit -m dev:1 && > > + test_must_fail git merge master && > > + git ls-files -t --dedup >actual1 && > > + test_cmp expect1 actual1 && > > + rm delete.txt && > > + git ls-files -d -m -t --dedup >actual2 && > > + test_cmp expect2 actual2 > > We usually don't bother giving temporary files unique names like > "actual1" and "actual2" unless those files must exist at the same > time. This is because unique names like this may confuse readers into > wondering if there is some hidden interdependency between the files. > In this case, the files don't need to exist at the same time, so it > may be better simply to use the names "actual" and "expect", like > this: > > ...other stuff... > cat >expect <<-\EOF && > ... > EOF > git ls-files -t --dedup >actual && > test_cmp expect actual && > rm delete.txt && > cat >expect <<-\EOF && > ... > EOF > git ls-files -d -m -t --dedup >actual && > test_cmp expect actual > > (It also has the benefit that the "expect" content is closer to the > place where it is actually used, which may make it a bit easier for a > person reading the test to understand what is supposed to be > produced.)
