From b8afb908c261752310d4792f901c68fe27b77b40 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:15:48 -0800 Subject: [PATCH 01/15] t/lib-git-p4: use test_path_is_missing() Previously, cleanup_git() would 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/lib-git-p4.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh index 547b9f88e1..5aff2abe8b 100644 --- a/t/lib-git-p4.sh +++ b/t/lib-git-p4.sh @@ -175,7 +175,7 @@ stop_and_cleanup_p4d () { cleanup_git () { retry_until_success rm -r "$git" - test_must_fail test -d "$git" && + test_path_is_missing "$git" && retry_until_success mkdir "$git" } From 7717242014dc59f58db7e56c2b8f9fc79e04fa0b Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:15:49 -0800 Subject: [PATCH 02/15] t0000: replace test_must_fail with run_sub_test_lib_test_err() The test_must_fail function should only be used for git commands since we should assume that external commands work sanely. We use test_must_fail to test run_sub_test_lib_test() but that function does not invoke any git commands internally. Even better, we have a function that's exactly meant to be used when we expect to have a failing test suite: run_sub_test_lib_test_err()! Replace `test_must_fail run_sub_test_lib_test` with `run_sub_test_lib_test_err`. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t0000-basic.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index 4d3f7ba295..acb9c4f98b 100755 --- a/t/t0000-basic.sh +++ b/t/t0000-basic.sh @@ -154,7 +154,7 @@ test_expect_success 'pretend we have a fully passing test suite' " " test_expect_success 'pretend we have a partially passing test suite' " - test_must_fail run_sub_test_lib_test \ + run_sub_test_lib_test_err \ partial-pass '2/3 tests passing' <<-\\EOF && test_expect_success 'passing test #1' 'true' test_expect_success 'failing test #2' 'false' @@ -218,7 +218,7 @@ test_expect_success 'pretend we have fixed one of two known breakages (run in su " test_expect_success 'pretend we have a pass, fail, and known breakage' " - test_must_fail run_sub_test_lib_test \ + run_sub_test_lib_test_err \ mixed-results1 'mixed results #1' <<-\\EOF && test_expect_success 'passing test' 'true' test_expect_success 'failing test' 'false' @@ -237,7 +237,7 @@ test_expect_success 'pretend we have a pass, fail, and known breakage' " " test_expect_success 'pretend we have a mix of all possible results' " - test_must_fail run_sub_test_lib_test \ + run_sub_test_lib_test_err \ mixed-results2 'mixed results #2' <<-\\EOF && test_expect_success 'passing test' 'true' test_expect_success 'passing test' 'true' @@ -273,7 +273,7 @@ test_expect_success 'pretend we have a mix of all possible results' " " test_expect_success C_LOCALE_OUTPUT 'test --verbose' ' - test_must_fail run_sub_test_lib_test \ + run_sub_test_lib_test_err \ t1234-verbose "test verbose" --verbose <<-\EOF && test_expect_success "passing test" true test_expect_success "test with output" "echo foo" @@ -300,7 +300,7 @@ test_expect_success C_LOCALE_OUTPUT 'test --verbose' ' ' test_expect_success 'test --verbose-only' ' - test_must_fail run_sub_test_lib_test \ + run_sub_test_lib_test_err \ t2345-verbose-only-2 "test verbose-only=2" \ --verbose-only=2 <<-\EOF && test_expect_success "passing test" true @@ -833,7 +833,7 @@ then fi test_expect_success 'tests clean up even on failures' " - test_must_fail run_sub_test_lib_test \ + run_sub_test_lib_test_err \ failing-cleanup 'Failing tests with cleanup commands' <<-\\EOF && test_expect_success 'tests clean up even after a failure' ' touch clean-after-failure && @@ -862,7 +862,7 @@ test_expect_success 'tests clean up even on failures' " " test_expect_success 'test_atexit is run' " - test_must_fail run_sub_test_lib_test \ + run_sub_test_lib_test_err \ atexit-cleanup 'Run atexit commands' -i <<-\\EOF && test_expect_success 'tests clean up even after a failure' ' > ../../clean-atexit && From 3738439c7779acf2dafadbbc488bb222c94a2b12 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:15:50 -0800 Subject: [PATCH 03/15] t0003: use named parameters in attr_check() We had named the parameters in attr_check() but $2 was being used instead of $expect. Make all variable accesses in attr_check() use named variables instead of numbered arguments for clarity. While we're at it, add variable assignments to the &&-chain. These aren't ever expected to fail but if a future developer ever adds some code above the assignments and they could fail in some way, the intact &&-chain will ensure that the failure is caught. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t0003-attributes.sh | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index 71e63d8b50..3569bef75d 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -5,19 +5,16 @@ test_description=gitattributes . ./test-lib.sh attr_check () { - path="$1" expect="$2" + path="$1" expect="$2" git_opts="$3" && - git $3 check-attr test -- "$path" >actual 2>err && - echo "$path: test: $2" >expect && + git $git_opts check-attr test -- "$path" >actual 2>err && + echo "$path: test: $expect" >expect && test_cmp expect actual && test_line_count = 0 err } attr_check_quote () { - - path="$1" - quoted_path="$2" - expect="$3" + path="$1" quoted_path="$2" expect="$3" && git check-attr test -- "$path" >actual && echo "\"$quoted_path\": test: $expect" >expect && From 99c049bc4c029471af5efcbd1a4ca1fd4a45b923 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:15:51 -0800 Subject: [PATCH 04/15] t0003: use test_must_be_empty() In several places, we used `test_line_count = 0` to check for an empty file. Although this is correct, it's overkill. Use test_must_be_empty() instead because it's more suited for this purpose. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t0003-attributes.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index 3569bef75d..c30c736d3f 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -10,7 +10,7 @@ attr_check () { git $git_opts check-attr test -- "$path" >actual 2>err && echo "$path: test: $expect" >expect && test_cmp expect actual && - test_line_count = 0 err + test_must_be_empty err } attr_check_quote () { @@ -241,7 +241,7 @@ EOF git check-attr foo -- "a/b/f" >>actual 2>>err && git check-attr foo -- "a/b/c/f" >>actual 2>>err && test_cmp expect actual && - test_line_count = 0 err + test_must_be_empty err ' test_expect_success '"**" with no slashes test' ' @@ -262,7 +262,7 @@ EOF git check-attr foo -- "a/b/f" >>actual 2>>err && git check-attr foo -- "a/b/c/f" >>actual 2>>err && test_cmp expect actual && - test_line_count = 0 err + test_must_be_empty err ' test_expect_success 'using --git-dir and --work-tree' ' From f46c243e669f5f1e6ef2f25000d5a5f593d2e1f3 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:15:52 -0800 Subject: [PATCH 05/15] t0003: don't use `test_must_fail attr_check` In an effort to remove test_must_fail for all invocations not related to git or test-tool, replace invocations of `test_must_fail attr_check` with a plain attr_check call with the $expect argument set to the actual value output by git. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t0003-attributes.sh | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index c30c736d3f..b660593c20 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -24,7 +24,7 @@ attr_check_quote () { test_expect_success 'open-quoted pathname' ' echo "\"a test=a" >.gitattributes && - test_must_fail attr_check a a + attr_check a unspecified ' @@ -109,20 +109,20 @@ test_expect_success 'attribute test' ' test_expect_success 'attribute matching is case sensitive when core.ignorecase=0' ' - test_must_fail attr_check F f "-c core.ignorecase=0" && - test_must_fail attr_check a/F f "-c core.ignorecase=0" && - test_must_fail attr_check a/c/F f "-c core.ignorecase=0" && - test_must_fail attr_check a/G a/g "-c core.ignorecase=0" && - test_must_fail attr_check a/B/g a/b/g "-c core.ignorecase=0" && - test_must_fail attr_check a/b/G a/b/g "-c core.ignorecase=0" && - test_must_fail attr_check a/b/H a/b/h "-c core.ignorecase=0" && - test_must_fail attr_check a/b/D/g "a/b/d/*" "-c core.ignorecase=0" && - test_must_fail attr_check oNoFf unset "-c core.ignorecase=0" && - test_must_fail attr_check oFfOn set "-c core.ignorecase=0" && + attr_check F unspecified "-c core.ignorecase=0" && + attr_check a/F unspecified "-c core.ignorecase=0" && + attr_check a/c/F unspecified "-c core.ignorecase=0" && + attr_check a/G unspecified "-c core.ignorecase=0" && + attr_check a/B/g a/g "-c core.ignorecase=0" && + attr_check a/b/G unspecified "-c core.ignorecase=0" && + attr_check a/b/H unspecified "-c core.ignorecase=0" && + attr_check a/b/D/g a/g "-c core.ignorecase=0" && + attr_check oNoFf unspecified "-c core.ignorecase=0" && + attr_check oFfOn unspecified "-c core.ignorecase=0" && attr_check NO unspecified "-c core.ignorecase=0" && - test_must_fail attr_check a/b/D/NO "a/b/d/*" "-c core.ignorecase=0" && + attr_check a/b/D/NO unspecified "-c core.ignorecase=0" && attr_check a/b/d/YES a/b/d/* "-c core.ignorecase=0" && - test_must_fail attr_check a/E/f "A/e/F" "-c core.ignorecase=0" + attr_check a/E/f f "-c core.ignorecase=0" ' @@ -146,8 +146,8 @@ test_expect_success 'attribute matching is case insensitive when core.ignorecase ' test_expect_success CASE_INSENSITIVE_FS 'additional case insensitivity tests' ' - test_must_fail attr_check a/B/D/g "a/b/d/*" "-c core.ignorecase=0" && - test_must_fail attr_check A/B/D/NO "a/b/d/*" "-c core.ignorecase=0" && + attr_check a/B/D/g a/g "-c core.ignorecase=0" && + attr_check A/B/D/NO unspecified "-c core.ignorecase=0" && attr_check A/b/h a/b/h "-c core.ignorecase=1" && attr_check a/B/D/g "a/b/d/*" "-c core.ignorecase=1" && attr_check A/B/D/NO "a/b/d/*" "-c core.ignorecase=1" From f6041abdcd7bfefed1cbe14cbe221f0e848cbec4 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:15:53 -0800 Subject: [PATCH 06/15] t0020: don't use `test_must_fail has_cr` The test_must_fail function should only be used for git commands since we should assume that external commands work sanely. Since has_cr() just wraps a tr and grep pipeline, replace `test_must_fail has_cr` with `! has_cr`. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t0020-crlf.sh | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh index 854da0ae16..b63ba62e5d 100755 --- a/t/t0020-crlf.sh +++ b/t/t0020-crlf.sh @@ -159,8 +159,8 @@ test_expect_success 'checkout with autocrlf=input' ' rm -f tmp one dir/two three && git config core.autocrlf input && git read-tree --reset -u HEAD && - test_must_fail has_cr one && - test_must_fail has_cr dir/two && + ! has_cr one && + ! has_cr dir/two && git update-index -- one dir/two && test "$one" = $(git hash-object --stdin .gitattributes && git read-tree --reset -u HEAD && - test_must_fail has_cr dir/two + ! has_cr dir/two ' test_expect_success '.gitattributes says two and three are text' ' @@ -270,7 +270,7 @@ test_expect_success 'in-tree .gitattributes (1)' ' rm -rf tmp one dir .gitattributes patch.file three && git read-tree --reset -u HEAD && - test_must_fail has_cr one && + ! has_cr one && verbose has_cr three ' @@ -280,7 +280,7 @@ test_expect_success 'in-tree .gitattributes (2)' ' git read-tree --reset HEAD && git checkout-index -f -q -u -a && - test_must_fail has_cr one && + ! has_cr one && verbose has_cr three ' @@ -291,7 +291,7 @@ test_expect_success 'in-tree .gitattributes (3)' ' git checkout-index -u .gitattributes && git checkout-index -u one dir/two three && - test_must_fail has_cr one && + ! has_cr one && verbose has_cr three ' @@ -302,7 +302,7 @@ test_expect_success 'in-tree .gitattributes (4)' ' git checkout-index -u one dir/two three && git checkout-index -u .gitattributes && - test_must_fail has_cr one && + ! has_cr one && verbose has_cr three ' From f511bc02edc8e2729e7babb1b4caa98684d941bc Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:15:54 -0800 Subject: [PATCH 07/15] t0020: use ! check_packed_refs_marked The test_must_fail function should only be used for git commands since we should assume that external commands work sanely. Since check_packed_refs_marked() just wraps a grep invocation, replace `test_must_fail check_packed_refs_marked` with `! check_packed_refs_marked`. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t1409-avoid-packing-refs.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t1409-avoid-packing-refs.sh b/t/t1409-avoid-packing-refs.sh index e5cb8a252d..c46848eb8e 100755 --- a/t/t1409-avoid-packing-refs.sh +++ b/t/t1409-avoid-packing-refs.sh @@ -46,7 +46,7 @@ test_expect_success 'check that marking the packed-refs file works' ' git for-each-ref >actual && test_cmp expected actual && git pack-refs --all && - test_must_fail check_packed_refs_marked && + ! check_packed_refs_marked && git for-each-ref >actual2 && test_cmp expected actual2 ' @@ -80,7 +80,7 @@ test_expect_success 'touch packed-refs on delete of packed' ' git pack-refs --all && mark_packed_refs && git update-ref -d refs/heads/packed-delete && - test_must_fail check_packed_refs_marked + ! check_packed_refs_marked ' test_expect_success 'leave packed-refs untouched on update of loose' ' From 3595d10c26db483239d97ca8fc435c1c9dabcfb1 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:15:55 -0800 Subject: [PATCH 08/15] t1306: convert `test_might_fail rm` to `rm -f` The test_must_fail() family of functions (including test_might_fail()) should only be used on git commands. Replace `test_might_fail rm` with `rm -f` so that we don't use `test_might_fail` on a non-git command. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t1306-xdg-files.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh index 21e139a313..dd87b43be1 100755 --- a/t/t1306-xdg-files.sh +++ b/t/t1306-xdg-files.sh @@ -153,7 +153,7 @@ test_expect_success 'Checking attributes in both XDG and local attributes files' test_expect_success 'Checking attributes in a non-XDG global attributes file' ' - test_might_fail rm .gitattributes && + rm -f .gitattributes && echo "f attr_f=test" >"$HOME"/my_gitattributes && git config core.attributesfile "$HOME"/my_gitattributes && echo "f: attr_f: test" >expected && @@ -165,7 +165,7 @@ test_expect_success 'Checking attributes in a non-XDG global attributes file' ' test_expect_success 'write: xdg file exists and ~/.gitconfig doesn'\''t' ' mkdir -p "$HOME"/.config/git && >"$HOME"/.config/git/config && - test_might_fail rm "$HOME"/.gitconfig && + rm -f "$HOME"/.gitconfig && git config --global user.name "write_config" && echo "[user]" >expected && echo " name = write_config" >>expected && @@ -183,8 +183,8 @@ test_expect_success 'write: xdg file exists and ~/.gitconfig exists' ' test_expect_success 'write: ~/.config/git/ exists and config file doesn'\''t' ' - test_might_fail rm "$HOME"/.gitconfig && - test_might_fail rm "$HOME"/.config/git/config && + rm -f "$HOME"/.gitconfig && + rm -f "$HOME"/.config/git/config && git config --global user.name "write_gitconfig" && echo "[user]" >expected && echo " name = write_gitconfig" >>expected && From 9b92070e5231773f564c1fe631cddd6d436963a5 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:15:56 -0800 Subject: [PATCH 09/15] t1307: reorder `nongit test_must_fail` In the future, we plan on only allowing `test_must_fail` to work on a restricted subset of commands, including `git`. Reorder the commands so that `nongit` comes before `test_must_fail`. This way, `test_must_fail` operates on a git command. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t1307-config-blob.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t1307-config-blob.sh b/t/t1307-config-blob.sh index 37dc689d8c..002e6d3388 100755 --- a/t/t1307-config-blob.sh +++ b/t/t1307-config-blob.sh @@ -74,7 +74,7 @@ test_expect_success 'can parse blob ending with CR' ' ' test_expect_success 'config --blob outside of a repository is an error' ' - test_must_fail nongit git config --blob=foo --list + nongit test_must_fail git config --blob=foo --list ' test_done From b87b02cfe6973ac8cc778e0463bc3ad6accb96a6 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:15:57 -0800 Subject: [PATCH 10/15] t1409: let sed open its own input file In one case, we were using a redirection operator to feed input into sed. However, since sed is capable of opening its own input file, make sed do that instead of redirecting input into it. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t1409-avoid-packing-refs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t1409-avoid-packing-refs.sh b/t/t1409-avoid-packing-refs.sh index c46848eb8e..f74d890e82 100755 --- a/t/t1409-avoid-packing-refs.sh +++ b/t/t1409-avoid-packing-refs.sh @@ -8,7 +8,7 @@ test_description='avoid rewriting packed-refs unnecessarily' # shouldn't upset readers, and it should be omitted if the file is # ever rewritten. mark_packed_refs () { - sed -e "s/^\(#.*\)/\1 t1409 /" <.git/packed-refs >.git/packed-refs.new && + sed -e "s/^\(#.*\)/\1 t1409 /" .git/packed-refs >.git/packed-refs.new && mv .git/packed-refs.new .git/packed-refs } From 62d58cda69621eb2f70517c87f1baeeb7b2f398c Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:15:58 -0800 Subject: [PATCH 11/15] t1409: 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 -f` 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/t1409-avoid-packing-refs.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t1409-avoid-packing-refs.sh b/t/t1409-avoid-packing-refs.sh index f74d890e82..be12fb6350 100755 --- a/t/t1409-avoid-packing-refs.sh +++ b/t/t1409-avoid-packing-refs.sh @@ -27,15 +27,15 @@ test_expect_success 'setup' ' ' test_expect_success 'do not create packed-refs file gratuitously' ' - test_must_fail test -f .git/packed-refs && + test_path_is_missing .git/packed-refs && git update-ref refs/heads/foo $A && - test_must_fail test -f .git/packed-refs && + test_path_is_missing .git/packed-refs && git update-ref refs/heads/foo $B && - test_must_fail test -f .git/packed-refs && + test_path_is_missing .git/packed-refs && git update-ref refs/heads/foo $C $B && - test_must_fail test -f .git/packed-refs && + test_path_is_missing .git/packed-refs && git update-ref -d refs/heads/foo && - test_must_fail test -f .git/packed-refs + test_path_is_missing .git/packed-refs ' test_expect_success 'check that marking the packed-refs file works' ' From 10812c2337eb04f2857000fd644efdc02247ca92 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:15:59 -0800 Subject: [PATCH 12/15] t1501: remove use of `test_might_fail cp` The test_must_fail() family of functions (including test_might_fail()) should only be used on git commands. Replace test_might_fail() with a compound command wrapping the old cp invocation that always returns 0. The `test_might_fail cp` line was introduced in 466e8d5d66 (t1501: fix test with split index, 2015-03-24). It is necessary because there might exist some index files in `repo.git/sharedindex.*` and, if they exist, we want to copy them over. However, if they don't exist, we don't want to error out because we expect that possibility. As a result, we want to keep the "might fail" semantics so we always return 0, even if the underlying cp errors out. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t1501-work-tree.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t1501-work-tree.sh b/t/t1501-work-tree.sh index 3498d3d55e..b75558040f 100755 --- a/t/t1501-work-tree.sh +++ b/t/t1501-work-tree.sh @@ -350,7 +350,7 @@ test_expect_success 'Multi-worktree setup' ' mkdir work && mkdir -p repo.git/repos/foo && cp repo.git/HEAD repo.git/index repo.git/repos/foo && - test_might_fail cp repo.git/sharedindex.* repo.git/repos/foo && + { cp repo.git/sharedindex.* repo.git/repos/foo || :; } && sane_unset GIT_DIR GIT_CONFIG GIT_WORK_TREE ' From 5236fce6b4b597588f5e3955b309195f376a858f Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:16:00 -0800 Subject: [PATCH 13/15] t1507: stop losing return codes of git commands The return code of git commands are lost when a command is in a non-assignment command substitution in favour of the surrounding command's. Rewrite instances of this so that git commands run on their own. In commit_subject(), use a `tformat` instead of `format` since, previously, we were testing the output of a command substitution which didn't care if there was a trailing newline since it was automatically stripped. Since we use test_cmp() now, the trailing newline matters so use `tformat` to always output it. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t1507-rev-parse-upstream.sh | 45 +++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh index 8b4cf8a6e3..d81f289ace 100755 --- a/t/t1507-rev-parse-upstream.sh +++ b/t/t1507-rev-parse-upstream.sh @@ -35,7 +35,7 @@ full_name () { commit_subject () { (cd clone && - git show -s --pretty=format:%s "$@") + git show -s --pretty=tformat:%s "$@") } error_message () { @@ -44,18 +44,27 @@ error_message () { } test_expect_success '@{upstream} resolves to correct full name' ' - test refs/remotes/origin/master = "$(full_name @{upstream})" && - test refs/remotes/origin/master = "$(full_name @{UPSTREAM})" && - test refs/remotes/origin/master = "$(full_name @{UpSTReam})" + echo refs/remotes/origin/master >expect && + full_name @{upstream} >actual && + test_cmp expect actual && + full_name @{UPSTREAM} >actual && + test_cmp expect actual && + full_name @{UpSTReam} >actual && + test_cmp expect actual ' test_expect_success '@{u} resolves to correct full name' ' - test refs/remotes/origin/master = "$(full_name @{u})" && - test refs/remotes/origin/master = "$(full_name @{U})" + echo refs/remotes/origin/master >expect && + full_name @{u} >actual && + test_cmp expect actual && + full_name @{U} >actual && + test_cmp expect actual ' test_expect_success 'my-side@{upstream} resolves to correct full name' ' - test refs/remotes/origin/side = "$(full_name my-side@{u})" + echo refs/remotes/origin/side >expect && + full_name my-side@{u} >actual && + test_cmp expect actual ' test_expect_success 'upstream of branch with @ in middle' ' @@ -86,8 +95,11 @@ test_expect_success 'my-side@{u} resolves to correct commit' ' git checkout side && test_commit 5 && (cd clone && git fetch) && - test 2 = "$(commit_subject my-side)" && - test 5 = "$(commit_subject my-side@{u})" + echo 2 >expect && + commit_subject my-side >actual && + test_cmp expect actual && + echo 5 >expect && + commit_subject my-side@{u} >actual ' test_expect_success 'not-tracking@{u} fails' ' @@ -99,8 +111,11 @@ test_expect_success 'not-tracking@{u} fails' ' test_expect_success '@{u}@{1} resolves correctly' ' test_commit 6 && (cd clone && git fetch) && - test 5 = $(commit_subject my-side@{u}@{1}) && - test 5 = $(commit_subject my-side@{U}@{1}) + echo 5 >expect && + commit_subject my-side@{u}@{1} >actual && + test_cmp expect actual && + commit_subject my-side@{U}@{1} >actual && + test_cmp expect actual ' test_expect_success '@{u} without specifying branch fails on a detached HEAD' ' @@ -149,7 +164,9 @@ test_expect_success 'checkout other@{u}' ' ' test_expect_success 'branch@{u} works when tracking a local branch' ' - test refs/heads/master = "$(full_name local-master@{u})" + echo refs/heads/master >expect && + full_name local-master@{u} >actual && + test_cmp expect actual ' test_expect_success 'branch@{u} error message when no upstream' ' @@ -203,7 +220,9 @@ test_expect_success 'pull works when tracking a local branch' ' # makes sense if the previous one succeeded test_expect_success '@{u} works when tracking a local branch' ' - test refs/heads/master = "$(full_name @{u})" + echo refs/heads/master >expect && + full_name @{u} >actual && + test_cmp expect actual ' commit=$(git rev-parse HEAD) From 9291e6329ef3333f9447b0b4cdec0607e00c39cf Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:16:01 -0800 Subject: [PATCH 14/15] t1507: run commands within test_expect_success The expected test style is to have all commands tested within a test_expect_success block. Move the generation of the 'expect' text into their corresponding blocks. While we're at it, insert a second `commit=$(git rev-parse HEAD)` into the next test case so that it's clear where $commit is coming from. The biggest advantage of doing this is that we now check the return code of `git rev-parse HEAD` so we can catch it in case it fails. This patch is best viewed with `--color-moved --ignore-all-space`. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t1507-rev-parse-upstream.sh | 40 +++++++++++++++++------------------ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh index d81f289ace..f68b77e7ba 100755 --- a/t/t1507-rev-parse-upstream.sh +++ b/t/t1507-rev-parse-upstream.sh @@ -225,32 +225,32 @@ test_expect_success '@{u} works when tracking a local branch' ' test_cmp expect actual ' -commit=$(git rev-parse HEAD) -cat >expect <) -Reflog message: branch: Created from HEAD -Author: A U Thor -Date: Thu Apr 7 15:15:13 2005 -0700 - - 3 -EOF test_expect_success 'log -g other@{u}' ' + commit=$(git rev-parse HEAD) && + cat >expect <<-EOF && + commit $commit + Reflog: master@{0} (C O Mitter ) + Reflog message: branch: Created from HEAD + Author: A U Thor + Date: Thu Apr 7 15:15:13 2005 -0700 + + 3 + EOF git log -1 -g other@{u} >actual && test_cmp expect actual ' -cat >expect <) -Reflog message: branch: Created from HEAD -Author: A U Thor -Date: Thu Apr 7 15:15:13 2005 -0700 - - 3 -EOF - test_expect_success 'log -g other@{u}@{now}' ' + commit=$(git rev-parse HEAD) && + cat >expect <<-EOF && + commit $commit + Reflog: master@{Thu Apr 7 15:17:13 2005 -0700} (C O Mitter ) + Reflog message: branch: Created from HEAD + Author: A U Thor + Date: Thu Apr 7 15:15:13 2005 -0700 + + 3 + EOF git log -1 -g other@{u}@{now} >actual && test_cmp expect actual ' From b44171725646880db07cbee6446e15b4afaa1930 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Fri, 20 Dec 2019 10:16:02 -0800 Subject: [PATCH 15/15] t1507: inline full_name() Before, we were running `test_must_fail full_name`. However, `test_must_fail` should only be used on git commands. Inline full_name() so that we can use test_must_fail on the git command directly. When full_name() was introduced in 28fb84382b (Introduce @{upstream} notation, 2009-09-10), the `git -C` option wasn't available yet (since it was introduced in 44e1e4d67d (git: run in a directory given with -C option, 2013-09-09)). As a result, the helper function removed the need to manually cd each time. However, since `git -C` is available now, we can just use that instead and inline full_name(). An alternate approach was taken where we taught full_name() to accept an optional `!` arg to trigger test_must_fail behavior. However, this added more unnecessary complexity than inlining so we inline instead. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t1507-rev-parse-upstream.sh | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/t/t1507-rev-parse-upstream.sh b/t/t1507-rev-parse-upstream.sh index f68b77e7ba..dfc0d96d8a 100755 --- a/t/t1507-rev-parse-upstream.sh +++ b/t/t1507-rev-parse-upstream.sh @@ -28,11 +28,6 @@ test_expect_success 'setup' ' ) ' -full_name () { - (cd clone && - git rev-parse --symbolic-full-name "$@") -} - commit_subject () { (cd clone && git show -s --pretty=tformat:%s "$@") @@ -45,50 +40,50 @@ error_message () { test_expect_success '@{upstream} resolves to correct full name' ' echo refs/remotes/origin/master >expect && - full_name @{upstream} >actual && + git -C clone rev-parse --symbolic-full-name @{upstream} >actual && test_cmp expect actual && - full_name @{UPSTREAM} >actual && + git -C clone rev-parse --symbolic-full-name @{UPSTREAM} >actual && test_cmp expect actual && - full_name @{UpSTReam} >actual && + git -C clone rev-parse --symbolic-full-name @{UpSTReam} >actual && test_cmp expect actual ' test_expect_success '@{u} resolves to correct full name' ' echo refs/remotes/origin/master >expect && - full_name @{u} >actual && + git -C clone rev-parse --symbolic-full-name @{u} >actual && test_cmp expect actual && - full_name @{U} >actual && + git -C clone rev-parse --symbolic-full-name @{U} >actual && test_cmp expect actual ' test_expect_success 'my-side@{upstream} resolves to correct full name' ' echo refs/remotes/origin/side >expect && - full_name my-side@{u} >actual && + git -C clone rev-parse --symbolic-full-name my-side@{u} >actual && test_cmp expect actual ' test_expect_success 'upstream of branch with @ in middle' ' - full_name fun@ny@{u} >actual && + git -C clone rev-parse --symbolic-full-name fun@ny@{u} >actual && echo refs/remotes/origin/side >expect && test_cmp expect actual && - full_name fun@ny@{U} >actual && + git -C clone rev-parse --symbolic-full-name fun@ny@{U} >actual && test_cmp expect actual ' test_expect_success 'upstream of branch with @ at start' ' - full_name @funny@{u} >actual && + git -C clone rev-parse --symbolic-full-name @funny@{u} >actual && echo refs/remotes/origin/side >expect && test_cmp expect actual ' test_expect_success 'upstream of branch with @ at end' ' - full_name funny@@{u} >actual && + git -C clone rev-parse --symbolic-full-name funny@@{u} >actual && echo refs/remotes/origin/side >expect && test_cmp expect actual ' test_expect_success 'refs/heads/my-side@{upstream} does not resolve to my-side{upstream}' ' - test_must_fail full_name refs/heads/my-side@{upstream} + test_must_fail git -C clone rev-parse --symbolic-full-name refs/heads/my-side@{upstream} ' test_expect_success 'my-side@{u} resolves to correct commit' ' @@ -103,9 +98,9 @@ test_expect_success 'my-side@{u} resolves to correct commit' ' ' test_expect_success 'not-tracking@{u} fails' ' - test_must_fail full_name non-tracking@{u} && + test_must_fail git -C clone rev-parse --symbolic-full-name non-tracking@{u} && (cd clone && git checkout --no-track -b non-tracking) && - test_must_fail full_name non-tracking@{u} + test_must_fail git -C clone rev-parse --symbolic-full-name non-tracking@{u} ' test_expect_success '@{u}@{1} resolves correctly' ' @@ -165,7 +160,7 @@ test_expect_success 'checkout other@{u}' ' test_expect_success 'branch@{u} works when tracking a local branch' ' echo refs/heads/master >expect && - full_name local-master@{u} >actual && + git -C clone rev-parse --symbolic-full-name local-master@{u} >actual && test_cmp expect actual ' @@ -221,7 +216,7 @@ test_expect_success 'pull works when tracking a local branch' ' # makes sense if the previous one succeeded test_expect_success '@{u} works when tracking a local branch' ' echo refs/heads/master >expect && - full_name @{u} >actual && + git -C clone rev-parse --symbolic-full-name @{u} >actual && test_cmp expect actual '