From f1842ff5314a3bbd22232663a4eeadfedd10f05f Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 27 Dec 2019 08:47:10 -0500 Subject: [PATCH 01/16] t2018: remove trailing space from test description Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t2018-checkout-branch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index 822381dd9d..e18abce3d2 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -1,6 +1,6 @@ #!/bin/sh -test_description='checkout ' +test_description='checkout' . ./test-lib.sh From 7ffb54618be9c44ff3213f660fca0a487893ef9a Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:52:59 -0500 Subject: [PATCH 02/16] t2018: add space between function name and () Add a space between the function name and () which brings the style in line with Git's coding guidelines. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t2018-checkout-branch.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index e18abce3d2..79b16e4677 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -12,7 +12,7 @@ test_description='checkout' # 2) HEAD is ; if is not specified, the old HEAD is used. # # If is not specified, "git checkout" is run with -b. -do_checkout() { +do_checkout () { exp_branch=$1 && exp_ref="refs/heads/$exp_branch" && @@ -32,19 +32,19 @@ do_checkout() { test $exp_sha = $(git rev-parse --verify HEAD) } -test_dirty_unmergeable() { +test_dirty_unmergeable () { ! git diff --exit-code >/dev/null } -setup_dirty_unmergeable() { +setup_dirty_unmergeable () { echo >>file1 change2 } -test_dirty_mergeable() { +test_dirty_mergeable () { ! git diff --cached --exit-code >/dev/null } -setup_dirty_mergeable() { +setup_dirty_mergeable () { echo >file2 file2 && git add file2 } From 5020f6806ac910d52f781ac76719c1cf66715632 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:53:00 -0500 Subject: [PATCH 03/16] t2018: improve style of if-statement Convert `[]` to `test` and break if-then into separate lines, both of which bring the style in line with Git's coding guidelines. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t2018-checkout-branch.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index 79b16e4677..61206a90fb 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -20,7 +20,8 @@ do_checkout () { exp_sha=${2:-$(git rev-parse --verify HEAD)} && # default options for git checkout: -b - if [ -z "$3" ]; then + if test -z "$3" + then opts="-b" else opts="$3" From 40caa5366a84fba6e64ea32bf98d9c1aba0c537b Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Sun, 26 Jan 2020 15:23:05 -0500 Subject: [PATCH 04/16] t2018: be more discerning when checking for expected exit codes Functions test_dirty_unmergeable() and test_dirty_mergeable() expect git-diff to exit with the specific code 1. However, rather than checking for that value explicitly, they instead negate the exit code. Unfortunately, this negation makes it impossible to distinguish the expected code from some other unexpected non-zero code, for instance, from a segmentation fault. Therefore, be more discerning by checking the exit code explicitly using test_expect_code(). Furthermore, some callers of those functions want to negate the result again, and do so with test_must_fail(). However, test_must_fail() should only be used with git commands. Address this by introducing a couple new tiny helper functions which test the exact condition expected (without the unnecessarily confusing double-negation). Helped-by: Eric Sunshine Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t2018-checkout-branch.sh | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index 61206a90fb..7ca55efc6b 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -34,7 +34,11 @@ do_checkout () { } test_dirty_unmergeable () { - ! git diff --exit-code >/dev/null + test_expect_code 1 git diff --exit-code +} + +test_dirty_unmergeable_discards_changes () { + git diff --exit-code } setup_dirty_unmergeable () { @@ -42,7 +46,11 @@ setup_dirty_unmergeable () { } test_dirty_mergeable () { - ! git diff --cached --exit-code >/dev/null + test_expect_code 1 git diff --cached --exit-code +} + +test_dirty_mergeable_discards_changes () { + git diff --cached --exit-code } setup_dirty_mergeable () { @@ -94,7 +102,7 @@ test_expect_success 'checkout -f -b to a new branch with unmergeable changes dis # still dirty and on branch1 do_checkout branch2 $HEAD1 "-f -b" && - test_must_fail test_dirty_unmergeable + test_dirty_unmergeable_discards_changes ' test_expect_success 'checkout -b to a new branch preserves mergeable changes' ' @@ -112,7 +120,7 @@ test_expect_success 'checkout -f -b to a new branch with mergeable changes disca test_when_finished git reset --hard HEAD && setup_dirty_mergeable && do_checkout branch2 $HEAD1 "-f -b" && - test_must_fail test_dirty_mergeable + test_dirty_mergeable_discards_changes ' test_expect_success 'checkout -b to an existing branch fails' ' @@ -163,7 +171,7 @@ test_expect_success 'checkout -B to an existing branch with unmergeable changes test_expect_success 'checkout -f -B to an existing branch with unmergeable changes discards changes' ' # still dirty and on branch1 do_checkout branch2 $HEAD1 "-f -B" && - test_must_fail test_dirty_unmergeable + test_dirty_unmergeable_discards_changes ' test_expect_success 'checkout -B to an existing branch preserves mergeable changes' ' @@ -180,7 +188,7 @@ test_expect_success 'checkout -f -B to an existing branch with mergeable changes setup_dirty_mergeable && do_checkout branch2 $HEAD1 "-f -B" && - test_must_fail test_dirty_mergeable + test_dirty_mergeable_discards_changes ' test_expect_success 'checkout -b ' ' From 30c0367668ab919cc1bca3cff73820f77b573765 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:53:02 -0500 Subject: [PATCH 05/16] t2018: teach do_checkout() to accept `!` arg We are running `test_must_fail do_checkout`. However, `test_must_fail` should only be used on git commands. Teach do_checkout() to accept `!` as a potential first argument which will cause the function to expect the "git checkout" to fail. This increases the granularity of the test as, instead of blindly checking that do_checkout() failed, we check that only the specific expected invocation of git fails. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t2018-checkout-branch.sh | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index 7ca55efc6b..687ab6713c 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -4,7 +4,7 @@ test_description='checkout' . ./test-lib.sh -# Arguments: [] +# Arguments: [!] [] # # Runs "git checkout" to switch to , testing that # @@ -12,7 +12,16 @@ test_description='checkout' # 2) HEAD is ; if is not specified, the old HEAD is used. # # If is not specified, "git checkout" is run with -b. +# +# If the first argument is `!`, "git checkout" is expected to fail when +# it is run. do_checkout () { + should_fail= && + if test "x$1" = "x!" + then + should_fail=yes && + shift + fi && exp_branch=$1 && exp_ref="refs/heads/$exp_branch" && @@ -27,10 +36,14 @@ do_checkout () { opts="$3" fi - git checkout $opts $exp_branch $exp_sha && - - test $exp_ref = $(git rev-parse --symbolic-full-name HEAD) && - test $exp_sha = $(git rev-parse --verify HEAD) + if test -n "$should_fail" + then + test_must_fail git checkout $opts $exp_branch $exp_sha + else + git checkout $opts $exp_branch $exp_sha && + test $exp_ref = $(git rev-parse --symbolic-full-name HEAD) && + test $exp_sha = $(git rev-parse --verify HEAD) + fi } test_dirty_unmergeable () { @@ -91,7 +104,7 @@ test_expect_success 'checkout -b to a new branch, set to an explicit ref' ' test_expect_success 'checkout -b to a new branch with unmergeable changes fails' ' setup_dirty_unmergeable && - test_must_fail do_checkout branch2 $HEAD1 && + do_checkout ! branch2 $HEAD1 && test_dirty_unmergeable ' @@ -125,7 +138,7 @@ test_expect_success 'checkout -f -b to a new branch with mergeable changes disca test_expect_success 'checkout -b to an existing branch fails' ' test_when_finished git reset --hard HEAD && - test_must_fail do_checkout branch2 $HEAD2 + do_checkout ! branch2 $HEAD2 ' test_expect_success 'checkout -b to @{-1} fails with the right branch name' ' @@ -164,7 +177,7 @@ test_expect_success 'checkout -B to an existing branch with unmergeable changes git checkout branch1 && setup_dirty_unmergeable && - test_must_fail do_checkout branch2 $HEAD1 -B && + do_checkout ! branch2 $HEAD1 -B && test_dirty_unmergeable ' From 62e80fcb480d388777a9d6d96d977e02c3e7dd36 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:53:03 -0500 Subject: [PATCH 06/16] t2018: don't lose return code of git commands Fix invocations of git commands so their exit codes are not lost within non-assignment command substitutions. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t2018-checkout-branch.sh | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index 687ab6713c..caf43571b1 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -41,8 +41,12 @@ do_checkout () { test_must_fail git checkout $opts $exp_branch $exp_sha else git checkout $opts $exp_branch $exp_sha && - test $exp_ref = $(git rev-parse --symbolic-full-name HEAD) && - test $exp_sha = $(git rev-parse --verify HEAD) + echo "$exp_ref" >ref.expect && + git rev-parse --symbolic-full-name HEAD >ref.actual && + test_cmp ref.expect ref.actual && + echo "$exp_sha" >sha.expect && + git rev-parse --verify HEAD >sha.actual && + test_cmp sha.expect sha.actual fi } @@ -162,7 +166,8 @@ test_expect_success 'checkout -B to a merge base' ' ' test_expect_success 'checkout -B to an existing branch from detached HEAD resets branch to HEAD' ' - git checkout $(git rev-parse --verify HEAD) && + head=$(git rev-parse --verify HEAD) && + git checkout "$head" && do_checkout branch2 "" -B ' From 4a6f11fd7be30a215b972a4d1dccd09ff146a36b Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:53:04 -0500 Subject: [PATCH 07/16] t2018: replace "sha" with "oid" As part of the effort to become more hash-agnostic, replace all instances of "sha" with "oid". Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t2018-checkout-branch.sh | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh index caf43571b1..bbca7ef8da 100755 --- a/t/t2018-checkout-branch.sh +++ b/t/t2018-checkout-branch.sh @@ -4,12 +4,12 @@ test_description='checkout' . ./test-lib.sh -# Arguments: [!] [] +# Arguments: [!] [] # # Runs "git checkout" to switch to , testing that # # 1) we are on the specified branch, ; -# 2) HEAD is ; if is not specified, the old HEAD is used. +# 2) HEAD is ; if is not specified, the old HEAD is used. # # If is not specified, "git checkout" is run with -b. # @@ -25,8 +25,8 @@ do_checkout () { exp_branch=$1 && exp_ref="refs/heads/$exp_branch" && - # if is not specified, use HEAD. - exp_sha=${2:-$(git rev-parse --verify HEAD)} && + # if is not specified, use HEAD. + exp_oid=${2:-$(git rev-parse --verify HEAD)} && # default options for git checkout: -b if test -z "$3" @@ -38,15 +38,15 @@ do_checkout () { if test -n "$should_fail" then - test_must_fail git checkout $opts $exp_branch $exp_sha + test_must_fail git checkout $opts $exp_branch $exp_oid else - git checkout $opts $exp_branch $exp_sha && + git checkout $opts $exp_branch $exp_oid && echo "$exp_ref" >ref.expect && git rev-parse --symbolic-full-name HEAD >ref.actual && test_cmp ref.expect ref.actual && - echo "$exp_sha" >sha.expect && - git rev-parse --verify HEAD >sha.actual && - test_cmp sha.expect sha.actual + echo "$exp_oid" >oid.expect && + git rev-parse --verify HEAD >oid.actual && + test_cmp oid.expect oid.actual fi } From 245b9ba0ba51a9e19f77a2a96a82a4f021d4a9fc Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:53:05 -0500 Subject: [PATCH 08/16] t3030: use test_path_is_missing() We use `test_must_fail test -d` to ensure that the directory is removed. However, test_must_fail() should only be used for git commands. Use test_path_is_missing() instead to check that the directory has been removed. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3030-merge-recursive.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh index 2170758e38..d48d211a95 100755 --- a/t/t3030-merge-recursive.sh +++ b/t/t3030-merge-recursive.sh @@ -604,7 +604,7 @@ test_expect_success 'merge removes empty directories' ' git commit -mremoved-d/e && git checkout master && git merge -s recursive rm && - test_must_fail test -d d + test_path_is_missing d ' test_expect_success 'merge-recursive simple w/submodule' ' From 86ce6e0dd195d0b0494b0866c4b2771141a3c06c Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:53:06 -0500 Subject: [PATCH 09/16] t3310: extract common notes_merge_files_gone() We have many statements which are duplicated. Extract and replace these duplicate statements with notes_merge_files_gone(). While we're at it, replace the test_might_fail(), which should only be used on git commands. Also, remove the redirection from stderr to /dev/null. This is because the test scripts automatically suppress output already. Otherwise, if a developer asks for verbose output via the `-v` flag, the stderr output may be useful for debugging. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3310-notes-merge-manual-resolve.sh | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh index 2dea846e25..d746f4ba55 100755 --- a/t/t3310-notes-merge-manual-resolve.sh +++ b/t/t3310-notes-merge-manual-resolve.sh @@ -32,6 +32,12 @@ verify_notes () { test_cmp "expect_log_$notes_ref" "output_log_$notes_ref" } +notes_merge_files_gone () { + # No .git/NOTES_MERGE_* files left + { ls .git/NOTES_MERGE_* >output || :; } && + test_must_be_empty output +} + cat <expect_notes_x 6e8e3febca3c2bb896704335cc4d0c34cb2f8715 $commit_sha4 e5388c10860456ee60673025345fe2e153eb8cf8 $commit_sha3 @@ -335,9 +341,7 @@ EOF y and z notes on 4th commit EOF git notes merge --commit && - # No .git/NOTES_MERGE_* files left - test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null && - test_must_be_empty output && + notes_merge_files_gone && # Merge commit has pre-merge y and pre-merge z as parents test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" && test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" && @@ -397,9 +401,7 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol test_expect_success 'abort notes merge' ' git notes merge --abort && - # No .git/NOTES_MERGE_* files left - test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null && - test_must_be_empty output && + notes_merge_files_gone && # m has not moved (still == y) test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" && # Verify that other notes refs has not changed (w, x, y and z) @@ -464,9 +466,7 @@ EOF echo "new note on 5th commit" > .git/NOTES_MERGE_WORKTREE/$commit_sha5 && # Finalize merge git notes merge --commit && - # No .git/NOTES_MERGE_* files left - test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null && - test_must_be_empty output && + notes_merge_files_gone && # Merge commit has pre-merge y and pre-merge z as parents test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" && test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" && @@ -553,9 +553,7 @@ EOF test_expect_success 'resolve situation by aborting the notes merge' ' git notes merge --abort && - # No .git/NOTES_MERGE_* files left - test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null && - test_must_be_empty output && + notes_merge_files_gone && # m has not moved (still == w) test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" && # Verify that other notes refs has not changed (w, x, y and z) From a781cd6fefcb40297cdb52cb02e7cb6b940598f2 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:53:07 -0500 Subject: [PATCH 10/16] t3415: stop losing return codes of git commands Fix invocations of git commands so their exit codes are not lost within non-assignment command substitutions. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3415-rebase-autosquash.sh | 113 +++++++++++++++++++++++++---------- 1 file changed, 82 insertions(+), 31 deletions(-) diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh index 22d218698e..b0add36f82 100755 --- a/t/t3415-rebase-autosquash.sh +++ b/t/t3415-rebase-autosquash.sh @@ -37,8 +37,12 @@ test_auto_fixup () { git log --oneline >actual && test_line_count = 3 actual && git diff --exit-code $1 && - test 1 = "$(git cat-file blob HEAD^:file1)" && - test 1 = $(git cat-file commit HEAD^ | grep first | wc -l) + echo 1 >expect && + git cat-file blob HEAD^:file1 >actual && + test_cmp expect actual && + git cat-file commit HEAD^ >commit && + grep first commit >actual && + test_line_count = 1 actual } test_expect_success 'auto fixup (option)' ' @@ -66,8 +70,12 @@ test_auto_squash () { git log --oneline >actual && test_line_count = 3 actual && git diff --exit-code $1 && - test 1 = "$(git cat-file blob HEAD^:file1)" && - test 2 = $(git cat-file commit HEAD^ | grep first | wc -l) + echo 1 >expect && + git cat-file blob HEAD^:file1 >actual && + test_cmp expect actual && + git cat-file commit HEAD^ >commit && + grep first commit >actual && + test_line_count = 2 actual } test_expect_success 'auto squash (option)' ' @@ -94,7 +102,8 @@ test_expect_success 'misspelled auto squash' ' git log --oneline >actual && test_line_count = 4 actual && git diff --exit-code final-missquash && - test 0 = $(git rev-list final-missquash...HEAD | wc -l) + git rev-list final-missquash...HEAD >list && + test_must_be_empty list ' test_expect_success 'auto squash that matches 2 commits' ' @@ -113,9 +122,15 @@ test_expect_success 'auto squash that matches 2 commits' ' git log --oneline >actual && test_line_count = 4 actual && git diff --exit-code final-multisquash && - test 1 = "$(git cat-file blob HEAD^^:file1)" && - test 2 = $(git cat-file commit HEAD^^ | grep first | wc -l) && - test 1 = $(git cat-file commit HEAD | grep first | wc -l) + echo 1 >expect && + git cat-file blob HEAD^^:file1 >actual && + test_cmp expect actual && + git cat-file commit HEAD^^ >commit && + grep first commit >actual && + test_line_count = 2 actual && + git cat-file commit HEAD >commit && + grep first commit >actual && + test_line_count = 1 actual ' test_expect_success 'auto squash that matches a commit after the squash' ' @@ -134,25 +149,38 @@ test_expect_success 'auto squash that matches a commit after the squash' ' git log --oneline >actual && test_line_count = 5 actual && git diff --exit-code final-presquash && - test 0 = "$(git cat-file blob HEAD^^:file1)" && - test 1 = "$(git cat-file blob HEAD^:file1)" && - test 1 = $(git cat-file commit HEAD | grep third | wc -l) && - test 1 = $(git cat-file commit HEAD^ | grep third | wc -l) + echo 0 >expect && + git cat-file blob HEAD^^:file1 >actual && + test_cmp expect actual && + echo 1 >expect && + git cat-file blob HEAD^:file1 >actual && + test_cmp expect actual && + git cat-file commit HEAD >commit && + grep third commit >actual && + test_line_count = 1 actual && + git cat-file commit HEAD^ >commit && + grep third commit >actual && + test_line_count = 1 actual ' test_expect_success 'auto squash that matches a sha1' ' git reset --hard base && echo 1 >file1 && git add -u && test_tick && - git commit -m "squash! $(git rev-parse --short HEAD^)" && + oid=$(git rev-parse --short HEAD^) && + git commit -m "squash! $oid" && git tag final-shasquash && test_tick && git rebase --autosquash -i HEAD^^^ && git log --oneline >actual && test_line_count = 3 actual && git diff --exit-code final-shasquash && - test 1 = "$(git cat-file blob HEAD^:file1)" && - test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l) + echo 1 >expect && + git cat-file blob HEAD^:file1 >actual && + test_cmp expect actual && + git cat-file commit HEAD^ >commit && + grep squash commit >actual && + test_line_count = 1 actual ' test_expect_success 'auto squash that matches longer sha1' ' @@ -160,15 +188,20 @@ test_expect_success 'auto squash that matches longer sha1' ' echo 1 >file1 && git add -u && test_tick && - git commit -m "squash! $(git rev-parse --short=11 HEAD^)" && + oid=$(git rev-parse --short=11 HEAD^) && + git commit -m "squash! $oid" && git tag final-longshasquash && test_tick && git rebase --autosquash -i HEAD^^^ && git log --oneline >actual && test_line_count = 3 actual && git diff --exit-code final-longshasquash && - test 1 = "$(git cat-file blob HEAD^:file1)" && - test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l) + echo 1 >expect && + git cat-file blob HEAD^:file1 >actual && + test_cmp expect actual && + git cat-file commit HEAD^ >commit && + grep squash commit >actual && + test_line_count = 1 actual ' test_auto_commit_flags () { @@ -183,8 +216,12 @@ test_auto_commit_flags () { git log --oneline >actual && test_line_count = 3 actual && git diff --exit-code final-commit-$1 && - test 1 = "$(git cat-file blob HEAD^:file1)" && - test $2 = $(git cat-file commit HEAD^ | grep first | wc -l) + echo 1 >expect && + git cat-file blob HEAD^:file1 >actual && + test_cmp expect actual && + git cat-file commit HEAD^ >commit && + grep first commit >actual && + test_line_count = $2 actual } test_expect_success 'use commit --fixup' ' @@ -210,11 +247,15 @@ test_auto_fixup_fixup () { ( set_cat_todo_editor && test_must_fail git rebase --autosquash -i HEAD^^^^ >actual && + head=$(git rev-parse --short HEAD) && + parent1=$(git rev-parse --short HEAD^) && + parent2=$(git rev-parse --short HEAD^^) && + parent3=$(git rev-parse --short HEAD^^^) && cat >expected <<-EOF && - pick $(git rev-parse --short HEAD^^^) first commit - $1 $(git rev-parse --short HEAD^) $1! first - $1 $(git rev-parse --short HEAD) $1! $2! first - pick $(git rev-parse --short HEAD^^) second commit + pick $parent3 first commit + $1 $parent1 $1! first + $1 $head $1! $2! first + pick $parent2 second commit EOF test_cmp expected actual ) && @@ -222,13 +263,17 @@ test_auto_fixup_fixup () { git log --oneline >actual && test_line_count = 3 actual git diff --exit-code "final-$1-$2" && - test 2 = "$(git cat-file blob HEAD^:file1)" && + echo 2 >expect && + git cat-file blob HEAD^:file1 >actual && + test_cmp expect actual && + git cat-file commit HEAD^ >commit && + grep first commit >actual && if test "$1" = "fixup" then - test 1 = $(git cat-file commit HEAD^ | grep first | wc -l) + test_line_count = 1 actual elif test "$1" = "squash" then - test 3 = $(git cat-file commit HEAD^ | grep first | wc -l) + test_line_count = 3 actual else false fi @@ -256,19 +301,25 @@ test_expect_success C_LOCALE_OUTPUT 'autosquash with custom inst format' ' echo 2 >file1 && git add -u && test_tick && - git commit -m "squash! $(git rev-parse --short HEAD^)" && + oid=$(git rev-parse --short HEAD^) && + git commit -m "squash! $oid" && echo 1 >file1 && git add -u && test_tick && - git commit -m "squash! $(git log -n 1 --format=%s HEAD~2)" && + subject=$(git log -n 1 --format=%s HEAD~2) && + git commit -m "squash! $subject" && git tag final-squash-instFmt && test_tick && git rebase --autosquash -i HEAD~4 && git log --oneline >actual && test_line_count = 3 actual && git diff --exit-code final-squash-instFmt && - test 1 = "$(git cat-file blob HEAD^:file1)" && - test 2 = $(git cat-file commit HEAD^ | grep squash | wc -l) + echo 1 >expect && + git cat-file blob HEAD^:file1 >actual && + test_cmp expect actual && + git cat-file commit HEAD^ >commit && + grep squash commit >actual && + test_line_count = 2 actual ' test_expect_success 'autosquash with empty custom instructionFormat' ' From c232ffa83cb3b21e4e6671c50609bfaf5659e899 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:53:08 -0500 Subject: [PATCH 11/16] t3415: increase granularity of test_auto_{fixup,squash}() We are using `test_must_fail test_auto_{fixup,squash}` which would ensure that the function failed. However, this is a little ham-fisted as there is no way of ensuring that the expected part of the function actually fails. Increase the granularity by accepting an optional `!` first argument which will check that the rebase does not squash in any commits by ensuring that there are still 4 commits. If `!` is not provided, the old logic is run. This patch may be better reviewed with `--ignore-all-space`. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3415-rebase-autosquash.sh | 64 +++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh index b0add36f82..093de9005b 100755 --- a/t/t3415-rebase-autosquash.sh +++ b/t/t3415-rebase-autosquash.sh @@ -25,6 +25,13 @@ test_expect_success setup ' ' test_auto_fixup () { + no_squash= && + if test "x$1" = 'x!' + then + no_squash=true + shift + fi && + git reset --hard base && echo 1 >file1 && git add -u && @@ -35,14 +42,19 @@ test_auto_fixup () { test_tick && git rebase $2 -i HEAD^^^ && git log --oneline >actual && - test_line_count = 3 actual && - git diff --exit-code $1 && - echo 1 >expect && - git cat-file blob HEAD^:file1 >actual && - test_cmp expect actual && - git cat-file commit HEAD^ >commit && - grep first commit >actual && - test_line_count = 1 actual + if test -n "$no_squash" + then + test_line_count = 4 actual + else + test_line_count = 3 actual && + git diff --exit-code $1 && + echo 1 >expect && + git cat-file blob HEAD^:file1 >actual && + test_cmp expect actual && + git cat-file commit HEAD^ >commit && + grep first commit >actual && + test_line_count = 1 actual + fi } test_expect_success 'auto fixup (option)' ' @@ -52,12 +64,19 @@ test_expect_success 'auto fixup (option)' ' test_expect_success 'auto fixup (config)' ' git config rebase.autosquash true && test_auto_fixup final-fixup-config-true && - test_must_fail test_auto_fixup fixup-config-true-no --no-autosquash && + test_auto_fixup ! fixup-config-true-no --no-autosquash && git config rebase.autosquash false && - test_must_fail test_auto_fixup final-fixup-config-false + test_auto_fixup ! final-fixup-config-false ' test_auto_squash () { + no_squash= && + if test "x$1" = 'x!' + then + no_squash=true + shift + fi && + git reset --hard base && echo 1 >file1 && git add -u && @@ -68,14 +87,19 @@ test_auto_squash () { test_tick && git rebase $2 -i HEAD^^^ && git log --oneline >actual && - test_line_count = 3 actual && - git diff --exit-code $1 && - echo 1 >expect && - git cat-file blob HEAD^:file1 >actual && - test_cmp expect actual && - git cat-file commit HEAD^ >commit && - grep first commit >actual && - test_line_count = 2 actual + if test -n "$no_squash" + then + test_line_count = 4 actual + else + test_line_count = 3 actual && + git diff --exit-code $1 && + echo 1 >expect && + git cat-file blob HEAD^:file1 >actual && + test_cmp expect actual && + git cat-file commit HEAD^ >commit && + grep first commit >actual && + test_line_count = 2 actual + fi } test_expect_success 'auto squash (option)' ' @@ -85,9 +109,9 @@ test_expect_success 'auto squash (option)' ' test_expect_success 'auto squash (config)' ' git config rebase.autosquash true && test_auto_squash final-squash-config-true && - test_must_fail test_auto_squash squash-config-true-no --no-autosquash && + test_auto_squash ! squash-config-true-no --no-autosquash && git config rebase.autosquash false && - test_must_fail test_auto_squash final-squash-config-false + test_auto_squash ! final-squash-config-false ' test_expect_success 'misspelled auto squash' ' From 1c9fd32fd2b5e1b8f2807d93f3a138d5846dbaf0 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:53:09 -0500 Subject: [PATCH 12/16] t3419: stop losing return code of git command Fix invocation of git command so its exit codes is not lost within a non-assignment command substitution. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3419-rebase-patch-id.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh index 49f548cdb9..94552669ae 100755 --- a/t/t3419-rebase-patch-id.sh +++ b/t/t3419-rebase-patch-id.sh @@ -80,7 +80,8 @@ do_tests () { git commit -q -m "change big file again" && git checkout -q other^{} && git rebase master && - test_must_fail test -n "$(git rev-list master...HEAD~)" + git rev-list master...HEAD~ >revs && + test_must_be_empty revs ' test_expect_success $pr 'do not drop patch' ' From e8a1c686aee560e07e4419f5adba1b1245289dd9 Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Mon, 6 Jan 2020 23:53:10 -0500 Subject: [PATCH 13/16] t3504: do check for conflict marker after failed cherry-pick The test with disabled rerere should make sure that the cherry-picked result does not have the conflict replaced with a recorded resolution. It attempts to do so by ensuring that the file content is _not_ equal to some other file. That by itself is a very dubious check because just about every random result of an incomplete cherry-pick would satisfy the condition. In this case, the intent was to check that the conflicting file does _not_ contain the resolved content. But the check actually uses the wrong reference file, but not the resolved content. Needless to say that the non-equality is satisfied. And, on top of it, it uses a commit that does not even touch the file that is checked. Do check for the expected result, which is content from both sides of the merge and merge conflicts. (The latter we check for just the middle separator for brevity.) As a side-effect, this also removes an incorrect use of test_must_fail. Signed-off-by: Johannes Sixt Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3504-cherry-pick-rerere.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/t3504-cherry-pick-rerere.sh b/t/t3504-cherry-pick-rerere.sh index a267b2d144..80a0d08706 100755 --- a/t/t3504-cherry-pick-rerere.sh +++ b/t/t3504-cherry-pick-rerere.sh @@ -94,8 +94,10 @@ test_expect_success 'cherry-pick --rerere-autoupdate more than once' ' test_expect_success 'cherry-pick conflict without rerere' ' test_config rerere.enabled false && - test_must_fail git cherry-pick master && - test_must_fail test_cmp expect foo + test_must_fail git cherry-pick foo-master && + grep ===== foo && + grep foo-dev foo && + grep foo-master foo ' test_done From 2def7f017ccedd04d86f7f343020f1f1bf3dfac8 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:53:11 -0500 Subject: [PATCH 14/16] t3507: fix indentation We have some test cases which are indented 7-spaces instead of a tab. Reindent with a tab instead. This patch should appear empty with `--ignore-all-space`. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3507-cherry-pick-conflict.sh | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh index 9b9b4ca8d4..2a0d119c8a 100755 --- a/t/t3507-cherry-pick-conflict.sh +++ b/t/t3507-cherry-pick-conflict.sh @@ -381,23 +381,23 @@ test_expect_success 'failed commit does not clear REVERT_HEAD' ' ' test_expect_success 'successful final commit clears revert state' ' - pristine_detach picked-signed && + pristine_detach picked-signed && - test_must_fail git revert picked-signed base && - echo resolved >foo && - test_path_is_file .git/sequencer/todo && - git commit -a && - test_must_fail test_path_exists .git/sequencer + test_must_fail git revert picked-signed base && + echo resolved >foo && + test_path_is_file .git/sequencer/todo && + git commit -a && + test_must_fail test_path_exists .git/sequencer ' test_expect_success 'reset after final pick clears revert state' ' - pristine_detach picked-signed && + pristine_detach picked-signed && - test_must_fail git revert picked-signed base && - echo resolved >foo && - test_path_is_file .git/sequencer/todo && - git reset && - test_must_fail test_path_exists .git/sequencer + test_must_fail git revert picked-signed base && + echo resolved >foo && + test_path_is_file .git/sequencer/todo && + git reset && + test_must_fail test_path_exists .git/sequencer ' test_expect_success 'revert conflict, diff3 -m style' ' From a8c663cf65ae1e40c90bfd4a1dd3e8d1826eafbe Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:53:12 -0500 Subject: [PATCH 15/16] t3507: use test_path_is_missing() The test_must_fail() function should only be used for git commands since we should assume that external commands work sanely. Replace `test_must_fail test_path_exists` with `test_path_is_missing` since we expect these paths to not exist. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t3507-cherry-pick-conflict.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh index 2a0d119c8a..9bd482ce3b 100755 --- a/t/t3507-cherry-pick-conflict.sh +++ b/t/t3507-cherry-pick-conflict.sh @@ -168,7 +168,7 @@ test_expect_success 'successful final commit clears cherry-pick state' ' echo resolved >foo && test_path_is_file .git/sequencer/todo && git commit -a && - test_must_fail test_path_exists .git/sequencer + test_path_is_missing .git/sequencer ' test_expect_success 'reset after final pick clears cherry-pick state' ' @@ -178,7 +178,7 @@ test_expect_success 'reset after final pick clears cherry-pick state' ' echo resolved >foo && test_path_is_file .git/sequencer/todo && git reset && - test_must_fail test_path_exists .git/sequencer + test_path_is_missing .git/sequencer ' test_expect_success 'failed cherry-pick produces dirty index' ' @@ -387,7 +387,7 @@ test_expect_success 'successful final commit clears revert state' ' echo resolved >foo && test_path_is_file .git/sequencer/todo && git commit -a && - test_must_fail test_path_exists .git/sequencer + test_path_is_missing .git/sequencer ' test_expect_success 'reset after final pick clears revert state' ' @@ -397,7 +397,7 @@ test_expect_success 'reset after final pick clears revert state' ' echo resolved >foo && test_path_is_file .git/sequencer/todo && git reset && - test_must_fail test_path_exists .git/sequencer + test_path_is_missing .git/sequencer ' test_expect_success 'revert conflict, diff3 -m style' ' From 37a63faae58aee5cd2dd9806d2e98c9ad998a7e5 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Mon, 6 Jan 2020 23:53:13 -0500 Subject: [PATCH 16/16] t4124: only mark git command with test_must_fail The test_must_fail function should only be used for git commands since we should assume that external commands work sanely. Since apply_patch wraps a sed and git invocation, rewrite it to accept an `!` argument which would cause only the git command to be prefixed with `test_must_fail`. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t4124-apply-ws-rule.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh index ff51e9e789..971a5a7512 100755 --- a/t/t4124-apply-ws-rule.sh +++ b/t/t4124-apply-ws-rule.sh @@ -35,9 +35,15 @@ prepare_test_file () { } apply_patch () { + cmd_prefix= && + if test "x$1" = 'x!' + then + cmd_prefix=test_must_fail && + shift + fi && >target && sed -e "s|\([ab]\)/file|\1/target|"