From 12b1826609d5c309336ffd9f56e42abaf8d96cef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Thu, 1 Aug 2019 17:53:07 +0200 Subject: [PATCH 1/3] t5510-fetch: run non-httpd-specific test before sourcing 'lib-httpd.sh' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 't5510-fetch.sh' sources 'lib-httpd.sh' near the end to run a httpd-specific test, but 'lib-httpd.sh' skips all the rest of the test script if the dependencies for running httpd tests are not fulfilled. Alas, recently cdbd70c437 (fetch: add --[no-]show-forced-updates argument, 2019-06-18) appended a non-httpd-specific test at the end, and this test is then skipped as well when httpd tests can't be run. Move this new test earlier in the test script, before 'lib-httpd.sh' is sourced, so it will be run even when httpd tests aren't. Also add a comment at the end of this test script to warn against adding non-httpd-specific tests at the end, in the hope that it will help prevent similar issues in the future. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- t/t5510-fetch.sh | 47 +++++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index f2481de577..34b486f1a4 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -902,6 +902,29 @@ test_expect_success C_LOCALE_OUTPUT 'fetch compact output' ' test_cmp expect actual ' +test_expect_success '--no-show-forced-updates' ' + mkdir forced-updates && + ( + cd forced-updates && + git init && + test_commit 1 && + test_commit 2 + ) && + git clone forced-updates forced-update-clone && + git clone forced-updates no-forced-update-clone && + git -C forced-updates reset --hard HEAD~1 && + ( + cd forced-update-clone && + git fetch --show-forced-updates origin 2>output && + test_i18ngrep "(forced update)" output + ) && + ( + cd no-forced-update-clone && + git fetch --no-show-forced-updates origin 2>output && + test_i18ngrep ! "(forced update)" output + ) +' + setup_negotiation_tip () { SERVER="$1" URL="$2" @@ -978,27 +1001,7 @@ test_expect_success '--negotiation-tip limits "have" lines sent with HTTP protoc check_negotiation_tip ' -test_expect_success '--no-show-forced-updates' ' - mkdir forced-updates && - ( - cd forced-updates && - git init && - test_commit 1 && - test_commit 2 - ) && - git clone forced-updates forced-update-clone && - git clone forced-updates no-forced-update-clone && - git -C forced-updates reset --hard HEAD~1 && - ( - cd forced-update-clone && - git fetch --show-forced-updates origin 2>output && - test_i18ngrep "(forced update)" output - ) && - ( - cd no-forced-update-clone && - git fetch --no-show-forced-updates origin 2>output && - test_i18ngrep ! "(forced update)" output - ) -' +# DO NOT add non-httpd-specific tests here, because the last part of this +# test script is only executed when httpd is available and enabled. test_done From 7f005b0f483a507d3c7b353f4753ac3fece88bc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Thu, 1 Aug 2019 17:53:08 +0200 Subject: [PATCH 2/3] t5703: run all non-httpd-specific tests before sourcing 'lib-httpd.sh' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 't5703-upload-pack-ref-in-want.sh' sources 'lib-httpd.sh' near the end to run a couple of httpd-specific tests, but 'lib-httpd.sh' skips all the rest of the test script if the dependencies for running httpd tests are not fulfilled. However, the last six tests in 't5703' are not httpd-specific, but they are skipped as well when httpd tests can't be run. Move these six tests earlier in the test script, before 'lib-httpd.sh' is sourced, so they will be run even when httpd tests aren't. Note that this is not merely a pure code movement, because the setup test case for the httpd tests needed an additional 'rm -rf "$LOCAL_PRISTINE"' to clean up a directory left behind by the moved non-httpd-specific tests. Also add a comment at the end of this test script to warn against adding non-httpd-specific tests at the end, in the hope that it will help prevent similar issues in the future. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- t/t5703-upload-pack-ref-in-want.sh | 204 +++++++++++++++-------------- 1 file changed, 104 insertions(+), 100 deletions(-) diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh index de4b6106ef..3a2c143c6d 100755 --- a/t/t5703-upload-pack-ref-in-want.sh +++ b/t/t5703-upload-pack-ref-in-want.sh @@ -157,106 +157,6 @@ test_expect_success 'want-ref with ref we already have commit for' ' check_output ' -. "$TEST_DIRECTORY"/lib-httpd.sh -start_httpd - -REPO="$HTTPD_DOCUMENT_ROOT_PATH/repo" -LOCAL_PRISTINE="$(pwd)/local_pristine" - -test_expect_success 'setup repos for change-while-negotiating test' ' - ( - git init "$REPO" && - cd "$REPO" && - >.git/git-daemon-export-ok && - test_commit m1 && - git tag -d m1 && - - # Local repo with many commits (so that negotiation will take - # more than 1 request/response pair) - git clone "http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo" "$LOCAL_PRISTINE" && - cd "$LOCAL_PRISTINE" && - git checkout -b side && - test_commit_bulk --id=s 33 && - - # Add novel commits to upstream - git checkout master && - cd "$REPO" && - test_commit m2 && - test_commit m3 && - git tag -d m2 m3 - ) && - git -C "$LOCAL_PRISTINE" remote set-url origin "http://127.0.0.1:$LIB_HTTPD_PORT/one_time_sed/repo" && - git -C "$LOCAL_PRISTINE" config protocol.version 2 -' - -inconsistency () { - # Simulate that the server initially reports $2 as the ref - # corresponding to $1, and after that, $1 as the ref corresponding to - # $1. This corresponds to the real-life situation where the server's - # repository appears to change during negotiation, for example, when - # different servers in a load-balancing arrangement serve (stateless) - # RPCs during a single negotiation. - printf "s/%s/%s/" \ - $(git -C "$REPO" rev-parse $1 | tr -d "\n") \ - $(git -C "$REPO" rev-parse $2 | tr -d "\n") \ - >"$HTTPD_ROOT_PATH/one-time-sed" -} - -test_expect_success 'server is initially ahead - no ref in want' ' - git -C "$REPO" config uploadpack.allowRefInWant false && - rm -rf local && - cp -r "$LOCAL_PRISTINE" local && - inconsistency master 1234567890123456789012345678901234567890 && - test_must_fail git -C local fetch 2>err && - test_i18ngrep "fatal: remote error: upload-pack: not our ref" err -' - -test_expect_success 'server is initially ahead - ref in want' ' - git -C "$REPO" config uploadpack.allowRefInWant true && - rm -rf local && - cp -r "$LOCAL_PRISTINE" local && - inconsistency master 1234567890123456789012345678901234567890 && - git -C local fetch && - - git -C "$REPO" rev-parse --verify master >expected && - git -C local rev-parse --verify refs/remotes/origin/master >actual && - test_cmp expected actual -' - -test_expect_success 'server is initially behind - no ref in want' ' - git -C "$REPO" config uploadpack.allowRefInWant false && - rm -rf local && - cp -r "$LOCAL_PRISTINE" local && - inconsistency master "master^" && - git -C local fetch && - - git -C "$REPO" rev-parse --verify "master^" >expected && - git -C local rev-parse --verify refs/remotes/origin/master >actual && - test_cmp expected actual -' - -test_expect_success 'server is initially behind - ref in want' ' - git -C "$REPO" config uploadpack.allowRefInWant true && - rm -rf local && - cp -r "$LOCAL_PRISTINE" local && - inconsistency master "master^" && - git -C local fetch && - - git -C "$REPO" rev-parse --verify "master" >expected && - git -C local rev-parse --verify refs/remotes/origin/master >actual && - test_cmp expected actual -' - -test_expect_success 'server loses a ref - ref in want' ' - git -C "$REPO" config uploadpack.allowRefInWant true && - rm -rf local && - cp -r "$LOCAL_PRISTINE" local && - echo "s/master/raster/" >"$HTTPD_ROOT_PATH/one-time-sed" && - test_must_fail git -C local fetch 2>err && - - test_i18ngrep "fatal: remote error: unknown ref refs/heads/raster" err -' - REPO="$(pwd)/repo" LOCAL_PRISTINE="$(pwd)/local_pristine" @@ -372,4 +272,108 @@ test_expect_success 'fetching with wildcard that matches multiple refs' ' grep "want-ref refs/heads/o/bar" log ' +. "$TEST_DIRECTORY"/lib-httpd.sh +start_httpd + +REPO="$HTTPD_DOCUMENT_ROOT_PATH/repo" +LOCAL_PRISTINE="$(pwd)/local_pristine" + +test_expect_success 'setup repos for change-while-negotiating test' ' + ( + git init "$REPO" && + cd "$REPO" && + >.git/git-daemon-export-ok && + test_commit m1 && + git tag -d m1 && + + # Local repo with many commits (so that negotiation will take + # more than 1 request/response pair) + rm -rf "$LOCAL_PRISTINE" && + git clone "http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo" "$LOCAL_PRISTINE" && + cd "$LOCAL_PRISTINE" && + git checkout -b side && + test_commit_bulk --id=s 33 && + + # Add novel commits to upstream + git checkout master && + cd "$REPO" && + test_commit m2 && + test_commit m3 && + git tag -d m2 m3 + ) && + git -C "$LOCAL_PRISTINE" remote set-url origin "http://127.0.0.1:$LIB_HTTPD_PORT/one_time_sed/repo" && + git -C "$LOCAL_PRISTINE" config protocol.version 2 +' + +inconsistency () { + # Simulate that the server initially reports $2 as the ref + # corresponding to $1, and after that, $1 as the ref corresponding to + # $1. This corresponds to the real-life situation where the server's + # repository appears to change during negotiation, for example, when + # different servers in a load-balancing arrangement serve (stateless) + # RPCs during a single negotiation. + printf "s/%s/%s/" \ + $(git -C "$REPO" rev-parse $1 | tr -d "\n") \ + $(git -C "$REPO" rev-parse $2 | tr -d "\n") \ + >"$HTTPD_ROOT_PATH/one-time-sed" +} + +test_expect_success 'server is initially ahead - no ref in want' ' + git -C "$REPO" config uploadpack.allowRefInWant false && + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + inconsistency master 1234567890123456789012345678901234567890 && + test_must_fail git -C local fetch 2>err && + test_i18ngrep "fatal: remote error: upload-pack: not our ref" err +' + +test_expect_success 'server is initially ahead - ref in want' ' + git -C "$REPO" config uploadpack.allowRefInWant true && + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + inconsistency master 1234567890123456789012345678901234567890 && + git -C local fetch && + + git -C "$REPO" rev-parse --verify master >expected && + git -C local rev-parse --verify refs/remotes/origin/master >actual && + test_cmp expected actual +' + +test_expect_success 'server is initially behind - no ref in want' ' + git -C "$REPO" config uploadpack.allowRefInWant false && + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + inconsistency master "master^" && + git -C local fetch && + + git -C "$REPO" rev-parse --verify "master^" >expected && + git -C local rev-parse --verify refs/remotes/origin/master >actual && + test_cmp expected actual +' + +test_expect_success 'server is initially behind - ref in want' ' + git -C "$REPO" config uploadpack.allowRefInWant true && + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + inconsistency master "master^" && + git -C local fetch && + + git -C "$REPO" rev-parse --verify "master" >expected && + git -C local rev-parse --verify refs/remotes/origin/master >actual && + test_cmp expected actual +' + +test_expect_success 'server loses a ref - ref in want' ' + git -C "$REPO" config uploadpack.allowRefInWant true && + rm -rf local && + cp -r "$LOCAL_PRISTINE" local && + echo "s/master/raster/" >"$HTTPD_ROOT_PATH/one-time-sed" && + test_must_fail git -C local fetch 2>err && + + test_i18ngrep "fatal: remote error: unknown ref refs/heads/raster" err +' + +# DO NOT add non-httpd-specific tests here, because the last part of this +# test script is only executed when httpd is available and enabled. + test_done From decfe05bb668771fb25a23021b2b33bc7649cd32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= Date: Thu, 1 Aug 2019 17:53:09 +0200 Subject: [PATCH 3/3] t: warn against adding non-httpd-specific tests after sourcing 'lib-httpd' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have a couple of test scripts that are not completely httpd-specific, but do run a few httpd-specific tests at the end. These test scripts source 'lib-httpd.sh' somewhere mid-script, which then skips all the rest of the test script if the dependencies for running httpd tests are not fulfilled. As the previous two patches in this series show, already on two occasions non-httpd-specific tests were appended at the end of such test scripts, and, consequently, they were skipped as well when httpd tests couldn't be run. Add a comment at the end of these test scripts to warn against adding non-httpd-specific tests at the end, in the hope that they will help prevent similar issues in the future. Signed-off-by: SZEDER Gábor Signed-off-by: Junio C Hamano --- t/t0410-partial-clone.sh | 3 +++ t/t5500-fetch-pack.sh | 3 +++ t/t5537-fetch-shallow.sh | 3 +++ t/t5545-push-options.sh | 3 +++ t/t5601-clone.sh | 3 +++ t/t5616-partial-clone.sh | 3 +++ t/t5700-protocol-v1.sh | 3 +++ t/t5702-protocol-v2.sh | 3 +++ 8 files changed, 24 insertions(+) diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh index 5bd892f2f7..6415063980 100755 --- a/t/t0410-partial-clone.sh +++ b/t/t0410-partial-clone.sh @@ -518,4 +518,7 @@ test_expect_success 'fetching of missing objects from an HTTP server' ' git verify-pack --verbose "$IDX" | grep "$HASH" ' +# DO NOT add non-httpd-specific tests here, because the last part of this +# test script is only executed when httpd is available and enabled. + test_done diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 1c71c0ec77..8210f63d41 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -920,4 +920,7 @@ test_expect_success 'fetch with --filter=blob:limit=0 and HTTP' ' fetch_filter_blob_limit_zero "$HTTPD_DOCUMENT_ROOT_PATH/server" "$HTTPD_URL/smart/server" ' +# DO NOT add non-httpd-specific tests here, because the last part of this +# test script is only executed when httpd is available and enabled. + test_done diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index 66f0b64d39..97a67728ca 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -255,4 +255,7 @@ test_expect_success 'shallow fetches check connectivity before writing shallow f git -C client fsck ' +# DO NOT add non-httpd-specific tests here, because the last part of this +# test script is only executed when httpd is available and enabled. + test_done diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh index 6d1d59c9b1..04b34c4de1 100755 --- a/t/t5545-push-options.sh +++ b/t/t5545-push-options.sh @@ -278,4 +278,7 @@ test_expect_success 'push options keep quoted characters intact (http)' ' test_cmp expect "$HTTPD_DOCUMENT_ROOT_PATH"/upstream.git/hooks/pre-receive.push_options ' +# DO NOT add non-httpd-specific tests here, because the last part of this +# test script is only executed when httpd is available and enabled. + test_done diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 37d76808d4..4a3b901f06 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -739,4 +739,7 @@ test_expect_success 'partial clone using HTTP' ' partial_clone "$HTTPD_DOCUMENT_ROOT_PATH/server" "$HTTPD_URL/smart/server" ' +# DO NOT add non-httpd-specific tests here, because the last part of this +# test script is only executed when httpd is available and enabled. + test_done diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index b91ef548f8..565254558f 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -417,4 +417,7 @@ test_expect_success 'tolerate server sending REF_DELTA against missing promisor ! test -e "$HTTPD_ROOT_PATH/one-time-sed" ' +# DO NOT add non-httpd-specific tests here, because the last part of this +# test script is only executed when httpd is available and enabled. + test_done diff --git a/t/t5700-protocol-v1.sh b/t/t5700-protocol-v1.sh index 7c9511c593..2571eb90b7 100755 --- a/t/t5700-protocol-v1.sh +++ b/t/t5700-protocol-v1.sh @@ -292,4 +292,7 @@ test_expect_success 'push with http:// using protocol v1' ' grep "git< version 1" log ' +# DO NOT add non-httpd-specific tests here, because the last part of this +# test script is only executed when httpd is available and enabled. + test_done diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh index 011b81d4fc..fbddd0aea9 100755 --- a/t/t5702-protocol-v2.sh +++ b/t/t5702-protocol-v2.sh @@ -723,4 +723,7 @@ test_expect_success 'when server does not send "ready", expect FLUSH' ' test_i18ngrep "expected no other sections to be sent after no .ready." err ' +# DO NOT add non-httpd-specific tests here, because the last part of this +# test script is only executed when httpd is available and enabled. + test_done