Re: [PATCH v2 2/6] test-reach: add run_three_modes method
Junio C Hamano writes: > I also noticed that 2/6 made "commti_contains --tag" enclosed in dq > pair for one test, but the next test after it has the identical one. > > Here is what I queued in the meantime. > ... And of course, I find out that 3/6 needs a matching update after I've almost finished day's integration cycle, and need to redo the whole thing X-<. Here is a squashable update for 3/6 to match the proposed change. -- >8 -- Subject: fixup! test-reach: add rev-list tests t/t6600-test-reach.sh | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh index 990ab56e7a..cf9179bdb8 100755 --- a/t/t6600-test-reach.sh +++ b/t/t6600-test-reach.sh @@ -252,7 +252,7 @@ test_expect_success 'rev-list: basic topo-order' ' commit-6-2 commit-5-2 commit-4-2 commit-3-2 commit-2-2 commit-1-2 \ commit-6-1 commit-5-1 commit-4-1 commit-3-1 commit-2-1 commit-1-1 \ >expect && - run_three_modes "git rev-list --topo-order commit-6-6" + run_three_modes git rev-list --topo-order commit-6-6 ' test_expect_success 'rev-list: first-parent topo-order' ' @@ -264,7 +264,7 @@ test_expect_success 'rev-list: first-parent topo-order' ' commit-6-2 \ commit-6-1 commit-5-1 commit-4-1 commit-3-1 commit-2-1 commit-1-1 \ >expect && - run_three_modes "git rev-list --first-parent --topo-order commit-6-6" + run_three_modes git rev-list --first-parent --topo-order commit-6-6 ' test_expect_success 'rev-list: range topo-order' ' @@ -276,7 +276,7 @@ test_expect_success 'rev-list: range topo-order' ' commit-6-2 commit-5-2 commit-4-2 \ commit-6-1 commit-5-1 commit-4-1 \ >expect && - run_three_modes "git rev-list --topo-order commit-3-3..commit-6-6" + run_three_modes git rev-list --topo-order commit-3-3..commit-6-6 ' test_expect_success 'rev-list: range topo-order' ' @@ -288,7 +288,7 @@ test_expect_success 'rev-list: range topo-order' ' commit-6-2 commit-5-2 commit-4-2 \ commit-6-1 commit-5-1 commit-4-1 \ >expect && - run_three_modes "git rev-list --topo-order commit-3-8..commit-6-6" + run_three_modes git rev-list --topo-order commit-3-8..commit-6-6 ' test_expect_success 'rev-list: first-parent range topo-order' ' @@ -300,7 +300,7 @@ test_expect_success 'rev-list: first-parent range topo-order' ' commit-6-2 \ commit-6-1 commit-5-1 commit-4-1 \ >expect && - run_three_modes "git rev-list --first-parent --topo-order commit-3-8..commit-6-6" + run_three_modes git rev-list --first-parent --topo-order commit-3-8..commit-6-6 ' test_expect_success 'rev-list: ancestry-path topo-order' ' @@ -310,7 +310,7 @@ test_expect_success 'rev-list: ancestry-path topo-order' ' commit-6-4 commit-5-4 commit-4-4 commit-3-4 \ commit-6-3 commit-5-3 commit-4-3 \ >expect && - run_three_modes "git rev-list --topo-order --ancestry-path commit-3-3..commit-6-6" + run_three_modes git rev-list --topo-order --ancestry-path commit-3-3..commit-6-6 ' test_expect_success 'rev-list: symmetric difference topo-order' ' @@ -324,7 +324,7 @@ test_expect_success 'rev-list: symmetric difference topo-order' ' commit-3-8 commit-2-8 commit-1-8 \ commit-3-7 commit-2-7 commit-1-7 \ >expect && - run_three_modes "git rev-list --topo-order commit-3-8...commit-6-6" + run_three_modes git rev-list --topo-order commit-3-8...commit-6-6 ' test_done
Re: [PATCH v2 2/6] test-reach: add run_three_modes method
Junio C Hamano writes:
> SZEDER Gábor writes:
>
>>> While inspecting this code, I realized that the final test for
>>> 'commit_contains --tag' is silently dropping the '--tag' argument.
>>> It should be quoted to include both.
>>
>> Nit: while quoting the function's arguments does fix the issue, it
>> leaves the tests prone to the same issue in the future. Wouldn't it
>> be better to use $@ inside the function to refer to all its arguments?
>
> IOW, do it more like this?
>
>>> -test_three_modes () {
>>> +run_three_modes () {
>>> test_when_finished rm -rf .git/objects/info/commit-graph &&
>>> - test-tool reach $1 actual &&
>>> + $1 actual &&
>
> "$@" actual
>
> i.e. treat each parameter as separate things without further getting
> split at $IFS and ...
>
>>> +test_three_modes () {
>>> + run_three_modes "test-tool reach $1"
>
> run_three_modes test-tool reach "$1"
>
> ... make sure there three things are sent as separate, by quoting
> "$1" inside dq.
>
> I think that makes sense.
I also noticed that 2/6 made "commti_contains --tag" enclosed in dq
pair for one test, but the next test after it has the identical one.
Here is what I queued in the meantime.
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index 1b18e12a4e..1377849bf8 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -55,18 +55,18 @@ test_expect_success 'setup' '
run_three_modes () {
test_when_finished rm -rf .git/objects/info/commit-graph &&
- $1 actual &&
+ "$@" actual &&
test_cmp expect actual &&
cp commit-graph-full .git/objects/info/commit-graph &&
- $1 actual &&
+ "$@" actual &&
test_cmp expect actual &&
cp commit-graph-half .git/objects/info/commit-graph &&
- $1 actual &&
+ "$@" actual &&
test_cmp expect actual
}
test_three_modes () {
- run_three_modes "test-tool reach $1"
+ run_three_modes test-tool reach "$1"
}
test_expect_success 'ref_newer:miss' '
@@ -223,7 +223,7 @@ test_expect_success 'commit_contains:hit' '
EOF
echo "commit_contains(_,A,X,_):1" >expect &&
test_three_modes commit_contains &&
- test_three_modes "commit_contains --tag"
+ test_three_modes commit_contains --tag
'
test_expect_success 'commit_contains:miss' '
--
2.19.0-216-g2d3b1c576c
Re: [PATCH v2 2/6] test-reach: add run_three_modes method
SZEDER Gábor writes:
>> While inspecting this code, I realized that the final test for
>> 'commit_contains --tag' is silently dropping the '--tag' argument.
>> It should be quoted to include both.
>
> Nit: while quoting the function's arguments does fix the issue, it
> leaves the tests prone to the same issue in the future. Wouldn't it
> be better to use $@ inside the function to refer to all its arguments?
IOW, do it more like this?
>> -test_three_modes () {
>> +run_three_modes () {
>> test_when_finished rm -rf .git/objects/info/commit-graph &&
>> -test-tool reach $1 actual &&
>> +$1 actual &&
"$@" actual
i.e. treat each parameter as separate things without further getting
split at $IFS and ...
>> +test_three_modes () {
>> +run_three_modes "test-tool reach $1"
run_three_modes test-tool reach "$1"
... make sure there three things are sent as separate, by quoting
"$1" inside dq.
I think that makes sense.
Re: [PATCH v2 2/6] test-reach: add run_three_modes method
On Mon, Sep 17, 2018 at 09:08:44PM -0700, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee
>
> The 'test_three_modes' method assumes we are using the 'test-tool
> reach' command for our test. However, we may want to use the data
> shape of our commit graph and the three modes (no commit-graph,
> full commit-graph, partial commit-graph) for other git commands.
>
> Split test_three_modes to be a simple translation on a more general
> run_three_modes method that executes the given command and tests
> the actual output to the expected output.
>
> While inspecting this code, I realized that the final test for
> 'commit_contains --tag' is silently dropping the '--tag' argument.
> It should be quoted to include both.
Nit: while quoting the function's arguments does fix the issue, it
leaves the tests prone to the same issue in the future. Wouldn't it
be better to use $@ inside the function to refer to all its arguments?
> Signed-off-by: Derrick Stolee
> ---
> t/t6600-test-reach.sh | 14 +-
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
> index d139a00d1d..1b18e12a4e 100755
> --- a/t/t6600-test-reach.sh
> +++ b/t/t6600-test-reach.sh
> @@ -53,18 +53,22 @@ test_expect_success 'setup' '
> git config core.commitGraph true
> '
>
> -test_three_modes () {
> +run_three_modes () {
> test_when_finished rm -rf .git/objects/info/commit-graph &&
> - test-tool reach $1 actual &&
> + $1 actual &&
> test_cmp expect actual &&
> cp commit-graph-full .git/objects/info/commit-graph &&
> - test-tool reach $1 actual &&
> + $1 actual &&
> test_cmp expect actual &&
> cp commit-graph-half .git/objects/info/commit-graph &&
> - test-tool reach $1 actual &&
> + $1 actual &&
> test_cmp expect actual
> }
>
> +test_three_modes () {
> + run_three_modes "test-tool reach $1"
> +}
> +
> test_expect_success 'ref_newer:miss' '
> cat >input <<-\EOF &&
> A:commit-5-7
> @@ -219,7 +223,7 @@ test_expect_success 'commit_contains:hit' '
> EOF
> echo "commit_contains(_,A,X,_):1" >expect &&
> test_three_modes commit_contains &&
> - test_three_modes commit_contains --tag
> + test_three_modes "commit_contains --tag"
> '
>
> test_expect_success 'commit_contains:miss' '
> --
> gitgitgadget
>
[PATCH v2 2/6] test-reach: add run_three_modes method
From: Derrick Stolee
The 'test_three_modes' method assumes we are using the 'test-tool
reach' command for our test. However, we may want to use the data
shape of our commit graph and the three modes (no commit-graph,
full commit-graph, partial commit-graph) for other git commands.
Split test_three_modes to be a simple translation on a more general
run_three_modes method that executes the given command and tests
the actual output to the expected output.
While inspecting this code, I realized that the final test for
'commit_contains --tag' is silently dropping the '--tag' argument.
It should be quoted to include both.
Signed-off-by: Derrick Stolee
---
t/t6600-test-reach.sh | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index d139a00d1d..1b18e12a4e 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -53,18 +53,22 @@ test_expect_success 'setup' '
git config core.commitGraph true
'
-test_three_modes () {
+run_three_modes () {
test_when_finished rm -rf .git/objects/info/commit-graph &&
- test-tool reach $1 actual &&
+ $1 actual &&
test_cmp expect actual &&
cp commit-graph-full .git/objects/info/commit-graph &&
- test-tool reach $1 actual &&
+ $1 actual &&
test_cmp expect actual &&
cp commit-graph-half .git/objects/info/commit-graph &&
- test-tool reach $1 actual &&
+ $1 actual &&
test_cmp expect actual
}
+test_three_modes () {
+ run_three_modes "test-tool reach $1"
+}
+
test_expect_success 'ref_newer:miss' '
cat >input <<-\EOF &&
A:commit-5-7
@@ -219,7 +223,7 @@ test_expect_success 'commit_contains:hit' '
EOF
echo "commit_contains(_,A,X,_):1" >expect &&
test_three_modes commit_contains &&
- test_three_modes commit_contains --tag
+ test_three_modes "commit_contains --tag"
'
test_expect_success 'commit_contains:miss' '
--
gitgitgadget
