"ZheNing Hu via GitGitGadget" [off-list ref] writes:
quoted hunk
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.
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.
+ 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
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'.
+ 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.
+ 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.
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.
+ git merge --abort
+'
+test_done
Junio C Hamano [off-list ref] 于2021年1月18日周一 上午7:34写道：
"ZheNing Hu via GitGitGadget" [off-list ref] writes:
quoted
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.
quoted
+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".
quoted
+ 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 &&
quoted
+ 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.
quoted
+ 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.
quoted
+ test_must_fail git merge master &&
This will become "git merge tip" instead of 'master'.
use tag instead of use branch name...
quoted
+ 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.
quoted
+ 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:)
quoted
+ git merge --abort
+'
+test_done
Hi,Junio!
Here I am thinking about the role of this "--deduplicate" is to
suppress duplicate filenames rather than duplicate entries. Do you
think I should modify this sentence?
quoted
OPT_BOOL(0,"deduplicate",&skipping_duplicates,N_("suppress duplicate entries")),
胡哲宁 [off-list ref] 于2021年1月18日周一 下午12:09写道：
Junio C Hamano [off-list ref] 于2021年1月18日周一 上午7:34写道：
quoted
"ZheNing Hu via GitGitGadget" [off-list ref] writes:
quoted
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.
quoted
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.
quoted
+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".
quoted
quoted
+ 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 &&
quoted
+ 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.
quoted
+ 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.
quoted
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.
quoted
+ test_must_fail git merge master &&
This will become "git merge tip" instead of 'master'.
use tag instead of use branch name...
quoted
quoted
+ 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.
quoted
quoted
+ 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.
quoted
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:)
quoted
quoted
+ git merge --abort
+'
+test_done