From 0df90bdd12f57b03ac3ff7d16926c56a8f3d4682 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 8 Aug 2018 09:31:03 -0700 Subject: [PATCH 1/5] t7406: fix call that was failing for the wrong reason A test making use of test_must_fail was failing like this: fatal: ambiguous argument '|': unknown revision or path not in the working tree. when the intent was to verify that a specific string was not found in the output of the git diff command, i.e. that grep returned non-zero. Fix the test to do that. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t7406-submodule-update.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index f604ef7a72..ccdc2f9039 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -599,7 +599,8 @@ test_expect_success 'submodule update - update=none in .git/config but --checkou ) && git diff --raw | grep " submodule" && git submodule update --checkout && - test_must_fail git diff --raw \| grep " submodule" && + git diff --raw >out && + ! grep " submodule" out && (cd submodule && test_must_fail compare_head ) && From 602813cff3798f63b7f1f8c9a362e4da05772cad Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 8 Aug 2018 09:31:04 -0700 Subject: [PATCH 2/5] t7406: simplify by using diff --name-only instead of diff --raw We can get rid of some quoted tabs and make a few tests slightly easier to read and edit by just asking for the names of the files modified, since that's all these tests were interested in anyway. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t7406-submodule-update.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index ccdc2f9039..f821b01f15 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -579,9 +579,9 @@ test_expect_success 'submodule update - update=none in .git/config' ' git checkout master && compare_head ) && - git diff --raw | grep " submodule" && + git diff --name-only | grep ^submodule$ && git submodule update && - git diff --raw | grep " submodule" && + git diff --name-only | grep ^submodule$ && (cd submodule && compare_head ) && @@ -597,10 +597,10 @@ test_expect_success 'submodule update - update=none in .git/config but --checkou git checkout master && compare_head ) && - git diff --raw | grep " submodule" && + git diff --name-only | grep ^submodule$ && git submodule update --checkout && - git diff --raw >out && - ! grep " submodule" out && + git diff --name-only >out && + ! grep ^submodule$ out && (cd submodule && test_must_fail compare_head ) && From 65799fbca72a251c75b087e7d018f367a4f7794f Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 8 Aug 2018 09:31:05 -0700 Subject: [PATCH 3/5] t7406: avoid having git commands upstream of a pipe When a git command is on the left side of a pipe, the pipe will swallow its exit status, preventing us from detecting failures in said commands. Restructure the tests to put the output in a temporary file to avoid this problem. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t7406-submodule-update.sh | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index f821b01f15..2463863a51 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -481,7 +481,8 @@ test_expect_success 'recursive submodule update - command in .git/config catches test_expect_success 'submodule init does not copy command into .git/config' ' (cd super && - H=$(git ls-files -s submodule | cut -d" " -f2) && + git ls-files -s submodule >out && + H=$(cut -d" " -f2 out) && mkdir submodule1 && git update-index --add --cacheinfo 160000 $H submodule1 && git config -f .gitmodules submodule.submodule1.path submodule1 && @@ -579,9 +580,11 @@ test_expect_success 'submodule update - update=none in .git/config' ' git checkout master && compare_head ) && - git diff --name-only | grep ^submodule$ && + git diff --name-only >out && + grep ^submodule$ out && git submodule update && - git diff --name-only | grep ^submodule$ && + git diff --name-only >out && + grep ^submodule$ out && (cd submodule && compare_head ) && @@ -597,7 +600,8 @@ test_expect_success 'submodule update - update=none in .git/config but --checkou git checkout master && compare_head ) && - git diff --name-only | grep ^submodule$ && + git diff --name-only >out && + grep ^submodule$ out && git submodule update --checkout && git diff --name-only >out && ! grep ^submodule$ out && @@ -886,7 +890,8 @@ test_expect_success 'submodule update properly revives a moved submodule' ' H=$(git rev-parse --short HEAD) && git commit -am "pre move" && H2=$(git rev-parse --short HEAD) && - git status | sed "s/$H/XXX/" >expect && + git status >out && + sed "s/$H/XXX/" out >expect && H=$(cd submodule2 && git rev-parse HEAD) && git rm --cached submodule2 && rm -rf submodule2 && @@ -895,7 +900,8 @@ test_expect_success 'submodule update properly revives a moved submodule' ' git config -f .gitmodules submodule.submodule2.path "moved/sub module" && git commit -am "post move" && git submodule update && - git status | sed "s/$H2/XXX/" >actual && + git status > out && + sed "s/$H2/XXX/" out >actual && test_cmp expect actual ) ' @@ -913,7 +919,7 @@ test_expect_success SYMLINKS 'submodule update can handle symbolic links in pwd' test_expect_success 'submodule update clone shallow submodule' ' test_when_finished "rm -rf super3" && - first=$(git -C cloned submodule status submodule |cut -c2-41) && + first=$(git -C cloned rev-parse HEAD:submodule) && second=$(git -C submodule rev-parse HEAD) && commit_count=$(git -C submodule rev-list --count $first^..$second) && git clone cloned super3 && @@ -923,7 +929,8 @@ test_expect_success 'submodule update clone shallow submodule' ' sed -e "s#url = ../#url = file://$pwd/#" <.gitmodules >.gitmodules.tmp && mv -f .gitmodules.tmp .gitmodules && git submodule update --init --depth=$commit_count && - test 1 = $(git -C submodule log --oneline | wc -l) + git -C submodule log --oneline >out && + test_line_count = 1 out ) ' @@ -939,7 +946,8 @@ test_expect_success 'submodule update clone shallow submodule outside of depth' test_i18ngrep "Direct fetching of that commit failed." actual && git -C ../submodule config uploadpack.allowReachableSHA1InWant true && git submodule update --init --depth=1 >actual && - test 1 = $(git -C submodule log --oneline | wc -l) + git -C submodule log --oneline >out && + test_line_count = 1 out ) ' From 7e9055bb0062fa7163b55b56fdc7532a04685862 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 8 Aug 2018 09:31:06 -0700 Subject: [PATCH 4/5] t7406: prefer test_* helper functions to test -[feds] test -e, test -s, etc. do not provide nice error messages when we hit test failures, so use the test_* helper functions from test-lib-functions.sh. Also, add test_path_exists() to test-lib-function.sh while at it, so that we don't need to worry whether submodule/.git is a file or a directory. It currently is a file with contents of the form gitdir: ../.git/modules/submodule but it could be changed in the future to be a directory; this test only really cares that it exists. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t7406-submodule-update.sh | 6 +++--- t/test-lib-functions.sh | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 2463863a51..24aa732312 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -174,7 +174,7 @@ test_expect_success 'submodule update does not fetch already present commits' ' git submodule update > ../actual 2> ../actual.err ) && test_i18ncmp expected actual && - ! test -s actual.err + test_must_be_empty actual.err ' test_expect_success 'submodule update should fail due to local changes' ' @@ -620,8 +620,8 @@ test_expect_success 'submodule update --init skips submodule with update=none' ' git clone super cloned && (cd cloned && git submodule update --init && - test -e submodule/.git && - test_must_fail test -e none/.git + test_path_exists submodule/.git && + test_path_is_missing none/.git ) ' diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 2b2181dca0..4207af4077 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -565,6 +565,14 @@ test_path_is_dir () { fi } +test_path_exists () { + if ! test -e "$1" + then + echo "Path $1 doesn't exist. $2" + false + fi +} + # Check if the directory exists and is empty as expected, barf otherwise. test_dir_is_empty () { test_path_is_dir "$1" && From 9fd1080a2d0493c0cb5c7779709dc46b234f014f Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 8 Aug 2018 09:31:07 -0700 Subject: [PATCH 5/5] t7406: avoid using test_must_fail for commands other than git Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- t/t7406-submodule-update.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 24aa732312..815b60cb59 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -606,7 +606,7 @@ test_expect_success 'submodule update - update=none in .git/config but --checkou git diff --name-only >out && ! grep ^submodule$ out && (cd submodule && - test_must_fail compare_head + ! compare_head ) && git config --unset submodule.submodule.update )