From 8cb7980382855f9f696924fec70ed88ea6895030 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 13 Nov 2019 16:52:15 -0800 Subject: [PATCH 01/15] t0000: test multiple local assignment According to POSIX enhancement request '0000767: Add built-in "local"'[1], dash only allows one variable in a local definition; it permits assignment though it doesn't document that clearly. however, this isn't true since t0000 still passes with this patch applied on dash 0.5.10.2. Needless to say, since `local` isn't POSIX standardized, it is not exactly clear what `local` entails on different versions of different shells. We currently already have many instances of multiple local assignments in our codebase. Ensure that this is actually supported by explicitly testing that it is sane. [1]: http://austingroupbugs.net/bug_view_page.php?bug_id=767 Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t0000-basic.sh | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index 4d3f7ba295..a4af2342d1 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -20,9 +20,9 @@ modification *should* take notice and update the test vectors here. . ./test-lib.sh -try_local_x () { - local x="local" && - echo "$x" +try_local_xy () { + local x="local" y="alsolocal" && + echo "$x $y" } # Check whether the shell supports the "local" keyword. "local" is not @@ -35,11 +35,12 @@ try_local_x () { # relying on "local". test_expect_success 'verify that the running shell supports "local"' ' x="notlocal" && - echo "local" >expected1 && - try_local_x >actual1 && + y="alsonotlocal" && + echo "local alsolocal" >expected1 && + try_local_xy >actual1 && test_cmp expected1 actual1 && - echo "notlocal" >expected2 && - echo "$x" >actual2 && + echo "notlocal alsonotlocal" >expected2 && + echo "$x $y" >actual2 && test_cmp expected2 actual2 ' From 2c9e125b2727c33dccdba1b2a46f462b77e10bed Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:07:45 -0800 Subject: [PATCH 02/15] t: teach test_cmp_rev to accept ! for not-equals In the case where we are using test_cmp_rev() to report not-equals, we write `! test_cmp_rev`. However, since test_cmp_rev() contains r1=$(git rev-parse --verify "$1") && r2=$(git rev-parse --verify "$2") && `! test_cmp_rev` will succeed if any of the rev-parses fail. This behavior is not desired. We want the rev-parses to _always_ be successful. Rewrite test_cmp_rev() to optionally accept "!" as the first argument to do a not-equals comparison. Rewrite `! test_cmp_rev` to `test_cmp_rev !` in all tests to take advantage of this new functionality. Also, rewrite the rev-parse logic to end with a `|| return 1` instead of &&-chaining into the rev-comparison logic. This makes it obvious to future readers that we explicitly intend on returning early if either of the rev-parses fail. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t2400-worktree-add.sh | 4 ++-- t/t3400-rebase.sh | 2 +- t/t3421-rebase-topology-linear.sh | 6 +++--- t/t3430-rebase-merges.sh | 2 +- t/t3432-rebase-fast-forward.sh | 2 +- t/t3501-revert-cherry-pick.sh | 2 +- t/t3508-cherry-pick-many-commits.sh | 2 +- t/test-lib-functions.sh | 19 +++++++++++++++---- 8 files changed, 25 insertions(+), 14 deletions(-) diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh index e819ba741e..52d476979b 100755 --- a/t/t2400-worktree-add.sh +++ b/t/t2400-worktree-add.sh @@ -438,7 +438,7 @@ test_expect_success 'git worktree add does not match remote' ' cd foo && test_must_fail git config "branch.foo.remote" && test_must_fail git config "branch.foo.merge" && - ! test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo + test_cmp_rev ! refs/remotes/repo_a/foo refs/heads/foo ) ' @@ -483,7 +483,7 @@ test_expect_success 'git worktree --no-guess-remote option overrides config' ' cd foo && test_must_fail git config "branch.foo.remote" && test_must_fail git config "branch.foo.merge" && - ! test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo + test_cmp_rev ! refs/remotes/repo_a/foo refs/heads/foo ) ' diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index ab18ac5f28..f267f6cd54 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -64,7 +64,7 @@ test_expect_success 'rebase sets ORIG_HEAD to pre-rebase state' ' pre="$(git rev-parse --verify HEAD)" && git rebase master && test_cmp_rev "$pre" ORIG_HEAD && - ! test_cmp_rev "$pre" HEAD + test_cmp_rev ! "$pre" HEAD ' test_expect_success 'rebase, with and specified as :/quuxery' ' diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index b847064f91..325072b0a3 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -61,7 +61,7 @@ test_run_rebase () { test_expect_$result "rebase $* -f rewrites even if upstream is an ancestor" " reset_rebase && git rebase $* -f b e && - ! test_cmp_rev e HEAD && + test_cmp_rev ! e HEAD && test_cmp_rev b HEAD~2 && test_linear_range 'd e' b.. " @@ -78,7 +78,7 @@ test_run_rebase () { test_expect_$result "rebase $* -f rewrites even if remote upstream is an ancestor" " reset_rebase && git rebase $* -f branch-b branch-e && - ! test_cmp_rev branch-e origin/branch-e && + test_cmp_rev ! branch-e origin/branch-e && test_cmp_rev branch-b HEAD~2 && test_linear_range 'd e' branch-b.. " @@ -368,7 +368,7 @@ test_run_rebase () { test_expect_$result "rebase $* -f --root on linear history causes re-write" " reset_rebase && git rebase $* -f --root c && - ! test_cmp_rev a HEAD~2 && + test_cmp_rev ! a HEAD~2 && test_linear_range 'a b c' HEAD " } diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index 9efcf4808a..abbdc26b1b 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -346,7 +346,7 @@ test_expect_success 'A root commit can be a cousin, treat it that way' ' git merge --allow-unrelated-histories khnum && test_tick && git rebase -f -r HEAD^ && - ! test_cmp_rev HEAD^2 khnum && + test_cmp_rev ! HEAD^2 khnum && test_cmp_graph HEAD^.. <<-\EOF && * Merge branch '\''khnum'\'' into asherah |\ diff --git a/t/t3432-rebase-fast-forward.sh b/t/t3432-rebase-fast-forward.sh index 034ffc7e76..92f95b57da 100755 --- a/t/t3432-rebase-fast-forward.sh +++ b/t/t3432-rebase-fast-forward.sh @@ -64,7 +64,7 @@ test_rebase_same_head_ () { test_cmp_rev \$oldhead \$newhead elif test $cmp = diff then - ! test_cmp_rev \$oldhead \$newhead + test_cmp_rev ! \$oldhead \$newhead fi " } diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh index d1c68af8c5..1c51a9131d 100755 --- a/t/t3501-revert-cherry-pick.sh +++ b/t/t3501-revert-cherry-pick.sh @@ -106,7 +106,7 @@ test_expect_success 'cherry-pick on unborn branch' ' rm -rf * && git cherry-pick initial && git diff --quiet initial && - ! test_cmp_rev initial HEAD + test_cmp_rev ! initial HEAD ' test_expect_success 'cherry-pick "-" to pick from previous branch' ' diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh index b457333e18..23070a7b73 100755 --- a/t/t3508-cherry-pick-many-commits.sh +++ b/t/t3508-cherry-pick-many-commits.sh @@ -5,7 +5,7 @@ test_description='test cherry-picking many commits' . ./test-lib.sh check_head_differs_from() { - ! test_cmp_rev HEAD "$1" + test_cmp_rev ! HEAD "$1" } check_head_equals() { diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index b299ecc326..efcd96fc9e 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -1012,19 +1012,30 @@ test_must_be_empty () { fi } -# Tests that its two parameters refer to the same revision +# Tests that its two parameters refer to the same revision, or if '!' is +# provided first, that its other two parameters refer to different +# revisions. test_cmp_rev () { + local op='=' wrong_result=different + + if test $# -ge 1 && test "x$1" = 'x!' + then + op='!=' + wrong_result='the same' + shift + fi if test $# != 2 then error "bug in the test script: test_cmp_rev requires two revisions, but got $#" else local r1 r2 r1=$(git rev-parse --verify "$1") && - r2=$(git rev-parse --verify "$2") && - if test "$r1" != "$r2" + r2=$(git rev-parse --verify "$2") || return 1 + + if ! test "$r1" "$op" "$r2" then cat >&4 <<-EOF - error: two revisions point to different objects: + error: two revisions point to $wrong_result objects: '$1': $r1 '$2': $r2 EOF From e8d1eaf9b4b8cec4928962084d07147e0161a88e Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:07:48 -0800 Subject: [PATCH 03/15] t5520: improve test style Improve the test style by removing leading and trailing empty lines within test cases. Also, reformat multi-line subshells to conform to the existing style. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 86 +++++++++++++++++++++++++------------------------ 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index cf4cc32fd0..51d6ce8aec 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -538,7 +538,6 @@ test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-m ' test_expect_success '--rebase with rebased upstream' ' - git remote add -f me . && git checkout copy && git tag copy-orig && @@ -552,7 +551,6 @@ test_expect_success '--rebase with rebased upstream' ' git pull --rebase me copy && test "conflicting modification" = "$(cat file)" && test file = "$(cat file2)" - ' test_expect_success '--rebase -f with rebased upstream' ' @@ -564,14 +562,12 @@ test_expect_success '--rebase -f with rebased upstream' ' ' test_expect_success '--rebase with rebased default upstream' ' - git update-ref refs/remotes/me/copy copy-orig && git checkout --track -b to-rebase2 me/copy && git reset --hard to-rebase-orig && git pull --rebase && test "conflicting modification" = "$(cat file)" && test file = "$(cat file2)" - ' test_expect_success 'rebased upstream + fetch + pull --rebase' ' @@ -588,7 +584,6 @@ test_expect_success 'rebased upstream + fetch + pull --rebase' ' ' test_expect_success 'pull --rebase dies early with dirty working directory' ' - git checkout to-rebase && git update-ref refs/remotes/me/copy copy^ && COPY="$(git rev-parse --verify me/copy)" && @@ -603,16 +598,16 @@ test_expect_success 'pull --rebase dies early with dirty working directory' ' git checkout HEAD -- file && git pull && test "$COPY" != "$(git rev-parse --verify me/copy)" - ' test_expect_success 'pull --rebase works on branch yet to be born' ' git rev-parse master >expect && mkdir empty_repo && - (cd empty_repo && - git init && - git pull --rebase .. master && - git rev-parse HEAD >../actual + ( + cd empty_repo && + git init && + git pull --rebase .. master && + git rev-parse HEAD >../actual ) && test_cmp expect actual ' @@ -646,58 +641,65 @@ test_expect_success 'pull --rebase fails on corrupt HEAD' ' test_expect_success 'setup for detecting upstreamed changes' ' mkdir src && - (cd src && - git init && - printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" > stuff && - git add stuff && - git commit -m "Initial revision" + ( + cd src && + git init && + printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" > stuff && + git add stuff && + git commit -m "Initial revision" ) && git clone src dst && - (cd src && - modify s/5/43/ stuff && - git commit -a -m "5->43" && - modify s/6/42/ stuff && - git commit -a -m "Make it bigger" + ( + cd src && + modify s/5/43/ stuff && + git commit -a -m "5->43" && + modify s/6/42/ stuff && + git commit -a -m "Make it bigger" ) && - (cd dst && - modify s/5/43/ stuff && - git commit -a -m "Independent discovery of 5->43" + ( + cd dst && + modify s/5/43/ stuff && + git commit -a -m "Independent discovery of 5->43" ) ' test_expect_success 'git pull --rebase detects upstreamed changes' ' - (cd dst && - git pull --rebase && - test -z "$(git ls-files -u)" + ( + cd dst && + git pull --rebase && + test -z "$(git ls-files -u)" ) ' test_expect_success 'setup for avoiding reapplying old patches' ' - (cd dst && - test_might_fail git rebase --abort && - git reset --hard origin/master + ( + cd dst && + test_might_fail git rebase --abort && + git reset --hard origin/master ) && git clone --bare src src-replace.git && rm -rf src && mv src-replace.git src && - (cd dst && - modify s/2/22/ stuff && - git commit -a -m "Change 2" && - modify s/3/33/ stuff && - git commit -a -m "Change 3" && - modify s/4/44/ stuff && - git commit -a -m "Change 4" && - git push && + ( + cd dst && + modify s/2/22/ stuff && + git commit -a -m "Change 2" && + modify s/3/33/ stuff && + git commit -a -m "Change 3" && + modify s/4/44/ stuff && + git commit -a -m "Change 4" && + git push && - modify s/44/55/ stuff && - git commit --amend -a -m "Modified Change 4" + modify s/44/55/ stuff && + git commit --amend -a -m "Modified Change 4" ) ' test_expect_success 'git pull --rebase does not reapply old patches' ' - (cd dst && - test_must_fail git pull --rebase && - test 1 = $(find .git/rebase-apply -name "000*" | wc -l) + ( + cd dst && + test_must_fail git pull --rebase && + test 1 = $(find .git/rebase-apply -name "000*" | wc -l) ) ' From 53c62b9810b5318ad33aa263461568f12d8db1dd Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:07:50 -0800 Subject: [PATCH 04/15] t5520: use sq for test case names The usual convention is for test case names to be written between single-quotes. Change all double-quoted test case names to single-quotes except for two test case names that use variables within. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 51d6ce8aec..a3de2e19b6 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -408,7 +408,7 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test new = "$(git show HEAD:file2)" ' -test_expect_success "pull --rebase warns on --verify-signatures" ' +test_expect_success 'pull --rebase warns on --verify-signatures' ' git reset --hard before-rebase && git pull --rebase --verify-signatures . copy 2>err && test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && @@ -416,7 +416,7 @@ test_expect_success "pull --rebase warns on --verify-signatures" ' test_i18ngrep "ignoring --verify-signatures for rebase" err ' -test_expect_success "pull --rebase does not warn on --no-verify-signatures" ' +test_expect_success 'pull --rebase does not warn on --no-verify-signatures' ' git reset --hard before-rebase && git pull --rebase --no-verify-signatures . copy 2>err && test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && From 4c8b046f82cbc468fc28b6e52883a15ee2942ec7 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:07:52 -0800 Subject: [PATCH 05/15] t5520: let sed open its own input We were using a redirection operator to feed input into sed. However, since sed is capable of opening its own files, make sed open its own files instead of redirecting input into it. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index a3de2e19b6..55560ce3cd 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -5,7 +5,7 @@ test_description='pulling into void' . ./test-lib.sh modify () { - sed -e "$1" <"$2" >"$2.x" && + sed -e "$1" "$2" >"$2.x" && mv "$2.x" "$2" } From ceeef863defcb3d48e32915b37f2d748708c9fd4 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:07:55 -0800 Subject: [PATCH 06/15] t5520: replace test -f with test-lib functions Although `test -f` has the same functionality as test_path_is_file(), in the case where test_path_is_file() fails, we get much better debugging information. Replace `test -f` with test_path_is_file() so that future developers will have a better experience debugging these test cases. Also, in the case of `! test -f`, not only should that path not be a file, it shouldn't exist at all so replace it with test_path_is_missing(). Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 55560ce3cd..004d5884cd 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -39,8 +39,8 @@ test_expect_success 'pulling into void' ' cd cloned && git pull .. ) && - test -f file && - test -f cloned/file && + test_path_is_file file && + test_path_is_file cloned/file && test_cmp file cloned/file ' @@ -50,8 +50,8 @@ test_expect_success 'pulling into void using master:master' ' cd cloned-uho && git pull .. master:master ) && - test -f file && - test -f cloned-uho/file && + test_path_is_file file && + test_path_is_file cloned-uho/file && test_cmp file cloned-uho/file ' @@ -99,7 +99,7 @@ test_expect_success 'pulling into void must not create an octopus' ' ( cd cloned-octopus && test_must_fail git pull .. master master && - ! test -f file + test_path_is_missing file ) ' From 93a9bf876ba518464b12a5871965dc11df0012e4 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:07:58 -0800 Subject: [PATCH 07/15] t5520: remove spaces after redirect operator The style for tests in Git is to have the redirect operator attached to the filename with no spaces. Fix test cases where this is not the case. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 004d5884cd..7bb9031140 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -243,10 +243,10 @@ test_expect_success 'fast-forward fails with conflicting work tree' ' test_expect_success '--rebase' ' git branch to-rebase && - echo modified again > file && + echo modified again >file && git commit -m file file && git checkout to-rebase && - echo new > file2 && + echo new >file2 && git add file2 && git commit -m "new file" && git tag before-rebase && @@ -542,10 +542,10 @@ test_expect_success '--rebase with rebased upstream' ' git checkout copy && git tag copy-orig && git reset --hard HEAD^ && - echo conflicting modification > file && + echo conflicting modification >file && git commit -m conflict file && git checkout to-rebase && - echo file > file2 && + echo file >file2 && git commit -m to-rebase file2 && git tag to-rebase-orig && git pull --rebase me copy && @@ -591,7 +591,7 @@ test_expect_success 'pull --rebase dies early with dirty working directory' ' test_config branch.to-rebase.remote me && test_config branch.to-rebase.merge refs/heads/copy && test_config branch.to-rebase.rebase true && - echo dirty >> file && + echo dirty >>file && git add file && test_must_fail git pull && test "$COPY" = "$(git rev-parse --verify me/copy)" && From 3037d3db90c994cb3e5913a54f83acdefc219174 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:08:00 -0800 Subject: [PATCH 08/15] t5520: use test_line_count where possible Instead of rolling our own functionality to test the number of lines a command outputs, use test_line_count() which provides better debugging information in the case of a failure. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 7bb9031140..0ca4867e96 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -699,7 +699,8 @@ test_expect_success 'git pull --rebase does not reapply old patches' ' ( cd dst && test_must_fail git pull --rebase && - test 1 = $(find .git/rebase-apply -name "000*" | wc -l) + find .git/rebase-apply -name "000*" >patches && + test_line_count = 1 patches ) ' From 979f8891cca562f0af7044ff9ade9f3e5caa3be5 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:08:02 -0800 Subject: [PATCH 09/15] t5520: replace test -{n,z} with test-lib functions When wrapping a git command in a command substitution within another command, we throw away the git command's exit code. In case the git command fails, we would like to know about it rather than the failure being silent. Extract git commands so that their exit codes are not lost. Instead of using `test -n` or `test -z`, replace them respectively with invocations of test_file_not_empty() and test_must_be_empty() so that we get better debugging information in the case of a failure. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 0ca4867e96..18225d8430 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -206,15 +206,18 @@ test_expect_success 'fail if the index has unresolved entries' ' test_when_finished "git checkout -f copy && git branch -D third" && test "$(cat file)" = file && test_commit modified2 file && - test -z "$(git ls-files -u)" && + git ls-files -u >unmerged && + test_must_be_empty unmerged && test_must_fail git pull . second && - test -n "$(git ls-files -u)" && + git ls-files -u >unmerged && + test_file_not_empty unmerged && cp file expected && test_must_fail git pull . second 2>err && test_i18ngrep "Pulling is not possible because you have unmerged files." err && test_cmp expected file && git add file && - test -z "$(git ls-files -u)" && + git ls-files -u >unmerged && + test_must_be_empty unmerged && test_must_fail git pull . second 2>err && test_i18ngrep "You have not concluded your merge" err && test_cmp expected file @@ -667,7 +670,8 @@ test_expect_success 'git pull --rebase detects upstreamed changes' ' ( cd dst && git pull --rebase && - test -z "$(git ls-files -u)" + git ls-files -u >untracked && + test_must_be_empty untracked ) ' From dd0f1e767b79009f5edd01d3c917f1cf186e60c9 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:08:05 -0800 Subject: [PATCH 10/15] t5520: use test_cmp_rev where possible In case an invocation of `git rev-list` fails within the command substitution, the failure will be masked. Remove the command substitution and use test_cmp_rev() so that failures can be discovered. This change was done with the following sed expressions: s/test "$(git rev-parse.* \([^)]*\))" = "$(git rev-parse \([^)]*\))"/test_cmp_rev \1 \2/ s/test \([^ ]*\) = "$(git rev-parse.* \([^)]*\))"/test_cmp_rev \1 \2/ s/test "$(git rev-parse.* \([^)]*\))" != "$(git rev-parse.* \([^)]*\))"/test_cmp_rev ! \1 \2/ s/test \([^ ]*\) != "$(git rev-parse.* \([^)]*\))"/test_cmp_rev ! \1 \2/ Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 50 ++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 18225d8430..1af6ea06ee 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -230,7 +230,7 @@ test_expect_success 'fast-forwards working tree if branch head is updated' ' git pull . second:third 2>err && test_i18ngrep "fetch updated the current branch head" err && test "$(cat file)" = modified && - test "$(git rev-parse third)" = "$(git rev-parse second)" + test_cmp_rev third second ' test_expect_success 'fast-forward fails with conflicting work tree' ' @@ -241,7 +241,7 @@ test_expect_success 'fast-forward fails with conflicting work tree' ' test_must_fail git pull . second:third 2>err && test_i18ngrep "Cannot fast-forward your working tree" err && test "$(cat file)" = conflict && - test "$(git rev-parse third)" = "$(git rev-parse second)" + test_cmp_rev third second ' test_expect_success '--rebase' ' @@ -254,7 +254,7 @@ test_expect_success '--rebase' ' git commit -m "new file" && git tag before-rebase && git pull --rebase . copy && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" ' @@ -266,7 +266,7 @@ test_expect_success '--rebase fast forward' ' git checkout to-rebase && git pull --rebase . ff && - test "$(git rev-parse HEAD)" = "$(git rev-parse ff)" && + test_cmp_rev HEAD ff && # The above only validates the result. Did we actually bypass rebase? git reflog -1 >reflog.actual && @@ -290,7 +290,7 @@ test_expect_success '--rebase --autostash fast forward' ' git checkout behind && echo dirty >file && git pull --rebase --autostash . to-rebase-ff && - test "$(git rev-parse HEAD)" = "$(git rev-parse to-rebase-ff)" + test_cmp_rev HEAD to-rebase-ff ' test_expect_success '--rebase with conflicts shows advice' ' @@ -328,7 +328,7 @@ test_expect_success 'failed --rebase shows advice' ' test_expect_success '--rebase fails with multiple branches' ' git reset --hard before-rebase && test_must_fail git pull --rebase . copy master 2>err && - test "$(git rev-parse HEAD)" = "$(git rev-parse before-rebase)" && + test_cmp_rev HEAD before-rebase && test_i18ngrep "Cannot rebase onto multiple branches" err && test modified = "$(git show HEAD:file)" ' @@ -380,7 +380,7 @@ test_expect_success 'pull.rebase' ' git reset --hard before-rebase && test_config pull.rebase true && git pull . copy && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" ' @@ -398,7 +398,7 @@ test_expect_success 'branch.to-rebase.rebase' ' git reset --hard before-rebase && test_config branch.to-rebase.rebase true && git pull . copy && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" ' @@ -407,14 +407,14 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test_config pull.rebase true && test_config branch.to-rebase.rebase false && git pull . copy && - test "$(git rev-parse HEAD^)" != "$(git rev-parse copy)" && + test_cmp_rev ! HEAD^ copy && test new = "$(git show HEAD:file2)" ' test_expect_success 'pull --rebase warns on --verify-signatures' ' git reset --hard before-rebase && git pull --rebase --verify-signatures . copy 2>err && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" && test_i18ngrep "ignoring --verify-signatures for rebase" err ' @@ -422,7 +422,7 @@ test_expect_success 'pull --rebase warns on --verify-signatures' ' test_expect_success 'pull --rebase does not warn on --no-verify-signatures' ' git reset --hard before-rebase && git pull --rebase --no-verify-signatures . copy 2>err && - test "$(git rev-parse HEAD^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^ copy && test new = "$(git show HEAD:file2)" && test_i18ngrep ! "verify-signatures" err ' @@ -443,8 +443,8 @@ test_expect_success 'pull.rebase=false create a new merge commit' ' git reset --hard before-preserve-rebase && test_config pull.rebase false && git pull . copy && - test "$(git rev-parse HEAD^1)" = "$(git rev-parse before-preserve-rebase)" && - test "$(git rev-parse HEAD^2)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^1 before-preserve-rebase && + test_cmp_rev HEAD^2 copy && test file3 = "$(git show HEAD:file3.t)" ' @@ -452,7 +452,7 @@ test_expect_success 'pull.rebase=true flattens keep-merge' ' git reset --hard before-preserve-rebase && test_config pull.rebase true && git pull . copy && - test "$(git rev-parse HEAD^^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^^ copy && test file3 = "$(git show HEAD:file3.t)" ' @@ -460,7 +460,7 @@ test_expect_success 'pull.rebase=1 is treated as true and flattens keep-merge' ' git reset --hard before-preserve-rebase && test_config pull.rebase 1 && git pull . copy && - test "$(git rev-parse HEAD^^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^^ copy && test file3 = "$(git show HEAD:file3.t)" ' @@ -469,8 +469,8 @@ test_expect_success REBASE_P \ git reset --hard before-preserve-rebase && test_config pull.rebase preserve && git pull . copy && - test "$(git rev-parse HEAD^^)" = "$(git rev-parse copy)" && - test "$(git rev-parse HEAD^2)" = "$(git rev-parse keep-merge)" + test_cmp_rev HEAD^^ copy && + test_cmp_rev HEAD^2 keep-merge ' test_expect_success 'pull.rebase=interactive' ' @@ -505,8 +505,8 @@ test_expect_success '--rebase=false create a new merge commit' ' git reset --hard before-preserve-rebase && test_config pull.rebase true && git pull --rebase=false . copy && - test "$(git rev-parse HEAD^1)" = "$(git rev-parse before-preserve-rebase)" && - test "$(git rev-parse HEAD^2)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^1 before-preserve-rebase && + test_cmp_rev HEAD^2 copy && test file3 = "$(git show HEAD:file3.t)" ' @@ -514,7 +514,7 @@ test_expect_success '--rebase=true rebases and flattens keep-merge' ' git reset --hard before-preserve-rebase && test_config pull.rebase preserve && git pull --rebase=true . copy && - test "$(git rev-parse HEAD^^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^^ copy && test file3 = "$(git show HEAD:file3.t)" ' @@ -523,8 +523,8 @@ test_expect_success REBASE_P \ git reset --hard before-preserve-rebase && test_config pull.rebase true && git pull --rebase=preserve . copy && - test "$(git rev-parse HEAD^^)" = "$(git rev-parse copy)" && - test "$(git rev-parse HEAD^2)" = "$(git rev-parse keep-merge)" + test_cmp_rev HEAD^^ copy && + test_cmp_rev HEAD^2 keep-merge ' test_expect_success '--rebase=invalid fails' ' @@ -536,7 +536,7 @@ test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-m git reset --hard before-preserve-rebase && test_config pull.rebase preserve && git pull --rebase . copy && - test "$(git rev-parse HEAD^^)" = "$(git rev-parse copy)" && + test_cmp_rev HEAD^^ copy && test file3 = "$(git show HEAD:file3.t)" ' @@ -597,10 +597,10 @@ test_expect_success 'pull --rebase dies early with dirty working directory' ' echo dirty >>file && git add file && test_must_fail git pull && - test "$COPY" = "$(git rev-parse --verify me/copy)" && + test_cmp_rev "$COPY" me/copy && git checkout HEAD -- file && git pull && - test "$COPY" != "$(git rev-parse --verify me/copy)" + test_cmp_rev ! "$COPY" me/copy ' test_expect_success 'pull --rebase works on branch yet to be born' ' From 5540ed27bcf30869b2f56e421cb9f52520d2c0a0 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:08:07 -0800 Subject: [PATCH 11/15] t5520: test single-line files by git with test_cmp In case an invocation of a git command fails within the command substitution, the failure will be masked. Replace the command substitution with a file-redirection and a call to test_cmp. This change was done with the following GNU sed expressions: s/\(\s*\)test \([^ ]*\) = "$(\(git [^)]*\))"/\1echo \2 >expect \&\&\n\1\3 >actual \&\&\n\1test_cmp expect actual/ s/\(\s*\)test "$(\(git [^)]*\))" = \([^ ]*\)/\1echo \3 >expect \&\&\n\1\2 >actual \&\&\n\1test_cmp expect actual/ A future patch will clean up situations where we have multiple duplicate statements within a test case. This is done to keep this patch purely mechanical. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 64 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 16 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 1af6ea06ee..8b7e7ae55d 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -255,7 +255,9 @@ test_expect_success '--rebase' ' git tag before-rebase && git pull --rebase . copy && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success '--rebase fast forward' ' @@ -330,7 +332,9 @@ test_expect_success '--rebase fails with multiple branches' ' test_must_fail git pull --rebase . copy master 2>err && test_cmp_rev HEAD before-rebase && test_i18ngrep "Cannot rebase onto multiple branches" err && - test modified = "$(git show HEAD:file)" + echo modified >expect && + git show HEAD:file >actual && + test_cmp expect actual ' test_expect_success 'pull --rebase succeeds with dirty working directory and rebase.autostash set' ' @@ -381,7 +385,9 @@ test_expect_success 'pull.rebase' ' test_config pull.rebase true && git pull . copy && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success 'pull --autostash & pull.rebase=true' ' @@ -399,7 +405,9 @@ test_expect_success 'branch.to-rebase.rebase' ' test_config branch.to-rebase.rebase true && git pull . copy && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' @@ -408,14 +416,18 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' test_config branch.to-rebase.rebase false && git pull . copy && test_cmp_rev ! HEAD^ copy && - test new = "$(git show HEAD:file2)" + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual ' test_expect_success 'pull --rebase warns on --verify-signatures' ' git reset --hard before-rebase && git pull --rebase --verify-signatures . copy 2>err && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" && + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual && test_i18ngrep "ignoring --verify-signatures for rebase" err ' @@ -423,7 +435,9 @@ test_expect_success 'pull --rebase does not warn on --no-verify-signatures' ' git reset --hard before-rebase && git pull --rebase --no-verify-signatures . copy 2>err && test_cmp_rev HEAD^ copy && - test new = "$(git show HEAD:file2)" && + echo new >expect && + git show HEAD:file2 >actual && + test_cmp expect actual && test_i18ngrep ! "verify-signatures" err ' @@ -445,7 +459,9 @@ test_expect_success 'pull.rebase=false create a new merge commit' ' git pull . copy && test_cmp_rev HEAD^1 before-preserve-rebase && test_cmp_rev HEAD^2 copy && - test file3 = "$(git show HEAD:file3.t)" + echo file3 >expect && + git show HEAD:file3.t >actual && + test_cmp expect actual ' test_expect_success 'pull.rebase=true flattens keep-merge' ' @@ -453,7 +469,9 @@ test_expect_success 'pull.rebase=true flattens keep-merge' ' test_config pull.rebase true && git pull . copy && test_cmp_rev HEAD^^ copy && - test file3 = "$(git show HEAD:file3.t)" + echo file3 >expect && + git show HEAD:file3.t >actual && + test_cmp expect actual ' test_expect_success 'pull.rebase=1 is treated as true and flattens keep-merge' ' @@ -461,7 +479,9 @@ test_expect_success 'pull.rebase=1 is treated as true and flattens keep-merge' ' test_config pull.rebase 1 && git pull . copy && test_cmp_rev HEAD^^ copy && - test file3 = "$(git show HEAD:file3.t)" + echo file3 >expect && + git show HEAD:file3.t >actual && + test_cmp expect actual ' test_expect_success REBASE_P \ @@ -507,7 +527,9 @@ test_expect_success '--rebase=false create a new merge commit' ' git pull --rebase=false . copy && test_cmp_rev HEAD^1 before-preserve-rebase && test_cmp_rev HEAD^2 copy && - test file3 = "$(git show HEAD:file3.t)" + echo file3 >expect && + git show HEAD:file3.t >actual && + test_cmp expect actual ' test_expect_success '--rebase=true rebases and flattens keep-merge' ' @@ -515,7 +537,9 @@ test_expect_success '--rebase=true rebases and flattens keep-merge' ' test_config pull.rebase preserve && git pull --rebase=true . copy && test_cmp_rev HEAD^^ copy && - test file3 = "$(git show HEAD:file3.t)" + echo file3 >expect && + git show HEAD:file3.t >actual && + test_cmp expect actual ' test_expect_success REBASE_P \ @@ -537,7 +561,9 @@ test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-m test_config pull.rebase preserve && git pull --rebase . copy && test_cmp_rev HEAD^^ copy && - test file3 = "$(git show HEAD:file3.t)" + echo file3 >expect && + git show HEAD:file3.t >actual && + test_cmp expect actual ' test_expect_success '--rebase with rebased upstream' ' @@ -622,10 +648,16 @@ test_expect_success 'pull --rebase fails on unborn branch with staged changes' ' cd empty_repo2 && echo staged-file >staged-file && git add staged-file && - test "$(git ls-files)" = staged-file && + echo staged-file >expect && + git ls-files >actual && + test_cmp expect actual && test_must_fail git pull --rebase .. master 2>err && - test "$(git ls-files)" = staged-file && - test "$(git show :staged-file)" = staged-file && + echo staged-file >expect && + git ls-files >actual && + test_cmp expect actual && + echo staged-file >expect && + git show :staged-file >actual && + test_cmp expect actual && test_i18ngrep "unborn branch with changes added to the index" err ) ' From e959a18ee73d3e16c4ac2f300d61755e6b022bd7 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:08:10 -0800 Subject: [PATCH 12/15] t5520: don't put git in upstream of pipe Before, if the invocation of git failed, it would be masked by the pipe since only the return code of the last element of a pipe is used. Rewrite the test to put the git command on its own line so its return code is not masked. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 8b7e7ae55d..8ddf89e550 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -668,7 +668,8 @@ test_expect_success 'pull --rebase fails on corrupt HEAD' ' ( cd corrupt && test_commit one && - obj=$(git rev-parse --verify HEAD | sed "s#^..#&/#") && + git rev-parse --verify HEAD >head && + obj=$(sed "s#^..#&/#" head) && rm -f .git/objects/$obj && test_must_fail git pull --rebase ) From a1a64fdd0aa6e3fd450c06885f30225ebea6b74f Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:08:12 -0800 Subject: [PATCH 13/15] t5520: replace $(cat ...) comparison with test_cmp We currently have many instances of `test = $(cat )` and `test $(cat ) = `. In the case where this fails, it will be difficult for a developer to debug since the output will be masked. Replace these instances with invocations of test_cmp(). This change was done with the following GNU sed expressions: s/\(\s*\)test \([^=]*\)= "$(cat \([^)]*\))"/\1echo \2>expect \&\&\n\1test_cmp expect \3/ s/\(\s*\)test "$(cat \([^)]*\))" = \([^&]*\)\( &&\)\?$/\1echo \3 >expect \&\&\n\1test_cmp expect \2\4/ A future patch will clean up situations where we have multiple duplicate statements within a test case. This is done to keep this patch purely mechanical. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 105 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 70 insertions(+), 35 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 8ddf89e550..c9e4eec004 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -15,8 +15,10 @@ test_pull_autostash () { git add new_file && git pull "$@" . copy && test_cmp_rev HEAD^ copy && - test "$(cat new_file)" = dirty && - test "$(cat file)" = "modified again" + echo dirty >expect && + test_cmp expect new_file && + echo "modified again" >expect && + test_cmp expect file } test_pull_autostash_fail () { @@ -110,9 +112,11 @@ test_expect_success 'test . as a remote' ' echo updated >file && git commit -a -m updated && git checkout copy && - test "$(cat file)" = file && + echo file >expect && + test_cmp expect file && git pull && - test "$(cat file)" = updated && + echo updated >expect && + test_cmp expect file && git reflog -1 >reflog.actual && sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy && echo "OBJID HEAD@{0}: pull: Fast-forward" >reflog.expected && @@ -125,9 +129,11 @@ test_expect_success 'the default remote . should not break explicit pull' ' git commit -a -m modified && git checkout copy && git reset --hard HEAD^ && - test "$(cat file)" = file && + echo file >expect && + test_cmp expect file && git pull . second && - test "$(cat file)" = modified && + echo modified >expect && + test_cmp expect file && git reflog -1 >reflog.actual && sed "s/^[0-9a-f][0-9a-f]*/OBJID/" reflog.actual >reflog.fuzzy && echo "OBJID HEAD@{0}: pull . second: Fast-forward" >reflog.expected && @@ -137,10 +143,12 @@ test_expect_success 'the default remote . should not break explicit pull' ' test_expect_success 'fail if wildcard spec does not match any refs' ' git checkout -b test copy^ && test_when_finished "git checkout -f copy && git branch -D test" && - test "$(cat file)" = file && + echo file >expect && + test_cmp expect file && test_must_fail git pull . "refs/nonexisting1/*:refs/nonexisting2/*" 2>err && test_i18ngrep "no candidates for merging" err && - test "$(cat file)" = file + echo file >expect && + test_cmp expect file ' test_expect_success 'fail if no branches specified with non-default remote' ' @@ -148,11 +156,13 @@ test_expect_success 'fail if no branches specified with non-default remote' ' test_when_finished "git remote remove test_remote" && git checkout -b test copy^ && test_when_finished "git checkout -f copy && git branch -D test" && - test "$(cat file)" = file && + echo file >expect && + test_cmp expect file && test_config branch.test.remote origin && test_must_fail git pull test_remote 2>err && test_i18ngrep "specify a branch on the command line" err && - test "$(cat file)" = file + echo file >expect && + test_cmp expect file ' test_expect_success 'fail if not on a branch' ' @@ -160,10 +170,12 @@ test_expect_success 'fail if not on a branch' ' test_when_finished "git remote remove origin" && git checkout HEAD^ && test_when_finished "git checkout -f copy" && - test "$(cat file)" = file && + echo file >expect && + test_cmp expect file && test_must_fail git pull 2>err && test_i18ngrep "not currently on a branch" err && - test "$(cat file)" = file + echo file >expect && + test_cmp expect file ' test_expect_success 'fail if no configuration for current branch' ' @@ -172,10 +184,12 @@ test_expect_success 'fail if no configuration for current branch' ' git checkout -b test copy^ && test_when_finished "git checkout -f copy && git branch -D test" && test_config branch.test.remote test_remote && - test "$(cat file)" = file && + echo file >expect && + test_cmp expect file && test_must_fail git pull 2>err && test_i18ngrep "no tracking information" err && - test "$(cat file)" = file + echo file >expect && + test_cmp expect file ' test_expect_success 'pull --all: fail if no configuration for current branch' ' @@ -184,10 +198,12 @@ test_expect_success 'pull --all: fail if no configuration for current branch' ' git checkout -b test copy^ && test_when_finished "git checkout -f copy && git branch -D test" && test_config branch.test.remote test_remote && - test "$(cat file)" = file && + echo file >expect && + test_cmp expect file && test_must_fail git pull --all 2>err && test_i18ngrep "There is no tracking information" err && - test "$(cat file)" = file + echo file >expect && + test_cmp expect file ' test_expect_success 'fail if upstream branch does not exist' ' @@ -195,16 +211,19 @@ test_expect_success 'fail if upstream branch does not exist' ' test_when_finished "git checkout -f copy && git branch -D test" && test_config branch.test.remote . && test_config branch.test.merge refs/heads/nonexisting && - test "$(cat file)" = file && + echo file >expect && + test_cmp expect file && test_must_fail git pull 2>err && test_i18ngrep "no such ref was fetched" err && - test "$(cat file)" = file + echo file >expect && + test_cmp expect file ' test_expect_success 'fail if the index has unresolved entries' ' git checkout -b third second^ && test_when_finished "git checkout -f copy && git branch -D third" && - test "$(cat file)" = file && + echo file >expect && + test_cmp expect file && test_commit modified2 file && git ls-files -u >unmerged && test_must_be_empty unmerged && @@ -226,21 +245,25 @@ test_expect_success 'fail if the index has unresolved entries' ' test_expect_success 'fast-forwards working tree if branch head is updated' ' git checkout -b third second^ && test_when_finished "git checkout -f copy && git branch -D third" && - test "$(cat file)" = file && + echo file >expect && + test_cmp expect file && git pull . second:third 2>err && test_i18ngrep "fetch updated the current branch head" err && - test "$(cat file)" = modified && + echo modified >expect && + test_cmp expect file && test_cmp_rev third second ' test_expect_success 'fast-forward fails with conflicting work tree' ' git checkout -b third second^ && test_when_finished "git checkout -f copy && git branch -D third" && - test "$(cat file)" = file && + echo file >expect && + test_cmp expect file && echo conflict >file && test_must_fail git pull . second:third 2>err && test_i18ngrep "Cannot fast-forward your working tree" err && - test "$(cat file)" = conflict && + echo conflict >expect && + test_cmp expect file && test_cmp_rev third second ' @@ -501,7 +524,8 @@ test_expect_success 'pull.rebase=interactive' ' test_set_editor "$TRASH_DIRECTORY/fake-editor" && test_when_finished "test_might_fail git rebase --abort" && test_must_fail git pull --rebase=interactive . copy && - test "I was here" = "$(cat fake.out)" + echo "I was here" >expect && + test_cmp expect fake.out ' test_expect_success 'pull --rebase=i' ' @@ -512,7 +536,8 @@ test_expect_success 'pull --rebase=i' ' test_set_editor "$TRASH_DIRECTORY/fake-editor" && test_when_finished "test_might_fail git rebase --abort" && test_must_fail git pull --rebase=i . copy && - test "I was here, too" = "$(cat fake.out)" + echo "I was here, too" >expect && + test_cmp expect fake.out ' test_expect_success 'pull.rebase=invalid fails' ' @@ -578,16 +603,20 @@ test_expect_success '--rebase with rebased upstream' ' git commit -m to-rebase file2 && git tag to-rebase-orig && git pull --rebase me copy && - test "conflicting modification" = "$(cat file)" && - test file = "$(cat file2)" + echo "conflicting modification" >expect && + test_cmp expect file && + echo file >expect && + test_cmp expect file2 ' test_expect_success '--rebase -f with rebased upstream' ' test_when_finished "test_might_fail git rebase --abort" && git reset --hard to-rebase-orig && git pull --rebase -f me copy && - test "conflicting modification" = "$(cat file)" && - test file = "$(cat file2)" + echo "conflicting modification" >expect && + test_cmp expect file && + echo file >expect && + test_cmp expect file2 ' test_expect_success '--rebase with rebased default upstream' ' @@ -595,8 +624,10 @@ test_expect_success '--rebase with rebased default upstream' ' git checkout --track -b to-rebase2 me/copy && git reset --hard to-rebase-orig && git pull --rebase && - test "conflicting modification" = "$(cat file)" && - test file = "$(cat file2)" + echo "conflicting modification" >expect && + test_cmp expect file && + echo file >expect && + test_cmp expect file2 ' test_expect_success 'rebased upstream + fetch + pull --rebase' ' @@ -607,8 +638,10 @@ test_expect_success 'rebased upstream + fetch + pull --rebase' ' git reset --hard to-rebase-orig && git fetch && git pull --rebase && - test "conflicting modification" = "$(cat file)" && - test file = "$(cat file2)" + echo "conflicting modification" >expect && + test_cmp expect file && + echo file >expect && + test_cmp expect file2 ' @@ -744,8 +777,10 @@ test_expect_success 'git pull --rebase does not reapply old patches' ' test_expect_success 'git pull --rebase against local branch' ' git checkout -b copy2 to-rebase-orig && git pull --rebase . to-rebase && - test "conflicting modification" = "$(cat file)" && - test file = "$(cat file2)" + echo "conflicting modification" >expect && + test_cmp expect file && + echo file >expect && + test_cmp expect file2 ' test_done From c245e58bb6a76e807f46b312e9a4edb2ff76f2b6 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:08:14 -0800 Subject: [PATCH 14/15] t5520: remove redundant lines in test cases In the previous patches, the mechanical application of changes left some duplicate statements in the test case which were not strictly incorrect but were redundant and possibly misleading. Remove these duplicate statements so that it is clear that the intent behind the tests are that the content of the file stays the same throughout the whole test case. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 8 -------- 1 file changed, 8 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index c9e4eec004..ef3dbc201a 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -147,7 +147,6 @@ test_expect_success 'fail if wildcard spec does not match any refs' ' test_cmp expect file && test_must_fail git pull . "refs/nonexisting1/*:refs/nonexisting2/*" 2>err && test_i18ngrep "no candidates for merging" err && - echo file >expect && test_cmp expect file ' @@ -161,7 +160,6 @@ test_expect_success 'fail if no branches specified with non-default remote' ' test_config branch.test.remote origin && test_must_fail git pull test_remote 2>err && test_i18ngrep "specify a branch on the command line" err && - echo file >expect && test_cmp expect file ' @@ -174,7 +172,6 @@ test_expect_success 'fail if not on a branch' ' test_cmp expect file && test_must_fail git pull 2>err && test_i18ngrep "not currently on a branch" err && - echo file >expect && test_cmp expect file ' @@ -188,7 +185,6 @@ test_expect_success 'fail if no configuration for current branch' ' test_cmp expect file && test_must_fail git pull 2>err && test_i18ngrep "no tracking information" err && - echo file >expect && test_cmp expect file ' @@ -202,7 +198,6 @@ test_expect_success 'pull --all: fail if no configuration for current branch' ' test_cmp expect file && test_must_fail git pull --all 2>err && test_i18ngrep "There is no tracking information" err && - echo file >expect && test_cmp expect file ' @@ -215,7 +210,6 @@ test_expect_success 'fail if upstream branch does not exist' ' test_cmp expect file && test_must_fail git pull 2>err && test_i18ngrep "no such ref was fetched" err && - echo file >expect && test_cmp expect file ' @@ -685,10 +679,8 @@ test_expect_success 'pull --rebase fails on unborn branch with staged changes' ' git ls-files >actual && test_cmp expect actual && test_must_fail git pull --rebase .. master 2>err && - echo staged-file >expect && git ls-files >actual && test_cmp expect actual && - echo staged-file >expect && git show :staged-file >actual && test_cmp expect actual && test_i18ngrep "unborn branch with changes added to the index" err From 2a02262078e63dd2b90b2ab40ff024eccb444a48 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Tue, 12 Nov 2019 15:08:16 -0800 Subject: [PATCH 15/15] t5520: replace `! git` with `test_must_fail git` Currently, if a git command fails in an unexpected way, such as a segfault, it will be masked and ignored. Replace the ! with test_must_fail so that only expected failures pass. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t5520-pull.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index ef3dbc201a..602d996a33 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -537,7 +537,7 @@ test_expect_success 'pull --rebase=i' ' test_expect_success 'pull.rebase=invalid fails' ' git reset --hard before-preserve-rebase && test_config pull.rebase invalid && - ! git pull . copy + test_must_fail git pull . copy ' test_expect_success '--rebase=false create a new merge commit' ' @@ -572,7 +572,7 @@ test_expect_success REBASE_P \ test_expect_success '--rebase=invalid fails' ' git reset --hard before-preserve-rebase && - ! git pull --rebase=invalid . copy + test_must_fail git pull --rebase=invalid . copy ' test_expect_success '--rebase overrides pull.rebase=preserve and flattens keep-merge' '