{"thread":{"id":"53092","subject":"[PATCH 0/8] t: replace incorrect test_must_fail usage (part 3)","startedAt":"2020-03-25T05:55:15Z","lastAt":"2020-03-27T21:39:44Z","messageCount":30,"participants":["Denton Liu","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"393929","messageId":"cover.1585115341.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":null,"subject":"[PATCH 0/8] t: replace incorrect test_must_fail usage (part 3)","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-25T05:54:47Z","receivedAt":"2020-03-25T05:55:15Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The overall scope of these patches is to replace inappropriate uses of\ntest_must_fail. IOW, we should only allow test_must_fail to run on `git`\nand `test-tool`. Ultimately, we will conclude by making test_must_fail\nerror out on non-git commands. An advance view of the final series can\nbe found here[1].\n\nThis is the third part. It focuses on t5*.sh.\n\nThe first part can be found here[2]. The second part can be found here[3].\n\n[1]: (may be rebased at any time) https://github.com/Denton-L/git/tree/ready/cleanup-test-must-fail2\n[2]: https://lore.kernel.org/git/cover.1576583819.git.liu.denton@gmail.com/\n[3]: https://lore.kernel.org/git/cover.1577454401.git.liu.denton@gmail.com/\n\nDenton Liu (8):\n  t5512: don't use `test_must_fail test_cmp`\n  t5512: generate references with generate_references()\n  t5512: stop losing return codes of git commands\n  t5550: remove use of `test_might_fail grep`\n  t5607: reorder `nongit test_must_fail`\n  t5612: don't use `test_must_fail test_cmp`\n  t5612: stop losing return codes of git commands\n  t5801: teach compare_refs() to accept !\n\n t/t5512-ls-remote.sh       | 66 +++++++++++++++++---------------------\n t/t5550-http-fetch-dumb.sh |  2 +-\n t/t5607-clone-bundle.sh    |  2 +-\n t/t5612-clone-refspec.sh   | 26 +++++++--------\n t/t5801-remote-helpers.sh  | 10 ++++--\n 5 files changed, 52 insertions(+), 54 deletions(-)\n\n-- \n2.25.0.114.g5b0ca878e0\n\n"},{"id":"393930","messageId":"3d7dc8428d1992d3bc2802f5c828e8d77fe4de3b.1585115341.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585115341.git.liu.denton@gmail.com","subject":"[PATCH 1/8] t5512: don't use `test_must_fail test_cmp`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-25T05:54:48Z","receivedAt":"2020-03-25T05:55:16Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The test_must_fail function should only be used for git commands since\nwe should assume that external commands work sanely. Since test_cmp() just\nwraps an external command, replace `test_must_fail test_cmp` with\n`! test_cmp`.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5512-ls-remote.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 04b35402c7..08b98f12b8 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -92,7 +92,7 @@ test_expect_success 'use \"origin\" when no remote specified' '\n \n test_expect_success 'suppress \"From <url>\" with -q' '\n \tgit ls-remote -q 2>actual_err &&\n-\ttest_must_fail test_cmp exp_err actual_err\n+\t! test_cmp exp_err actual_err\n '\n \n test_expect_success 'use branch.<name>.remote if possible' '\n-- \n2.25.0.114.g5b0ca878e0\n\n"},{"id":"393931","messageId":"b3addeb98547783a31e2e2b876478a5dd96e04ec.1585115341.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585115341.git.liu.denton@gmail.com","subject":"[PATCH 3/8] t5512: stop losing return codes of git commands","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-25T05:54:50Z","receivedAt":"2020-03-25T05:55:18Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In a pipe, only the return code of the last command is used. Thus, all\nother commands will have their return codes masked. Rewrite pipes so\nthat there are no git commands upstream so that their failure is\nreported.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5512-ls-remote.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 62d02152c7..fea64eb5c2 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -21,11 +21,11 @@ test_expect_success setup '\n \tgit tag mark1.1 &&\n \tgit tag mark1.2 &&\n \tgit tag mark1.10 &&\n-\tgit show-ref --tags -d | sed -e \"s/ /\t/\" >expected.tag &&\n-\t(\n-\t\techo \"$(git rev-parse HEAD)\tHEAD\" &&\n-\t\tgit show-ref -d\t| sed -e \"s/ /\t/\"\n-\t) >expected.all &&\n+\tgit show-ref --tags -d >expected.tag.raw &&\n+\tsed -e \"s/ /\t/\" expected.tag.raw >expected.tag &&\n+\tgenerate_references HEAD >expected.all &&\n+\tgit show-ref -d\t>refs &&\n+\tsed -e \"s/ /\t/\" refs >>expected.all &&\n \n \tgit remote add self \"$(pwd)/.git\"\n '\n@@ -173,8 +173,8 @@ do\n \t\ttest_config $configsection.hiderefs refs/tags &&\n \t\tgit ls-remote . >actual &&\n \t\ttest_unconfig $configsection.hiderefs &&\n-\t\tgit ls-remote . |\n-\t\tsed -e \"/\trefs\\/tags\\//d\" >expect &&\n+\t\tgit ls-remote . >expect.raw &&\n+\t\tsed -e \"/\trefs\\/tags\\//d\" expect.raw >expect &&\n \t\ttest_cmp expect actual\n \t'\n \n-- \n2.25.0.114.g5b0ca878e0\n\n"},{"id":"393932","messageId":"674de50db28a50554d7af6e5c869c427d06f78aa.1585115341.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585115341.git.liu.denton@gmail.com","subject":"[PATCH 2/8] t5512: generate references with generate_references()","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-25T05:54:49Z","receivedAt":"2020-03-25T05:55:19Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The expected references are generated using a here-doc with some inline\nsubshells. If one of the `git rev-parse` invocations within the\nsubshells failed, its return code is swallowed and we won't know about\nit. Replace these here-docs with generate_references(), which actually\nreports when `git rev-parse` fails.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5512-ls-remote.sh | 50 +++++++++++++++++++-------------------------\n 1 file changed, 21 insertions(+), 29 deletions(-)\n\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 08b98f12b8..62d02152c7 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -4,6 +4,14 @@ test_description='git ls-remote'\n \n . ./test-lib.sh\n \n+generate_references () {\n+\tfor i\n+\tdo\n+\t\toid=$(git rev-parse \"$i\") || return 1\n+\t\tprintf '%s\\t%s\\n' \"$oid\" \"$i\"\n+\tdone\n+}\n+\n test_expect_success setup '\n \t>file &&\n \tgit add file &&\n@@ -43,34 +51,19 @@ test_expect_success 'ls-remote self' '\n '\n \n test_expect_success 'ls-remote --sort=\"version:refname\" --tags self' '\n-\tcat >expect <<-EOF &&\n-\t$(git rev-parse mark)\trefs/tags/mark\n-\t$(git rev-parse mark1.1)\trefs/tags/mark1.1\n-\t$(git rev-parse mark1.2)\trefs/tags/mark1.2\n-\t$(git rev-parse mark1.10)\trefs/tags/mark1.10\n-\tEOF\n+\tgenerate_references refs/tags/mark refs/tags/mark1.1 refs/tags/mark1.2 refs/tags/mark1.10 >expect &&\n \tgit ls-remote --sort=\"version:refname\" --tags self >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'ls-remote --sort=\"-version:refname\" --tags self' '\n-\tcat >expect <<-EOF &&\n-\t$(git rev-parse mark1.10)\trefs/tags/mark1.10\n-\t$(git rev-parse mark1.2)\trefs/tags/mark1.2\n-\t$(git rev-parse mark1.1)\trefs/tags/mark1.1\n-\t$(git rev-parse mark)\trefs/tags/mark\n-\tEOF\n+\tgenerate_references refs/tags/mark1.10 refs/tags/mark1.2 refs/tags/mark1.1 refs/tags/mark >expect &&\n \tgit ls-remote --sort=\"-version:refname\" --tags self >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'ls-remote --sort=\"-refname\" --tags self' '\n-\tcat >expect <<-EOF &&\n-\t$(git rev-parse mark1.2)\trefs/tags/mark1.2\n-\t$(git rev-parse mark1.10)\trefs/tags/mark1.10\n-\t$(git rev-parse mark1.1)\trefs/tags/mark1.1\n-\t$(git rev-parse mark)\trefs/tags/mark\n-\tEOF\n+\tgenerate_references refs/tags/mark1.2 refs/tags/mark1.10 refs/tags/mark1.1 refs/tags/mark >expect &&\n \tgit ls-remote --sort=\"-refname\" --tags self >actual &&\n \ttest_cmp expect actual\n '\n@@ -212,17 +205,16 @@ test_expect_success 'protocol v2 supports hiderefs' '\n \n test_expect_success 'ls-remote --symref' '\n \tgit fetch origin &&\n-\tcat >expect <<-EOF &&\n-\tref: refs/heads/master\tHEAD\n-\t$(git rev-parse HEAD)\tHEAD\n-\t$(git rev-parse refs/heads/master)\trefs/heads/master\n-\t$(git rev-parse HEAD)\trefs/remotes/origin/HEAD\n-\t$(git rev-parse refs/remotes/origin/master)\trefs/remotes/origin/master\n-\t$(git rev-parse refs/tags/mark)\trefs/tags/mark\n-\t$(git rev-parse refs/tags/mark1.1)\trefs/tags/mark1.1\n-\t$(git rev-parse refs/tags/mark1.10)\trefs/tags/mark1.10\n-\t$(git rev-parse refs/tags/mark1.2)\trefs/tags/mark1.2\n-\tEOF\n+\techo \"ref: refs/heads/master\tHEAD\" >expect &&\n+\tgenerate_references HEAD \\\n+\t\trefs/heads/master >>expect &&\n+\toid=$(git rev-parse HEAD) &&\n+\techo \"$oid\trefs/remotes/origin/HEAD\" >>expect &&\n+\tgenerate_references refs/remotes/origin/master \\\n+\t\trefs/tags/mark \\\n+\t\trefs/tags/mark1.1 \\\n+\t\trefs/tags/mark1.10 \\\n+\t\trefs/tags/mark1.2 >>expect &&\n \t# Protocol v2 supports sending symrefs for refs other than HEAD, so use\n \t# protocol v0 here.\n \tGIT_TEST_PROTOCOL_VERSION=0 git ls-remote --symref >actual &&\n-- \n2.25.0.114.g5b0ca878e0\n\n"},{"id":"393933","messageId":"4253f51feaf47029c1c3389e017d49f2148f03c6.1585115341.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585115341.git.liu.denton@gmail.com","subject":"[PATCH 5/8] t5607: reorder `nongit test_must_fail`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-25T05:54:52Z","receivedAt":"2020-03-25T05:55:23Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In the future, we plan on only allowing `test_must_fail` to work on a\nrestricted subset of commands, including `git`. Reorder the commands so\nthat `nongit` comes before `test_must_fail`. This way, `test_must_fail`\noperates on a git command.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5607-clone-bundle.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh\nindex 9108ff6fbd..6d5a977fcb 100755\n--- a/t/t5607-clone-bundle.sh\n+++ b/t/t5607-clone-bundle.sh\n@@ -16,7 +16,7 @@ test_expect_success 'setup' '\n \n test_expect_success '\"verify\" needs a worktree' '\n \tgit bundle create tip.bundle -1 master &&\n-\ttest_must_fail nongit git bundle verify ../tip.bundle 2>err &&\n+\tnongit test_must_fail git bundle verify ../tip.bundle 2>err &&\n \ttest_i18ngrep \"need a repository\" err\n '\n \n-- \n2.25.0.114.g5b0ca878e0\n\n"},{"id":"393934","messageId":"68c911e29b509d75e390aba573921d6ac385fcaf.1585115341.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585115341.git.liu.denton@gmail.com","subject":"[PATCH 4/8] t5550: remove use of `test_might_fail grep`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-25T05:54:51Z","receivedAt":"2020-03-25T05:55:23Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The test_must_fail() family of functions (including test_might_fail())\nshould only be used on git commands. Rewrite the use of\ntest_might_fail() with grep to remove this improper usage.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5550-http-fetch-dumb.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex b811d89cfd..a06294ad8f 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -248,7 +248,7 @@ test_expect_success 'fetch can handle previously-fetched .idx files' '\n '\n \n test_expect_success 'did not use upload-pack service' '\n-\ttest_might_fail grep '/git-upload-pack' <\"$HTTPD_ROOT_PATH\"/access.log >act &&\n+\t{ grep '/git-upload-pack' <\"$HTTPD_ROOT_PATH\"/access.log >act || :; } &&\n \t: >exp &&\n \ttest_cmp exp act\n '\n-- \n2.25.0.114.g5b0ca878e0\n\n"},{"id":"393935","messageId":"b51f97f6ae37e4b69b9651cbd60a480e5db3e72d.1585115341.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585115341.git.liu.denton@gmail.com","subject":"[PATCH 8/8] t5801: teach compare_refs() to accept !","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-25T05:54:55Z","receivedAt":"2020-03-25T05:55:24Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Before, testing if two refs weren't equal with compare_refs() was done\nwith `test_must_fail compare_refs`. This was wrong for two reasons.\nFirst, test_must_fail should only be used on git commands. Second,\nnegating the error code is a little heavy-handed since in the case where\none of the git invocations within compare_refs() fails, we will report\nsuccess, even though it failed at an unexpected point.\n\nTeach compare_refs() to accept `!` as the first argument which would\n_only_ negate the test_cmp()'s return code.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5801-remote-helpers.sh | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 121e5c6edb..0f04b6cddb 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -11,9 +11,15 @@ test_description='Test remote-helper import and export commands'\n PATH=\"$TEST_DIRECTORY/t5801:$PATH\"\n \n compare_refs() {\n+\tfail= &&\n+\tif test \"x$1\" = 'x!'\n+\tthen\n+\t\tfail='!' &&\n+\t\tshift\n+\tfi &&\n \tgit --git-dir=\"$1/.git\" rev-parse --verify $2 >expect &&\n \tgit --git-dir=\"$3/.git\" rev-parse --verify $4 >actual &&\n-\ttest_cmp expect actual\n+\teval $fail test_cmp expect actual\n }\n \n test_expect_success 'setup repository' '\n@@ -189,7 +195,7 @@ test_expect_success GPG 'push signed tag' '\n \tgit push origin signed-tag\n \t) &&\n \tcompare_refs local signed-tag^{} server signed-tag^{} &&\n-\ttest_must_fail compare_refs local signed-tag server signed-tag\n+\tcompare_refs ! local signed-tag server signed-tag\n '\n \n test_expect_success GPG 'push signed tag with signed-tags capability' '\n-- \n2.25.0.114.g5b0ca878e0\n\n"},{"id":"393936","messageId":"b490ccace0eb93b0f6d71f71e015afebcc9c9a51.1585115341.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585115341.git.liu.denton@gmail.com","subject":"[PATCH 7/8] t5612: stop losing return codes of git commands","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-25T05:54:54Z","receivedAt":"2020-03-25T05:55:26Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In a pipe, only the return code of the last command is used. Thus, all\nother commands will have their return codes masked. Rewrite pipes so\nthat there are no git commands upstream so that their failure is\nreported.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5612-clone-refspec.sh | 24 ++++++++++++------------\n 1 file changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh\nindex 28373e715a..e3b436d8ae 100755\n--- a/t/t5612-clone-refspec.sh\n+++ b/t/t5612-clone-refspec.sh\n@@ -71,9 +71,9 @@ test_expect_success 'by default all branches will be kept updated' '\n \t(\n \t\tcd dir_all &&\n \t\tgit fetch &&\n-\t\tgit for-each-ref refs/remotes/origin |\n+\t\tgit for-each-ref refs/remotes/origin >refs &&\n \t\tsed -e \"/HEAD$/d\" \\\n-\t\t    -e \"s|/remotes/origin/|/heads/|\" >../actual\n+\t\t    -e \"s|/remotes/origin/|/heads/|\" refs >../actual\n \t) &&\n \t# follow both master and side\n \tgit for-each-ref refs/heads >expect &&\n@@ -104,9 +104,9 @@ test_expect_success '--single-branch while HEAD pointing at master' '\n \t(\n \t\tcd dir_master &&\n \t\tgit fetch --force &&\n-\t\tgit for-each-ref refs/remotes/origin |\n+\t\tgit for-each-ref refs/remotes/origin >refs &&\n \t\tsed -e \"/HEAD$/d\" \\\n-\t\t    -e \"s|/remotes/origin/|/heads/|\" >../actual\n+\t\t    -e \"s|/remotes/origin/|/heads/|\" refs >../actual\n \t) &&\n \t# only follow master\n \tgit for-each-ref refs/heads/master >expect &&\n@@ -126,9 +126,9 @@ test_expect_success '--single-branch while HEAD pointing at master and --no-tags\n \t(\n \t\tcd dir_master_no_tags &&\n \t\tgit fetch &&\n-\t\tgit for-each-ref refs/remotes/origin |\n+\t\tgit for-each-ref refs/remotes/origin >refs &&\n \t\tsed -e \"/HEAD$/d\" \\\n-\t\t    -e \"s|/remotes/origin/|/heads/|\" >../actual\n+\t\t    -e \"s|/remotes/origin/|/heads/|\" refs >../actual\n \t) &&\n \t# only follow master\n \tgit for-each-ref refs/heads/master >expect &&\n@@ -156,9 +156,9 @@ test_expect_success '--single-branch while HEAD pointing at side' '\n \t(\n \t\tcd dir_side &&\n \t\tgit fetch &&\n-\t\tgit for-each-ref refs/remotes/origin |\n+\t\tgit for-each-ref refs/remotes/origin >refs &&\n \t\tsed -e \"/HEAD$/d\" \\\n-\t\t    -e \"s|/remotes/origin/|/heads/|\" >../actual\n+\t\t    -e \"s|/remotes/origin/|/heads/|\" refs >../actual\n \t) &&\n \t# only follow side\n \tgit for-each-ref refs/heads/side >expect &&\n@@ -169,9 +169,9 @@ test_expect_success '--single-branch with explicit --branch side' '\n \t(\n \t\tcd dir_side2 &&\n \t\tgit fetch &&\n-\t\tgit for-each-ref refs/remotes/origin |\n+\t\tgit for-each-ref refs/remotes/origin >refs &&\n \t\tsed -e \"/HEAD$/d\" \\\n-\t\t    -e \"s|/remotes/origin/|/heads/|\" >../actual\n+\t\t    -e \"s|/remotes/origin/|/heads/|\" refs >../actual\n \t) &&\n \t# only follow side\n \tgit for-each-ref refs/heads/side >expect &&\n@@ -223,9 +223,9 @@ test_expect_success '--single-branch with detached' '\n \t(\n \t\tcd dir_detached &&\n \t\tgit fetch &&\n-\t\tgit for-each-ref refs/remotes/origin |\n+\t\tgit for-each-ref refs/remotes/origin >refs &&\n \t\tsed -e \"/HEAD$/d\" \\\n-\t\t    -e \"s|/remotes/origin/|/heads/|\" >../actual\n+\t\t    -e \"s|/remotes/origin/|/heads/|\" refs >../actual\n \t) &&\n \t# nothing\n \ttest_must_be_empty actual\n-- \n2.25.0.114.g5b0ca878e0\n\n"},{"id":"393937","messageId":"2bac5f4a290620ef37e3dcf1e05773acd14cb725.1585115341.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585115341.git.liu.denton@gmail.com","subject":"[PATCH 6/8] t5612: don't use `test_must_fail test_cmp`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-25T05:54:53Z","receivedAt":"2020-03-25T05:55:26Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The test_must_fail function should only be used for git commands since\nwe should assume that external commands work sanely. Since test_cmp() just\nwraps an external command, replace `test_must_fail test_cmp` with\n`! test_cmp`.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5612-clone-refspec.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh\nindex e36ac01661..28373e715a 100755\n--- a/t/t5612-clone-refspec.sh\n+++ b/t/t5612-clone-refspec.sh\n@@ -87,7 +87,7 @@ test_expect_success 'by default no tags will be kept updated' '\n \t\tgit for-each-ref refs/tags >../actual\n \t) &&\n \tgit for-each-ref refs/tags >expect &&\n-\ttest_must_fail test_cmp expect actual &&\n+\t! test_cmp expect actual &&\n \ttest_line_count = 2 actual\n '\n \n-- \n2.25.0.114.g5b0ca878e0\n\n"},{"id":"393939","messageId":"CAPig+cQHZa4yELo3ncCVkbyrKWcPnZ4wjROMMbQGjm8dXK38GA@mail.gmail.com","threadId":"53092","inReplyTo":"674de50db28a50554d7af6e5c869c427d06f78aa.1585115341.git.liu.denton@gmail.com","subject":"Re: [PATCH 2/8] t5512: generate references with generate_references()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-03-25T06:08:31Z","receivedAt":"2020-03-25T06:08:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 25, 2020 at 1:55 AM Denton Liu <liu.denton@gmail.com> wrote:\n> t5512: generate references with generate_references()\n\nThis summary doesn't say anything useful. How about this instead?\n\n    t5512: stop losing git exit code in here-docs\n\n> The expected references are generated using a here-doc with some inline\n> subshells. If one of the `git rev-parse` invocations within the\n> subshells failed, its return code is swallowed and we won't know about\n\ns/failed/fails/\n\n> it. Replace these here-docs with generate_references(), which actually\n> reports when `git rev-parse` fails.\n\nA couple nits below...\n\n> Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> ---\n> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\n> @@ -4,6 +4,14 @@ test_description='git ls-remote'\n> +generate_references () {\n> +       for i\n> +       do\n> +               oid=$(git rev-parse \"$i\") || return 1\n> +               printf '%s\\t%s\\n' \"$oid\" \"$i\"\n\nI think the more usual way to say this in our test suite would be:\n\n    oid=$(git rev-parse \"$i\") &&\n    printf '%s\\t%s\\n' \"$oid\" \"$i\" || return 1\n\nwhich has the nice property that someone can come along and insert\nadditional code in the loop body before the final \"|| return 1\"\nwithout having to spend a lot of time trying to work out if the\n&&-chain is intact or broken.\n\n> +       done\n> +}\n> @@ -43,34 +51,19 @@ test_expect_success 'ls-remote self' '\n>  test_expect_success 'ls-remote --sort=\"version:refname\" --tags self' '\n> -       cat >expect <<-EOF &&\n> -       $(git rev-parse mark)   refs/tags/mark\n> -       $(git rev-parse mark1.1)        refs/tags/mark1.1\n> -       $(git rev-parse mark1.2)        refs/tags/mark1.2\n> -       $(git rev-parse mark1.10)       refs/tags/mark1.10\n> -       EOF\n> +       generate_references refs/tags/mark refs/tags/mark1.1 refs/tags/mark1.2 refs/tags/mark1.10 >expect &&\n\nThis gets awfully wide and loses some readability. Perhaps:\n\n    generate_references \\\n        refs/tags/mark \\\n        refs/tags/mark1.1 \\\n        refs/tags/mark1.2 \\\n        refs/tags/mark1.10 >expect &&\n\n>         git ls-remote --sort=\"version:refname\" --tags self >actual &&\n>         test_cmp expect actual\n>  '\n"},{"id":"393943","messageId":"xmqqv9mtx7wv.fsf@gitster.c.googlers.com","threadId":"53092","inReplyTo":"b51f97f6ae37e4b69b9651cbd60a480e5db3e72d.1585115341.git.liu.denton@gmail.com","subject":"Re: [PATCH 8/8] t5801: teach compare_refs() to accept !","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-25T06:31:12Z","receivedAt":"2020-03-25T06:31:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu <liu.denton@gmail.com> writes:\n\n>  compare_refs() {\n> +\tfail= &&\n> +\tif test \"x$1\" = 'x!'\n> +\tthen\n> +\t\tfail='!' &&\n> +\t\tshift\n> +\tfi &&\n>  \tgit --git-dir=\"$1/.git\" rev-parse --verify $2 >expect &&\n>  \tgit --git-dir=\"$3/.git\" rev-parse --verify $4 >actual &&\n> -\ttest_cmp expect actual\n> +\teval $fail test_cmp expect actual\n>  }\n>  \n>  test_expect_success 'setup repository' '\n> @@ -189,7 +195,7 @@ test_expect_success GPG 'push signed tag' '\n>  \tgit push origin signed-tag\n>  \t) &&\n>  \tcompare_refs local signed-tag^{} server signed-tag^{} &&\n> -\ttest_must_fail compare_refs local signed-tag server signed-tag\n> +\tcompare_refs ! local signed-tag server signed-tag\n\nWhile this is not wrong per-se, I do not know why we cannot just use\n\n\t! compare_refs local signed-tag server signed-tag\n\ni.e. \"we expect these two repositories have different tags\"?\n\nIt is totally plausible that I am missing something obvious, as it\nis getting late and I no longer am a night person.\n\nThanks.\n\n>  '\n>  \n>  test_expect_success GPG 'push signed tag with signed-tags capability' '\n"},{"id":"393944","messageId":"xmqqr1xhx7p6.fsf@gitster.c.googlers.com","threadId":"53092","inReplyTo":"68c911e29b509d75e390aba573921d6ac385fcaf.1585115341.git.liu.denton@gmail.com","subject":"Re: [PATCH 4/8] t5550: remove use of `test_might_fail grep`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-25T06:35:49Z","receivedAt":"2020-03-25T06:35:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu <liu.denton@gmail.com> writes:\n\n> The test_must_fail() family of functions (including test_might_fail())\n> should only be used on git commands. Rewrite the use of\n> test_might_fail() with grep to remove this improper usage.\n>\n> Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> ---\n>  t/t5550-http-fetch-dumb.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\n> index b811d89cfd..a06294ad8f 100755\n> --- a/t/t5550-http-fetch-dumb.sh\n> +++ b/t/t5550-http-fetch-dumb.sh\n> @@ -248,7 +248,7 @@ test_expect_success 'fetch can handle previously-fetched .idx files' '\n>  '\n>  \n>  test_expect_success 'did not use upload-pack service' '\n> -\ttest_might_fail grep '/git-upload-pack' <\"$HTTPD_ROOT_PATH\"/access.log >act &&\n> +\t{ grep '/git-upload-pack' <\"$HTTPD_ROOT_PATH\"/access.log >act || :; } &&\n\nWe can use\n\n\tsed -n -e \"/\\/git-upload-pack/p\" \"$HTTPD_ROOT_PATH/access.log\" >actual\n\ninstead, but \"grep for the pattern but don't worry if we found no\nmatch\" is also OK.\n\n>  \t: >exp &&\n>  \ttest_cmp exp act\n\nHaving said that, if the expectation is not to find any match,\nshouldn't the whole test be just\n\n\t! grep \"/git-upload-pack\" \"$HTTPD_ROOT_PATH/access.log\"\n\na single liner?  In any case, the use of sq in the original is broken.\n\n>  '\n"},{"id":"393945","messageId":"CAPig+cSJVdFEZ_V_FBtdAhR7KqocqXCgkHm1pFMDjBX_NPE+Jw@mail.gmail.com","threadId":"53092","inReplyTo":"xmqqv9mtx7wv.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 8/8] t5801: teach compare_refs() to accept !","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-03-25T06:36:41Z","receivedAt":"2020-03-25T06:36:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 25, 2020 at 2:31 AM Junio C Hamano <gitster@pobox.com> wrote:\n> Denton Liu <liu.denton@gmail.com> writes:\n> >  compare_refs() {\n> > +     fail= &&\n> > +     if test \"x$1\" = 'x!'\n> > +     then\n> > +             fail='!' &&\n> > +             shift\n> > +     fi &&\n> >       git --git-dir=\"$1/.git\" rev-parse --verify $2 >expect &&\n> >       git --git-dir=\"$3/.git\" rev-parse --verify $4 >actual &&\n> > -     test_cmp expect actual\n> > +     eval $fail test_cmp expect actual\n> >  }\n> > -     test_must_fail compare_refs local signed-tag server signed-tag\n> > +     compare_refs ! local signed-tag server signed-tag\n>\n> While this is not wrong per-se, I do not know why we cannot just use\n>\n>         ! compare_refs local signed-tag server signed-tag\n>\n> i.e. \"we expect these two repositories have different tags\"?\n\nAs mentioned in the commit message, if one of the git-rev-parse\ninvocations fails unexpectedly, then compare_refs() would return early\nwith a failure code, but the \"!\" would then (undesirably) turn that\nfailure into a success. We don't want to lose a failure code from\ngit-rev-parse, so the simple use of \"! compare_refs ...\" is avoided.\n"},{"id":"393946","messageId":"xmqqmu85x7fo.fsf@gitster.c.googlers.com","threadId":"53092","inReplyTo":"674de50db28a50554d7af6e5c869c427d06f78aa.1585115341.git.liu.denton@gmail.com","subject":"Re: [PATCH 2/8] t5512: generate references with generate_references()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-25T06:41:31Z","receivedAt":"2020-03-25T06:41:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu <liu.denton@gmail.com> writes:\n\n> +generate_references () {\n> +\tfor i\n\nIs it just me who thinks variables used in iteration should be\ncalled i, j, etc. only when they are integers?\n\n> +\tdo\n> +\t\toid=$(git rev-parse \"$i\") || return 1\n> +\t\tprintf '%s\\t%s\\n' \"$oid\" \"$i\"\n> +\tdone\n> +}\n> +\n>  test_expect_success setup '\n>  \t>file &&\n>  \tgit add file &&\n> @@ -43,34 +51,19 @@ test_expect_success 'ls-remote self' '\n>  '\n>  \n>  test_expect_success 'ls-remote --sort=\"version:refname\" --tags self' '\n> -\tcat >expect <<-EOF &&\n> -\t$(git rev-parse mark)\trefs/tags/mark\n> -\t$(git rev-parse mark1.1)\trefs/tags/mark1.1\n> -\t$(git rev-parse mark1.2)\trefs/tags/mark1.2\n> -\t$(git rev-parse mark1.10)\trefs/tags/mark1.10\n> -\tEOF\n> +\tgenerate_references refs/tags/mark refs/tags/mark1.1 refs/tags/mark1.2 refs/tags/mark1.10 >expect &&\n\nHmph, can we avoid overlong lines like this one?\n\n\tgenerate_references >expect <<-EOF\n\trefs/tags/mark\n\trefs/tags/mark1.1\n\trefs/tags/mark1.2\n\trefs/tags/mark1.10\n\tEOF\n\ni.e. teaching the helper function to read from its standard input\nstream, may make it more readable (i.e. it is more obvious what the\norder of expected output lines are, as you are listing them one by\none on a line of its own).\n\n"},{"id":"393947","messageId":"xmqqimitx7cq.fsf@gitster.c.googlers.com","threadId":"53092","inReplyTo":"cover.1585115341.git.liu.denton@gmail.com","subject":"Re: [PATCH 0/8] t: replace incorrect test_must_fail usage (part 3)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-25T06:43:17Z","receivedAt":"2020-03-25T06:43:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu <liu.denton@gmail.com> writes:\n\n> The overall scope of these patches is to replace inappropriate uses of\n> test_must_fail. IOW, we should only allow test_must_fail to run on `git`\n> and `test-tool`. Ultimately, we will conclude by making test_must_fail\n> error out on non-git commands. An advance view of the final series can\n> be found here[1].\n>\n> This is the third part. It focuses on t5*.sh.\n>\n> The first part can be found here[2]. The second part can be found here[3].\n>\n> [1]: (may be rebased at any time) https://github.com/Denton-L/git/tree/ready/cleanup-test-must-fail2\n> [2]: https://lore.kernel.org/git/cover.1576583819.git.liu.denton@gmail.com/\n> [3]: https://lore.kernel.org/git/cover.1577454401.git.liu.denton@gmail.com/\n\nI left comments on individual patches, but I think they were all\nminor.  Mostly the series looks good.\n\nThanks.\n\nThe dl/test-must-fail-fixes-* topics are always fun to read ;-)\n"},{"id":"394066","messageId":"cover.1585209554.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585115341.git.liu.denton@gmail.com","subject":"[PATCH v2 0/8] t: replace incorrect test_must_fail usage (part 3)","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-26T08:27:47Z","receivedAt":"2020-03-26T08:28:18Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The overall scope of these patches is to replace inappropriate uses of\ntest_must_fail. IOW, we should only allow test_must_fail to run on `git`\nand `test-tool`. Ultimately, we will conclude by making test_must_fail\nerror out on non-git commands. An advance view of the final series can\nbe found here[1].\n\nThis is the third part. It focuses on t5*.sh.\n\nThe first part can be found here[2]. The second part can be found here[3].\n\nChanges since v1:\n\n* Broke up long generate_references() lines for clarity in 2/8\n\n* For-loop &&-chain and renamed variable to \"ref\" in 2/8\n\n* Cleaned up commit message in 2/8\n\n* Rewrote 4/8 with a simpler test case\n\n[1]: (may be rebased at any time) https://github.com/Denton-L/git/tree/ready/cleanup-test-must-fail2\n[2]: https://lore.kernel.org/git/cover.1576583819.git.liu.denton@gmail.com/\n[3]: https://lore.kernel.org/git/cover.1577454401.git.liu.denton@gmail.com/\n\nDenton Liu (8):\n  t5512: don't use `test_must_fail test_cmp`\n  t5512: stop losing git exit code in here-docs\n  t5512: stop losing return codes of git commands\n  t5550: simplify no matching line check\n  t5607: reorder `nongit test_must_fail`\n  t5612: don't use `test_must_fail test_cmp`\n  t5612: stop losing return codes of git commands\n  t5801: teach compare_refs() to accept !\n\n t/t5512-ls-remote.sh       | 80 ++++++++++++++++++++------------------\n t/t5550-http-fetch-dumb.sh |  4 +-\n t/t5607-clone-bundle.sh    |  2 +-\n t/t5612-clone-refspec.sh   | 26 ++++++-------\n t/t5801-remote-helpers.sh  | 10 ++++-\n 5 files changed, 66 insertions(+), 56 deletions(-)\n\nRange-diff against v1:\n1:  3d7dc8428d = 1:  3d7dc8428d t5512: don't use `test_must_fail test_cmp`\n2:  674de50db2 ! 2:  97bc46e4a9 t5512: generate references with generate_references()\n    @@ Metadata\n     Author: Denton Liu <liu.denton@gmail.com>\n     \n      ## Commit message ##\n    -    t5512: generate references with generate_references()\n    +    t5512: stop losing git exit code in here-docs\n     \n         The expected references are generated using a here-doc with some inline\n    -    subshells. If one of the `git rev-parse` invocations within the\n    -    subshells failed, its return code is swallowed and we won't know about\n    -    it. Replace these here-docs with generate_references(), which actually\n    -    reports when `git rev-parse` fails.\n    +    command substitutions. If one of the `git rev-parse` invocations within\n    +    the command substitutions fails, its return code is swallowed and we\n    +    won't know about it. Replace these command substitutions with\n    +    generate_references(), which actually reports when `git rev-parse`\n    +    fails.\n     \n      ## t/t5512-ls-remote.sh ##\n     @@ t/t5512-ls-remote.sh: test_description='git ls-remote'\n    @@ t/t5512-ls-remote.sh: test_description='git ls-remote'\n      . ./test-lib.sh\n      \n     +generate_references () {\n    -+\tfor i\n    ++\tfor ref\n     +\tdo\n    -+\t\toid=$(git rev-parse \"$i\") || return 1\n    -+\t\tprintf '%s\\t%s\\n' \"$oid\" \"$i\"\n    ++\t\toid=$(git rev-parse \"$ref\") &&\n    ++\t\tprintf '%s\\t%s\\n' \"$oid\" \"$ref\" || return 1\n     +\tdone\n     +}\n     +\n    @@ t/t5512-ls-remote.sh: test_expect_success 'ls-remote self' '\n     -\t$(git rev-parse mark1.2)\trefs/tags/mark1.2\n     -\t$(git rev-parse mark1.10)\trefs/tags/mark1.10\n     -\tEOF\n    -+\tgenerate_references refs/tags/mark refs/tags/mark1.1 refs/tags/mark1.2 refs/tags/mark1.10 >expect &&\n    ++\tgenerate_references \\\n    ++\t\trefs/tags/mark \\\n    ++\t\trefs/tags/mark1.1 \\\n    ++\t\trefs/tags/mark1.2 \\\n    ++\t\trefs/tags/mark1.10 >expect &&\n      \tgit ls-remote --sort=\"version:refname\" --tags self >actual &&\n      \ttest_cmp expect actual\n      '\n    @@ t/t5512-ls-remote.sh: test_expect_success 'ls-remote self' '\n     -\t$(git rev-parse mark1.1)\trefs/tags/mark1.1\n     -\t$(git rev-parse mark)\trefs/tags/mark\n     -\tEOF\n    -+\tgenerate_references refs/tags/mark1.10 refs/tags/mark1.2 refs/tags/mark1.1 refs/tags/mark >expect &&\n    ++\tgenerate_references \\\n    ++\t\trefs/tags/mark1.10 \\\n    ++\t\trefs/tags/mark1.2 \\\n    ++\t\trefs/tags/mark1.1 \\\n    ++\t\trefs/tags/mark >expect &&\n      \tgit ls-remote --sort=\"-version:refname\" --tags self >actual &&\n      \ttest_cmp expect actual\n      '\n    @@ t/t5512-ls-remote.sh: test_expect_success 'ls-remote self' '\n     -\t$(git rev-parse mark1.1)\trefs/tags/mark1.1\n     -\t$(git rev-parse mark)\trefs/tags/mark\n     -\tEOF\n    -+\tgenerate_references refs/tags/mark1.2 refs/tags/mark1.10 refs/tags/mark1.1 refs/tags/mark >expect &&\n    ++\tgenerate_references \\\n    ++\t\trefs/tags/mark1.2 \\\n    ++\t\trefs/tags/mark1.10 \\\n    ++\t\trefs/tags/mark1.1 \\\n    ++\t\trefs/tags/mark >expect &&\n      \tgit ls-remote --sort=\"-refname\" --tags self >actual &&\n      \ttest_cmp expect actual\n      '\n    @@ t/t5512-ls-remote.sh: test_expect_success 'protocol v2 supports hiderefs' '\n     -\t$(git rev-parse refs/tags/mark1.2)\trefs/tags/mark1.2\n     -\tEOF\n     +\techo \"ref: refs/heads/master\tHEAD\" >expect &&\n    -+\tgenerate_references HEAD \\\n    ++\tgenerate_references \\\n    ++\t\tHEAD \\\n     +\t\trefs/heads/master >>expect &&\n     +\toid=$(git rev-parse HEAD) &&\n     +\techo \"$oid\trefs/remotes/origin/HEAD\" >>expect &&\n    -+\tgenerate_references refs/remotes/origin/master \\\n    ++\tgenerate_references \\\n    ++\trefs/remotes/origin/master \\\n     +\t\trefs/tags/mark \\\n     +\t\trefs/tags/mark1.1 \\\n     +\t\trefs/tags/mark1.10 \\\n3:  b3addeb985 = 3:  986ba1dd39 t5512: stop losing return codes of git commands\n4:  68c911e29b < -:  ---------- t5550: remove use of `test_might_fail grep`\n-:  ---------- > 4:  53e64e7077 t5550: simplify no matching line check\n5:  4253f51fea = 5:  4955b701e1 t5607: reorder `nongit test_must_fail`\n6:  2bac5f4a29 = 6:  a3d9d3673b t5612: don't use `test_must_fail test_cmp`\n7:  b490ccace0 = 7:  bc0e90d5ba t5612: stop losing return codes of git commands\n8:  b51f97f6ae = 8:  6a9d3cef2a t5801: teach compare_refs() to accept !\n-- \n2.26.0.159.g23e2136ad0\n\n"},{"id":"394067","messageId":"3d7dc8428d1992d3bc2802f5c828e8d77fe4de3b.1585209554.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585209554.git.liu.denton@gmail.com","subject":"[PATCH v2 1/8] t5512: don't use `test_must_fail test_cmp`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-26T08:27:48Z","receivedAt":"2020-03-26T08:28:18Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The test_must_fail function should only be used for git commands since\nwe should assume that external commands work sanely. Since test_cmp() just\nwraps an external command, replace `test_must_fail test_cmp` with\n`! test_cmp`.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5512-ls-remote.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 04b35402c7..08b98f12b8 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -92,7 +92,7 @@ test_expect_success 'use \"origin\" when no remote specified' '\n \n test_expect_success 'suppress \"From <url>\" with -q' '\n \tgit ls-remote -q 2>actual_err &&\n-\ttest_must_fail test_cmp exp_err actual_err\n+\t! test_cmp exp_err actual_err\n '\n \n test_expect_success 'use branch.<name>.remote if possible' '\n-- \n2.26.0.159.g23e2136ad0\n\n"},{"id":"394068","messageId":"97bc46e4a9105f13b5284f86907eb1459a9987b6.1585209554.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585209554.git.liu.denton@gmail.com","subject":"[PATCH v2 2/8] t5512: stop losing git exit code in here-docs","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-26T08:27:49Z","receivedAt":"2020-03-26T08:28:20Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The expected references are generated using a here-doc with some inline\ncommand substitutions. If one of the `git rev-parse` invocations within\nthe command substitutions fails, its return code is swallowed and we\nwon't know about it. Replace these command substitutions with\ngenerate_references(), which actually reports when `git rev-parse`\nfails.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5512-ls-remote.sh | 64 ++++++++++++++++++++++++--------------------\n 1 file changed, 35 insertions(+), 29 deletions(-)\n\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 08b98f12b8..dcb7349b0b 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -4,6 +4,14 @@ test_description='git ls-remote'\n \n . ./test-lib.sh\n \n+generate_references () {\n+\tfor ref\n+\tdo\n+\t\toid=$(git rev-parse \"$ref\") &&\n+\t\tprintf '%s\\t%s\\n' \"$oid\" \"$ref\" || return 1\n+\tdone\n+}\n+\n test_expect_success setup '\n \t>file &&\n \tgit add file &&\n@@ -43,34 +51,31 @@ test_expect_success 'ls-remote self' '\n '\n \n test_expect_success 'ls-remote --sort=\"version:refname\" --tags self' '\n-\tcat >expect <<-EOF &&\n-\t$(git rev-parse mark)\trefs/tags/mark\n-\t$(git rev-parse mark1.1)\trefs/tags/mark1.1\n-\t$(git rev-parse mark1.2)\trefs/tags/mark1.2\n-\t$(git rev-parse mark1.10)\trefs/tags/mark1.10\n-\tEOF\n+\tgenerate_references \\\n+\t\trefs/tags/mark \\\n+\t\trefs/tags/mark1.1 \\\n+\t\trefs/tags/mark1.2 \\\n+\t\trefs/tags/mark1.10 >expect &&\n \tgit ls-remote --sort=\"version:refname\" --tags self >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'ls-remote --sort=\"-version:refname\" --tags self' '\n-\tcat >expect <<-EOF &&\n-\t$(git rev-parse mark1.10)\trefs/tags/mark1.10\n-\t$(git rev-parse mark1.2)\trefs/tags/mark1.2\n-\t$(git rev-parse mark1.1)\trefs/tags/mark1.1\n-\t$(git rev-parse mark)\trefs/tags/mark\n-\tEOF\n+\tgenerate_references \\\n+\t\trefs/tags/mark1.10 \\\n+\t\trefs/tags/mark1.2 \\\n+\t\trefs/tags/mark1.1 \\\n+\t\trefs/tags/mark >expect &&\n \tgit ls-remote --sort=\"-version:refname\" --tags self >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'ls-remote --sort=\"-refname\" --tags self' '\n-\tcat >expect <<-EOF &&\n-\t$(git rev-parse mark1.2)\trefs/tags/mark1.2\n-\t$(git rev-parse mark1.10)\trefs/tags/mark1.10\n-\t$(git rev-parse mark1.1)\trefs/tags/mark1.1\n-\t$(git rev-parse mark)\trefs/tags/mark\n-\tEOF\n+\tgenerate_references \\\n+\t\trefs/tags/mark1.2 \\\n+\t\trefs/tags/mark1.10 \\\n+\t\trefs/tags/mark1.1 \\\n+\t\trefs/tags/mark >expect &&\n \tgit ls-remote --sort=\"-refname\" --tags self >actual &&\n \ttest_cmp expect actual\n '\n@@ -212,17 +217,18 @@ test_expect_success 'protocol v2 supports hiderefs' '\n \n test_expect_success 'ls-remote --symref' '\n \tgit fetch origin &&\n-\tcat >expect <<-EOF &&\n-\tref: refs/heads/master\tHEAD\n-\t$(git rev-parse HEAD)\tHEAD\n-\t$(git rev-parse refs/heads/master)\trefs/heads/master\n-\t$(git rev-parse HEAD)\trefs/remotes/origin/HEAD\n-\t$(git rev-parse refs/remotes/origin/master)\trefs/remotes/origin/master\n-\t$(git rev-parse refs/tags/mark)\trefs/tags/mark\n-\t$(git rev-parse refs/tags/mark1.1)\trefs/tags/mark1.1\n-\t$(git rev-parse refs/tags/mark1.10)\trefs/tags/mark1.10\n-\t$(git rev-parse refs/tags/mark1.2)\trefs/tags/mark1.2\n-\tEOF\n+\techo \"ref: refs/heads/master\tHEAD\" >expect &&\n+\tgenerate_references \\\n+\t\tHEAD \\\n+\t\trefs/heads/master >>expect &&\n+\toid=$(git rev-parse HEAD) &&\n+\techo \"$oid\trefs/remotes/origin/HEAD\" >>expect &&\n+\tgenerate_references \\\n+\trefs/remotes/origin/master \\\n+\t\trefs/tags/mark \\\n+\t\trefs/tags/mark1.1 \\\n+\t\trefs/tags/mark1.10 \\\n+\t\trefs/tags/mark1.2 >>expect &&\n \t# Protocol v2 supports sending symrefs for refs other than HEAD, so use\n \t# protocol v0 here.\n \tGIT_TEST_PROTOCOL_VERSION=0 git ls-remote --symref >actual &&\n-- \n2.26.0.159.g23e2136ad0\n\n"},{"id":"394069","messageId":"986ba1dd3921251a03c1c4cf92e7439e012b07ed.1585209554.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585209554.git.liu.denton@gmail.com","subject":"[PATCH v2 3/8] t5512: stop losing return codes of git commands","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-26T08:27:50Z","receivedAt":"2020-03-26T08:28:21Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In a pipe, only the return code of the last command is used. Thus, all\nother commands will have their return codes masked. Rewrite pipes so\nthat there are no git commands upstream so that their failure is\nreported.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5512-ls-remote.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex dcb7349b0b..8928d1f62d 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -21,11 +21,11 @@ test_expect_success setup '\n \tgit tag mark1.1 &&\n \tgit tag mark1.2 &&\n \tgit tag mark1.10 &&\n-\tgit show-ref --tags -d | sed -e \"s/ /\t/\" >expected.tag &&\n-\t(\n-\t\techo \"$(git rev-parse HEAD)\tHEAD\" &&\n-\t\tgit show-ref -d\t| sed -e \"s/ /\t/\"\n-\t) >expected.all &&\n+\tgit show-ref --tags -d >expected.tag.raw &&\n+\tsed -e \"s/ /\t/\" expected.tag.raw >expected.tag &&\n+\tgenerate_references HEAD >expected.all &&\n+\tgit show-ref -d\t>refs &&\n+\tsed -e \"s/ /\t/\" refs >>expected.all &&\n \n \tgit remote add self \"$(pwd)/.git\"\n '\n@@ -185,8 +185,8 @@ do\n \t\ttest_config $configsection.hiderefs refs/tags &&\n \t\tgit ls-remote . >actual &&\n \t\ttest_unconfig $configsection.hiderefs &&\n-\t\tgit ls-remote . |\n-\t\tsed -e \"/\trefs\\/tags\\//d\" >expect &&\n+\t\tgit ls-remote . >expect.raw &&\n+\t\tsed -e \"/\trefs\\/tags\\//d\" expect.raw >expect &&\n \t\ttest_cmp expect actual\n \t'\n \n-- \n2.26.0.159.g23e2136ad0\n\n"},{"id":"394070","messageId":"53e64e7077063a177cfafd97aa01340ee5b4cf1f.1585209554.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585209554.git.liu.denton@gmail.com","subject":"[PATCH v2 4/8] t5550: simplify no matching line check","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-26T08:27:51Z","receivedAt":"2020-03-26T08:28:33Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In the 'did not use upload-pack service' test, we have a complicated\nsong-and-dance to ensure that there are no \"/git-upload-pack\" lines in\n\"$HTTPD_ROOT_PATH/access.log\". Simplify this by just checking that grep\nreturns a non-zero exit code.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5550-http-fetch-dumb.sh | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex b811d89cfd..bcde886b87 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -248,9 +248,7 @@ test_expect_success 'fetch can handle previously-fetched .idx files' '\n '\n \n test_expect_success 'did not use upload-pack service' '\n-\ttest_might_fail grep '/git-upload-pack' <\"$HTTPD_ROOT_PATH\"/access.log >act &&\n-\t: >exp &&\n-\ttest_cmp exp act\n+\t! grep \"/git-upload-pack\" \"$HTTPD_ROOT_PATH/access.log\"\n '\n \n test_expect_success 'git client shows text/plain errors' '\n-- \n2.26.0.159.g23e2136ad0\n\n"},{"id":"394071","messageId":"4955b701e1f961e592e1c0c6567e28cbc12b3c14.1585209554.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585209554.git.liu.denton@gmail.com","subject":"[PATCH v2 5/8] t5607: reorder `nongit test_must_fail`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-26T08:27:52Z","receivedAt":"2020-03-26T08:28:34Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In the future, we plan on only allowing `test_must_fail` to work on a\nrestricted subset of commands, including `git`. Reorder the commands so\nthat `nongit` comes before `test_must_fail`. This way, `test_must_fail`\noperates on a git command.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5607-clone-bundle.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5607-clone-bundle.sh b/t/t5607-clone-bundle.sh\nindex 9108ff6fbd..6d5a977fcb 100755\n--- a/t/t5607-clone-bundle.sh\n+++ b/t/t5607-clone-bundle.sh\n@@ -16,7 +16,7 @@ test_expect_success 'setup' '\n \n test_expect_success '\"verify\" needs a worktree' '\n \tgit bundle create tip.bundle -1 master &&\n-\ttest_must_fail nongit git bundle verify ../tip.bundle 2>err &&\n+\tnongit test_must_fail git bundle verify ../tip.bundle 2>err &&\n \ttest_i18ngrep \"need a repository\" err\n '\n \n-- \n2.26.0.159.g23e2136ad0\n\n"},{"id":"394072","messageId":"a3d9d3673b24ab77e38e5c546f16fe9085d69496.1585209554.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585209554.git.liu.denton@gmail.com","subject":"[PATCH v2 6/8] t5612: don't use `test_must_fail test_cmp`","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-26T08:27:53Z","receivedAt":"2020-03-26T08:28:35Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"The test_must_fail function should only be used for git commands since\nwe should assume that external commands work sanely. Since test_cmp() just\nwraps an external command, replace `test_must_fail test_cmp` with\n`! test_cmp`.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5612-clone-refspec.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh\nindex e36ac01661..28373e715a 100755\n--- a/t/t5612-clone-refspec.sh\n+++ b/t/t5612-clone-refspec.sh\n@@ -87,7 +87,7 @@ test_expect_success 'by default no tags will be kept updated' '\n \t\tgit for-each-ref refs/tags >../actual\n \t) &&\n \tgit for-each-ref refs/tags >expect &&\n-\ttest_must_fail test_cmp expect actual &&\n+\t! test_cmp expect actual &&\n \ttest_line_count = 2 actual\n '\n \n-- \n2.26.0.159.g23e2136ad0\n\n"},{"id":"394073","messageId":"bc0e90d5baa73312bf77501c910e3457761b77f5.1585209554.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585209554.git.liu.denton@gmail.com","subject":"[PATCH v2 7/8] t5612: stop losing return codes of git commands","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-26T08:27:54Z","receivedAt":"2020-03-26T08:28:37Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"In a pipe, only the return code of the last command is used. Thus, all\nother commands will have their return codes masked. Rewrite pipes so\nthat there are no git commands upstream so that their failure is\nreported.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5612-clone-refspec.sh | 24 ++++++++++++------------\n 1 file changed, 12 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh\nindex 28373e715a..e3b436d8ae 100755\n--- a/t/t5612-clone-refspec.sh\n+++ b/t/t5612-clone-refspec.sh\n@@ -71,9 +71,9 @@ test_expect_success 'by default all branches will be kept updated' '\n \t(\n \t\tcd dir_all &&\n \t\tgit fetch &&\n-\t\tgit for-each-ref refs/remotes/origin |\n+\t\tgit for-each-ref refs/remotes/origin >refs &&\n \t\tsed -e \"/HEAD$/d\" \\\n-\t\t    -e \"s|/remotes/origin/|/heads/|\" >../actual\n+\t\t    -e \"s|/remotes/origin/|/heads/|\" refs >../actual\n \t) &&\n \t# follow both master and side\n \tgit for-each-ref refs/heads >expect &&\n@@ -104,9 +104,9 @@ test_expect_success '--single-branch while HEAD pointing at master' '\n \t(\n \t\tcd dir_master &&\n \t\tgit fetch --force &&\n-\t\tgit for-each-ref refs/remotes/origin |\n+\t\tgit for-each-ref refs/remotes/origin >refs &&\n \t\tsed -e \"/HEAD$/d\" \\\n-\t\t    -e \"s|/remotes/origin/|/heads/|\" >../actual\n+\t\t    -e \"s|/remotes/origin/|/heads/|\" refs >../actual\n \t) &&\n \t# only follow master\n \tgit for-each-ref refs/heads/master >expect &&\n@@ -126,9 +126,9 @@ test_expect_success '--single-branch while HEAD pointing at master and --no-tags\n \t(\n \t\tcd dir_master_no_tags &&\n \t\tgit fetch &&\n-\t\tgit for-each-ref refs/remotes/origin |\n+\t\tgit for-each-ref refs/remotes/origin >refs &&\n \t\tsed -e \"/HEAD$/d\" \\\n-\t\t    -e \"s|/remotes/origin/|/heads/|\" >../actual\n+\t\t    -e \"s|/remotes/origin/|/heads/|\" refs >../actual\n \t) &&\n \t# only follow master\n \tgit for-each-ref refs/heads/master >expect &&\n@@ -156,9 +156,9 @@ test_expect_success '--single-branch while HEAD pointing at side' '\n \t(\n \t\tcd dir_side &&\n \t\tgit fetch &&\n-\t\tgit for-each-ref refs/remotes/origin |\n+\t\tgit for-each-ref refs/remotes/origin >refs &&\n \t\tsed -e \"/HEAD$/d\" \\\n-\t\t    -e \"s|/remotes/origin/|/heads/|\" >../actual\n+\t\t    -e \"s|/remotes/origin/|/heads/|\" refs >../actual\n \t) &&\n \t# only follow side\n \tgit for-each-ref refs/heads/side >expect &&\n@@ -169,9 +169,9 @@ test_expect_success '--single-branch with explicit --branch side' '\n \t(\n \t\tcd dir_side2 &&\n \t\tgit fetch &&\n-\t\tgit for-each-ref refs/remotes/origin |\n+\t\tgit for-each-ref refs/remotes/origin >refs &&\n \t\tsed -e \"/HEAD$/d\" \\\n-\t\t    -e \"s|/remotes/origin/|/heads/|\" >../actual\n+\t\t    -e \"s|/remotes/origin/|/heads/|\" refs >../actual\n \t) &&\n \t# only follow side\n \tgit for-each-ref refs/heads/side >expect &&\n@@ -223,9 +223,9 @@ test_expect_success '--single-branch with detached' '\n \t(\n \t\tcd dir_detached &&\n \t\tgit fetch &&\n-\t\tgit for-each-ref refs/remotes/origin |\n+\t\tgit for-each-ref refs/remotes/origin >refs &&\n \t\tsed -e \"/HEAD$/d\" \\\n-\t\t    -e \"s|/remotes/origin/|/heads/|\" >../actual\n+\t\t    -e \"s|/remotes/origin/|/heads/|\" refs >../actual\n \t) &&\n \t# nothing\n \ttest_must_be_empty actual\n-- \n2.26.0.159.g23e2136ad0\n\n"},{"id":"394074","messageId":"6a9d3cef2a0daf1b058921bd0cd3397f7753e537.1585209554.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585209554.git.liu.denton@gmail.com","subject":"[PATCH v2 8/8] t5801: teach compare_refs() to accept !","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-26T08:27:55Z","receivedAt":"2020-03-26T08:28:38Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Before, testing if two refs weren't equal with compare_refs() was done\nwith `test_must_fail compare_refs`. This was wrong for two reasons.\nFirst, test_must_fail should only be used on git commands. Second,\nnegating the error code is a little heavy-handed since in the case where\none of the git invocations within compare_refs() fails, we will report\nsuccess, even though it failed at an unexpected point.\n\nTeach compare_refs() to accept `!` as the first argument which would\n_only_ negate the test_cmp()'s return code.\n\nSigned-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5801-remote-helpers.sh | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh\nindex 121e5c6edb..0f04b6cddb 100755\n--- a/t/t5801-remote-helpers.sh\n+++ b/t/t5801-remote-helpers.sh\n@@ -11,9 +11,15 @@ test_description='Test remote-helper import and export commands'\n PATH=\"$TEST_DIRECTORY/t5801:$PATH\"\n \n compare_refs() {\n+\tfail= &&\n+\tif test \"x$1\" = 'x!'\n+\tthen\n+\t\tfail='!' &&\n+\t\tshift\n+\tfi &&\n \tgit --git-dir=\"$1/.git\" rev-parse --verify $2 >expect &&\n \tgit --git-dir=\"$3/.git\" rev-parse --verify $4 >actual &&\n-\ttest_cmp expect actual\n+\teval $fail test_cmp expect actual\n }\n \n test_expect_success 'setup repository' '\n@@ -189,7 +195,7 @@ test_expect_success GPG 'push signed tag' '\n \tgit push origin signed-tag\n \t) &&\n \tcompare_refs local signed-tag^{} server signed-tag^{} &&\n-\ttest_must_fail compare_refs local signed-tag server signed-tag\n+\tcompare_refs ! local signed-tag server signed-tag\n '\n \n test_expect_success GPG 'push signed tag with signed-tags capability' '\n-- \n2.26.0.159.g23e2136ad0\n\n"},{"id":"394096","messageId":"CAPig+cTpV2n66nYJdQpG0tYCu5f=PB-a9Boa_NNfVaUOshDVug@mail.gmail.com","threadId":"53092","inReplyTo":"97bc46e4a9105f13b5284f86907eb1459a9987b6.1585209554.git.liu.denton@gmail.com","subject":"Re: [PATCH v2 2/8] t5512: stop losing git exit code in here-docs","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-03-26T15:24:06Z","receivedAt":"2020-03-26T15:24:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Mar 26, 2020 at 4:28 AM Denton Liu <liu.denton@gmail.com> wrote:\n> The expected references are generated using a here-doc with some inline\n> command substitutions. If one of the `git rev-parse` invocations within\n> the command substitutions fails, its return code is swallowed and we\n> won't know about it. Replace these command substitutions with\n> generate_references(), which actually reports when `git rev-parse`\n> fails.\n>\n> Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> ---\n> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\n> @@ -212,17 +217,18 @@ test_expect_success 'protocol v2 supports hiderefs' '\n>  test_expect_success 'ls-remote --symref' '\n> +       echo \"$oid      refs/remotes/origin/HEAD\" >>expect &&\n> +       generate_references \\\n> +       refs/remotes/origin/master \\\n> +               refs/tags/mark \\\n> +               refs/tags/mark1.1 \\\n> +               refs/tags/mark1.10 \\\n> +               refs/tags/mark1.2 >>expect &&\n\nBotched indentation of the \"refs/remotes/origin/master\" line.\n"},{"id":"394097","messageId":"CAPig+cREV63OmEkrqQ_ou=HDHnJZXLcd8j34hqBibiKe6uUnhQ@mail.gmail.com","threadId":"53092","inReplyTo":"53e64e7077063a177cfafd97aa01340ee5b4cf1f.1585209554.git.liu.denton@gmail.com","subject":"Re: [PATCH v2 4/8] t5550: simplify no matching line check","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-03-26T15:26:39Z","receivedAt":"2020-03-26T15:26:53Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Mar 26, 2020 at 4:28 AM Denton Liu <liu.denton@gmail.com> wrote:\n> In the 'did not use upload-pack service' test, we have a complicated\n> song-and-dance to ensure that there are no \"/git-upload-pack\" lines in\n> \"$HTTPD_ROOT_PATH/access.log\". Simplify this by just checking that grep\n> returns a non-zero exit code.\n>\n> Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> ---\n> diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\n> @@ -248,9 +248,7 @@ test_expect_success 'fetch can handle previously-fetched .idx files' '\n>  test_expect_success 'did not use upload-pack service' '\n> -       test_might_fail grep '/git-upload-pack' <\"$HTTPD_ROOT_PATH\"/access.log >act &&\n> -       : >exp &&\n> -       test_cmp exp act\n> +       ! grep \"/git-upload-pack\" \"$HTTPD_ROOT_PATH/access.log\"\n>  '\n\nIt would have been nice for the commit message to have mentioned that\nthe change is also eliminating an incorrect application of\ntest_might_fail(), but it's probably not worth a re-roll.\n"},{"id":"394132","messageId":"38924d9c4994b5bad93cbbd2659666f6b452a89f.1585269773.git.liu.denton@gmail.com","threadId":"53092","inReplyTo":"cover.1585209554.git.liu.denton@gmail.com","subject":"[PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2020-03-27T00:43:45Z","receivedAt":"2020-03-27T00:44:06Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Signed-off-by: Denton Liu <liu.denton@gmail.com>\n---\n t/t5512-ls-remote.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 8928d1f62d..e98c3a0174 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -224,7 +224,7 @@ test_expect_success 'ls-remote --symref' '\n \toid=$(git rev-parse HEAD) &&\n \techo \"$oid\trefs/remotes/origin/HEAD\" >>expect &&\n \tgenerate_references \\\n-\trefs/remotes/origin/master \\\n+\t\trefs/remotes/origin/master \\\n \t\trefs/tags/mark \\\n \t\trefs/tags/mark1.1 \\\n \t\trefs/tags/mark1.10 \\\n-- \n2.26.0.159.g23e2136ad0\n\n"},{"id":"394180","messageId":"xmqqimipherm.fsf@gitster.c.googlers.com","threadId":"53092","inReplyTo":"38924d9c4994b5bad93cbbd2659666f6b452a89f.1585269773.git.liu.denton@gmail.com","subject":"Re: [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-27T17:46:37Z","receivedAt":"2020-03-27T17:46:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Denton Liu <liu.denton@gmail.com> writes:\n\n> Subject: Re: [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs\n\nForgot to do the final \"rebase -i --autosquash\"?\n\n> Signed-off-by: Denton Liu <liu.denton@gmail.com>\n> ---\n>  t/t5512-ls-remote.sh | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\n> index 8928d1f62d..e98c3a0174 100755\n> --- a/t/t5512-ls-remote.sh\n> +++ b/t/t5512-ls-remote.sh\n> @@ -224,7 +224,7 @@ test_expect_success 'ls-remote --symref' '\n>  \toid=$(git rev-parse HEAD) &&\n>  \techo \"$oid\trefs/remotes/origin/HEAD\" >>expect &&\n>  \tgenerate_references \\\n> -\trefs/remotes/origin/master \\\n> +\t\trefs/remotes/origin/master \\\n>  \t\trefs/tags/mark \\\n>  \t\trefs/tags/mark1.1 \\\n>  \t\trefs/tags/mark1.10 \\\n"},{"id":"394194","messageId":"CAPig+cR0rNnVRF-Xei-n2HaxLR=9yF=7aya3mdApaqfEXmcPLg@mail.gmail.com","threadId":"53092","inReplyTo":"xmqqimipherm.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-03-27T20:29:20Z","receivedAt":"2020-03-27T20:29:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Mar 27, 2020 at 1:46 PM Junio C Hamano <gitster@pobox.com> wrote:\n> > Subject: Re: [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs\n>\n> Forgot to do the final \"rebase -i --autosquash\"?\n\nMy guess is that Denton sent this \"fixup!\" to have it queued atop [1]\nin response to the review comment[2] pointing out the bad indentation\n(rather than re-sending the entire series just for one minor change).\n\n[1]: https://lore.kernel.org/git/cover.1585209554.git.liu.denton@gmail.com/\n[2]: https://lore.kernel.org/git/CAPig+cTpV2n66nYJdQpG0tYCu5f=PB-a9Boa_NNfVaUOshDVug@mail.gmail.com/\n"},{"id":"394203","messageId":"xmqqwo75eaue.fsf@gitster.c.googlers.com","threadId":"53092","inReplyTo":"CAPig+cR0rNnVRF-Xei-n2HaxLR=9yF=7aya3mdApaqfEXmcPLg@mail.gmail.com","subject":"Re: [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-27T21:39:37Z","receivedAt":"2020-03-27T21:39:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Fri, Mar 27, 2020 at 1:46 PM Junio C Hamano <gitster@pobox.com> wrote:\n>> > Subject: Re: [PATCH v2 9/9] fixup! t5512: stop losing git exit code in here-docs\n>>\n>> Forgot to do the final \"rebase -i --autosquash\"?\n>\n> My guess is that Denton sent this \"fixup!\" to have it queued atop [1]\n> in response to the review comment[2] pointing out the bad indentation\n\nAh, [9/9] fooled me (and caused me not to see eight in [1/8]-[8/8]\nand I mistook they were [1/9]-[8/9].\n\nUsually people do the \"oops, here is a hotfix\" by marking such a\n\"beyond the original set\" patch as [9/8].\n\nIn any case, I think I've squashed it to the original.\nThanks.\n"}]}