{"thread":{"id":"55468","subject":"[RFC/PATCH 0/2] here-doc test bodies","startedAt":"2021-04-09T22:26:13Z","lastAt":"2021-04-10T08:31:47Z","messageCount":10,"participants":["Jeff King","Junio C Hamano","Derrick Stolee","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"421484","messageId":"YHDUg6ZR5vu93kGm@coredump.intra.peff.net","threadId":"55468","inReplyTo":null,"subject":"[RFC/PATCH 0/2] here-doc test bodies","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-09T22:26:11Z","receivedAt":"2021-04-09T22:26:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"I've been wanting to do this for years, but after getting bitten by a\nmisplaced quote the other day, I finally did. This series allows you to\ndo:\n\n  test_expect_success <<\\EOT\n          something 'with single quotes'\n  EOT\n\nThoughts?\n\nThe first patch is the implementation. The second one shows it off.\n\n  [1/2]: test-lib: allow test snippets as here-docs\n  [2/2]: t1404: convert to here-doc test bodies\n\n t/README                     |   8 +\n t/t1404-update-ref-errors.sh | 274 +++++++++++++++++------------------\n t/test-lib-functions.sh      |  30 +++-\n 3 files changed, 171 insertions(+), 141 deletions(-)\n\n-Peff\n"},{"id":"421485","messageId":"YHDVAxxKDzfTlq3h@coredump.intra.peff.net","threadId":"55468","inReplyTo":"YHDUg6ZR5vu93kGm@coredump.intra.peff.net","subject":"[PATCH 1/2] test-lib: allow test snippets as here-docs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-09T22:28:19Z","receivedAt":"2021-04-09T22:28:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Most test snippets are wrapped in single quotes, like:\n\n  test_expect_success 'some description' '\n          do_something\n  '\n\nThis sometimes makes the snippets awkward to write, because you can't\neasily use single quotes. We sometimes work around this with $SQ, or by\nloosening regexes to use \".\" instead of a literal quote, or by using\ndouble quotes when we'd prefer to use single-quotes (and just adding\nextra backslash-escapes to avoid interpolation).\n\nThis commit adds another option: feeding the snippet on the function's\nstdin. This doesn't conflict with anything the snippet would want to do,\nbecause we always redirect its stdin from /dev/null anyway (which we'll\ncontinue to do).\n\nA few notes on the implementation:\n\n  - it would be nice to push this down into test_run_, but we can't, as\n    test_expect_success and test_expect_failure want to see the actual\n    script content to report it for verbose-mode. A helper function\n    limits the amount of duplication in those callers here.\n\n  - The helper function is a little awkward to call, as you feed it the\n    name of the variable you want to set. The more natural thing in\n    shell would be command substitution like:\n\n      body=$(body_or_stdin \"$2\")\n\n    but that loses trailing whitespace. There are tricks around this,\n    like:\n\n      body=$(body_or_stdin \"$2\"; printf '.)\n      body=${body%.}\n\n    but we'd prefer to keep such tricks in the helper, not in each\n    caller.\n\n  - I implemented the helper using a sequence of \"read\" calls. Together\n    with \"-r\" and unsetting the IFS, this preserves incoming whitespace.\n    An alternative is to use \"cat\" (which then requires the gross \".\"\n    trick above). But this saves us a process, which is probably a good\n    thing. The \"read\" builtin does use more read() syscalls than\n    necessary (one per byte), but that is almost certainly a win over a\n    separate process.\n\n    Both are probably slower than passing a single-quoted string, but\n    the difference is lost in the noise for a script that I converted as\n    an experiment.\n\n  - I handle test_expect_success and test_expect_failure here. If we\n    like this style, we could easily extend it to other spots (e.g.,\n    lazy_prereq bodies) on top of this patch.\n\n  - even though we are using \"local\", we have to be careful about our\n    variable names. Within test_expect_success, any variable we declare\n    with local will be seen by the test snippets themselves (so it won't\n    persist between tests like normal variables would).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/README                |  8 ++++++++\n t/test-lib-functions.sh | 30 ++++++++++++++++++++++++++----\n 2 files changed, 34 insertions(+), 4 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex fd9375b146..a234c87792 100644\n--- a/t/README\n+++ b/t/README\n@@ -755,6 +755,14 @@ library for your script to use.\n \t    'git-write-tree should be able to write an empty tree.' \\\n \t    'tree=$(git-write-tree)'\n \n+   If <script> is `-` (a single dash), then the script to run is read\n+   from stdin. This lets you more easily use single quotes within the\n+   script by using a here-doc. For example:\n+\n+        test_expect_success 'output contains expected string' <<-\\EOT\n+\t        grep \"this string has 'quotes' in it\" output\n+\tEOT\n+\n    If you supply three parameters the first will be taken to be a\n    prerequisite; see the test_set_prereq and test_have_prereq\n    documentation below:\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 6348e8d733..3c8081b256 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -602,6 +602,24 @@ test_verify_prereq () {\n \tBUG \"'$test_prereq' does not look like a prereq\"\n }\n \n+# assign the variable named by \"$1\" with the contents of \"$2\";\n+# if \"$2\" is \"-\", then read stdin into \"$1\" instead\n+test_body_or_stdin () {\n+\tif test \"$2\" != \"-\"\n+\tthen\n+\t\teval \"$1=\\$2\"\n+\t\treturn\n+\tfi\n+\n+\t# start with a newline, to match hanging newline from open-quote style\n+\teval \"$1=\\$LF\"\n+\tlocal test_line\n+\twhile IFS= read -r test_line\n+\tdo\n+\t\teval \"$1=\\${$1}\\${test_line}\\${LF}\"\n+\tdone\n+}\n+\n test_expect_failure () {\n \ttest_start_\n \ttest \"$#\" = 3 && { test_prereq=$1; shift; } || test_prereq=\n@@ -611,8 +629,10 @@ test_expect_failure () {\n \texport test_prereq\n \tif ! test_skip \"$@\"\n \tthen\n-\t\tsay >&3 \"checking known breakage of $TEST_NUMBER.$test_count '$1': $2\"\n-\t\tif test_run_ \"$2\" expecting_failure\n+\t\tlocal test_body\n+\t\ttest_body_or_stdin test_body \"$2\"\n+\t\tsay >&3 \"checking known breakage of $TEST_NUMBER.$test_count '$1': $test_body\"\n+\t\tif test_run_ \"$test_body\" expecting_failure\n \t\tthen\n \t\t\ttest_known_broken_ok_ \"$1\"\n \t\telse\n@@ -631,8 +651,10 @@ test_expect_success () {\n \texport test_prereq\n \tif ! test_skip \"$@\"\n \tthen\n-\t\tsay >&3 \"expecting success of $TEST_NUMBER.$test_count '$1': $2\"\n-\t\tif test_run_ \"$2\"\n+\t\tlocal test_body\n+\t\ttest_body_or_stdin test_body \"$2\"\n+\t\tsay >&3 \"expecting success of $TEST_NUMBER.$test_count '$1': $test_body\"\n+\t\tif test_run_ \"$test_body\"\n \t\tthen\n \t\t\ttest_ok_ \"$1\"\n \t\telse\n-- \n2.31.1.629.gb61be4ec8d\n\n"},{"id":"421486","messageId":"YHDVF+oJ6kPje8Vh@coredump.intra.peff.net","threadId":"55468","inReplyTo":"YHDUg6ZR5vu93kGm@coredump.intra.peff.net","subject":"[PATCH 2/2] t1404: convert to here-doc test bodies","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-09T22:28:39Z","receivedAt":"2021-04-09T22:28:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The t1404 script checks a lot of output from Git which contains single\nquotes. Because the test snippets are themselves wrapped in the same\nsingle-quotes, we have to resort to using $SQ to match them. This is\nerror-prone and makes the tests harder to read.\n\nInstead, let's use the new here-doc feature added in the previous\ncommit, which lets us write anything in the test body we want (except\nthe here-doc end marker on a line by itself, of course).\n\nNote that we do use \"\\\" in our marker to avoid interpolation (which is\nthe whole point). But we don't use \"<<-\", as we want to preserve\nwhitespace in the snippet (and running with \"-v\" before and after shows\nthat we produce the exact same output, except with the ugly $SQ\nreferences fixed).\n\nI just converted every test here, even though only some of them use\n$SQ. But it would be equally correct to mix-and-match styles if we don't\nmind the inconsistency.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t1404-update-ref-errors.sh | 274 +++++++++++++++++------------------\n 1 file changed, 137 insertions(+), 137 deletions(-)\n\ndiff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh\nindex 8b51c4efc1..16ada22148 100755\n--- a/t/t1404-update-ref-errors.sh\n+++ b/t/t1404-update-ref-errors.sh\n@@ -91,7 +91,7 @@ df_test() {\n \t\tdelname=\"$delref\"\n \tfi &&\n \tcat >expected-err <<-EOF &&\n-\tfatal: cannot lock ref $SQ$addname$SQ: $SQ$delref$SQ exists; cannot create $SQ$addref$SQ\n+\tfatal: cannot lock ref '$addname': '$delref' exists; cannot create '$addref'\n \tEOF\n \t$pack &&\n \tif $add_del\n@@ -107,443 +107,443 @@ df_test() {\n \ttest_cmp expected-refs actual-refs\n }\n \n-test_expect_success 'setup' '\n+test_expect_success 'setup' - <<\\EOT\n \n \tgit commit --allow-empty -m Initial &&\n \tC=$(git rev-parse HEAD) &&\n \tgit commit --allow-empty -m Second &&\n \tD=$(git rev-parse HEAD) &&\n \tgit commit --allow-empty -m Third &&\n \tE=$(git rev-parse HEAD)\n-'\n+EOT\n \n-test_expect_success 'existing loose ref is a simple prefix of new' '\n+test_expect_success 'existing loose ref is a simple prefix of new' - <<\\EOT\n \n \tprefix=refs/1l &&\n \ttest_update_rejected \"a c e\" false \"b c/x d\" \\\n-\t\t\"$SQ$prefix/c$SQ exists; cannot create $SQ$prefix/c/x$SQ\"\n+\t\t\"'$prefix/c' exists; cannot create '$prefix/c/x'\"\n \n-'\n+EOT\n \n-test_expect_success 'existing packed ref is a simple prefix of new' '\n+test_expect_success 'existing packed ref is a simple prefix of new' - <<\\EOT\n \n \tprefix=refs/1p &&\n \ttest_update_rejected \"a c e\" true \"b c/x d\" \\\n-\t\t\"$SQ$prefix/c$SQ exists; cannot create $SQ$prefix/c/x$SQ\"\n+\t\t\"'$prefix/c' exists; cannot create '$prefix/c/x'\"\n \n-'\n+EOT\n \n-test_expect_success 'existing loose ref is a deeper prefix of new' '\n+test_expect_success 'existing loose ref is a deeper prefix of new' - <<\\EOT\n \n \tprefix=refs/2l &&\n \ttest_update_rejected \"a c e\" false \"b c/x/y d\" \\\n-\t\t\"$SQ$prefix/c$SQ exists; cannot create $SQ$prefix/c/x/y$SQ\"\n+\t\t\"'$prefix/c' exists; cannot create '$prefix/c/x/y'\"\n \n-'\n+EOT\n \n-test_expect_success 'existing packed ref is a deeper prefix of new' '\n+test_expect_success 'existing packed ref is a deeper prefix of new' - <<\\EOT\n \n \tprefix=refs/2p &&\n \ttest_update_rejected \"a c e\" true \"b c/x/y d\" \\\n-\t\t\"$SQ$prefix/c$SQ exists; cannot create $SQ$prefix/c/x/y$SQ\"\n+\t\t\"'$prefix/c' exists; cannot create '$prefix/c/x/y'\"\n \n-'\n+EOT\n \n-test_expect_success 'new ref is a simple prefix of existing loose' '\n+test_expect_success 'new ref is a simple prefix of existing loose' - <<\\EOT\n \n \tprefix=refs/3l &&\n \ttest_update_rejected \"a c/x e\" false \"b c d\" \\\n-\t\t\"$SQ$prefix/c/x$SQ exists; cannot create $SQ$prefix/c$SQ\"\n+\t\t\"'$prefix/c/x' exists; cannot create '$prefix/c'\"\n \n-'\n+EOT\n \n-test_expect_success 'new ref is a simple prefix of existing packed' '\n+test_expect_success 'new ref is a simple prefix of existing packed' - <<\\EOT\n \n \tprefix=refs/3p &&\n \ttest_update_rejected \"a c/x e\" true \"b c d\" \\\n-\t\t\"$SQ$prefix/c/x$SQ exists; cannot create $SQ$prefix/c$SQ\"\n+\t\t\"'$prefix/c/x' exists; cannot create '$prefix/c'\"\n \n-'\n+EOT\n \n-test_expect_success 'new ref is a deeper prefix of existing loose' '\n+test_expect_success 'new ref is a deeper prefix of existing loose' - <<\\EOT\n \n \tprefix=refs/4l &&\n \ttest_update_rejected \"a c/x/y e\" false \"b c d\" \\\n-\t\t\"$SQ$prefix/c/x/y$SQ exists; cannot create $SQ$prefix/c$SQ\"\n+\t\t\"'$prefix/c/x/y' exists; cannot create '$prefix/c'\"\n \n-'\n+EOT\n \n-test_expect_success 'new ref is a deeper prefix of existing packed' '\n+test_expect_success 'new ref is a deeper prefix of existing packed' - <<\\EOT\n \n \tprefix=refs/4p &&\n \ttest_update_rejected \"a c/x/y e\" true \"b c d\" \\\n-\t\t\"$SQ$prefix/c/x/y$SQ exists; cannot create $SQ$prefix/c$SQ\"\n+\t\t\"'$prefix/c/x/y' exists; cannot create '$prefix/c'\"\n \n-'\n+EOT\n \n-test_expect_success 'one new ref is a simple prefix of another' '\n+test_expect_success 'one new ref is a simple prefix of another' - <<\\EOT\n \n \tprefix=refs/5 &&\n \ttest_update_rejected \"a e\" false \"b c c/x d\" \\\n-\t\t\"cannot process $SQ$prefix/c$SQ and $SQ$prefix/c/x$SQ at the same time\"\n+\t\t\"cannot process '$prefix/c' and '$prefix/c/x' at the same time\"\n \n-'\n+EOT\n \n-test_expect_success 'empty directory should not fool rev-parse' '\n+test_expect_success 'empty directory should not fool rev-parse' - <<\\EOT\n \tprefix=refs/e-rev-parse &&\n \tgit update-ref $prefix/foo $C &&\n \tgit pack-refs --all &&\n \tmkdir -p .git/$prefix/foo/bar/baz &&\n \techo \"$C\" >expected &&\n \tgit rev-parse $prefix/foo >actual &&\n \ttest_cmp expected actual\n-'\n+EOT\n \n-test_expect_success 'empty directory should not fool for-each-ref' '\n+test_expect_success 'empty directory should not fool for-each-ref' - <<\\EOT\n \tprefix=refs/e-for-each-ref &&\n \tgit update-ref $prefix/foo $C &&\n \tgit for-each-ref $prefix >expected &&\n \tgit pack-refs --all &&\n \tmkdir -p .git/$prefix/foo/bar/baz &&\n \tgit for-each-ref $prefix >actual &&\n \ttest_cmp expected actual\n-'\n+EOT\n \n-test_expect_success 'empty directory should not fool create' '\n+test_expect_success 'empty directory should not fool create' - <<\\EOT\n \tprefix=refs/e-create &&\n \tmkdir -p .git/$prefix/foo/bar/baz &&\n \tprintf \"create %s $C\\n\" $prefix/foo |\n \tgit update-ref --stdin\n-'\n+EOT\n \n-test_expect_success 'empty directory should not fool verify' '\n+test_expect_success 'empty directory should not fool verify' - <<\\EOT\n \tprefix=refs/e-verify &&\n \tgit update-ref $prefix/foo $C &&\n \tgit pack-refs --all &&\n \tmkdir -p .git/$prefix/foo/bar/baz &&\n \tprintf \"verify %s $C\\n\" $prefix/foo |\n \tgit update-ref --stdin\n-'\n+EOT\n \n-test_expect_success 'empty directory should not fool 1-arg update' '\n+test_expect_success 'empty directory should not fool 1-arg update' - <<\\EOT\n \tprefix=refs/e-update-1 &&\n \tgit update-ref $prefix/foo $C &&\n \tgit pack-refs --all &&\n \tmkdir -p .git/$prefix/foo/bar/baz &&\n \tprintf \"update %s $D\\n\" $prefix/foo |\n \tgit update-ref --stdin\n-'\n+EOT\n \n-test_expect_success 'empty directory should not fool 2-arg update' '\n+test_expect_success 'empty directory should not fool 2-arg update' - <<\\EOT\n \tprefix=refs/e-update-2 &&\n \tgit update-ref $prefix/foo $C &&\n \tgit pack-refs --all &&\n \tmkdir -p .git/$prefix/foo/bar/baz &&\n \tprintf \"update %s $D $C\\n\" $prefix/foo |\n \tgit update-ref --stdin\n-'\n+EOT\n \n-test_expect_success 'empty directory should not fool 0-arg delete' '\n+test_expect_success 'empty directory should not fool 0-arg delete' - <<\\EOT\n \tprefix=refs/e-delete-0 &&\n \tgit update-ref $prefix/foo $C &&\n \tgit pack-refs --all &&\n \tmkdir -p .git/$prefix/foo/bar/baz &&\n \tprintf \"delete %s\\n\" $prefix/foo |\n \tgit update-ref --stdin\n-'\n+EOT\n \n-test_expect_success 'empty directory should not fool 1-arg delete' '\n+test_expect_success 'empty directory should not fool 1-arg delete' - <<\\EOT\n \tprefix=refs/e-delete-1 &&\n \tgit update-ref $prefix/foo $C &&\n \tgit pack-refs --all &&\n \tmkdir -p .git/$prefix/foo/bar/baz &&\n \tprintf \"delete %s $C\\n\" $prefix/foo |\n \tgit update-ref --stdin\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents add long + delete short' '\n+test_expect_success 'D/F conflict prevents add long + delete short' - <<\\EOT\n \tdf_test refs/df-al-ds --add-del foo/bar foo\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents add short + delete long' '\n+test_expect_success 'D/F conflict prevents add short + delete long' - <<\\EOT\n \tdf_test refs/df-as-dl --add-del foo foo/bar\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents delete long + add short' '\n+test_expect_success 'D/F conflict prevents delete long + add short' - <<\\EOT\n \tdf_test refs/df-dl-as --del-add foo/bar foo\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents delete short + add long' '\n+test_expect_success 'D/F conflict prevents delete short + add long' - <<\\EOT\n \tdf_test refs/df-ds-al --del-add foo foo/bar\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents add long + delete short packed' '\n+test_expect_success 'D/F conflict prevents add long + delete short packed' - <<\\EOT\n \tdf_test refs/df-al-dsp --pack --add-del foo/bar foo\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents add short + delete long packed' '\n+test_expect_success 'D/F conflict prevents add short + delete long packed' - <<\\EOT\n \tdf_test refs/df-as-dlp --pack --add-del foo foo/bar\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents delete long packed + add short' '\n+test_expect_success 'D/F conflict prevents delete long packed + add short' - <<\\EOT\n \tdf_test refs/df-dlp-as --pack --del-add foo/bar foo\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents delete short packed + add long' '\n+test_expect_success 'D/F conflict prevents delete short packed + add long' - <<\\EOT\n \tdf_test refs/df-dsp-al --pack --del-add foo foo/bar\n-'\n+EOT\n \n # Try some combinations involving symbolic refs...\n \n-test_expect_success 'D/F conflict prevents indirect add long + delete short' '\n+test_expect_success 'D/F conflict prevents indirect add long + delete short' - <<\\EOT\n \tdf_test refs/df-ial-ds --sym-add --add-del foo/bar foo\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents indirect add long + indirect delete short' '\n+test_expect_success 'D/F conflict prevents indirect add long + indirect delete short' - <<\\EOT\n \tdf_test refs/df-ial-ids --sym-add --sym-del --add-del foo/bar foo\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents indirect add short + indirect delete long' '\n+test_expect_success 'D/F conflict prevents indirect add short + indirect delete long' - <<\\EOT\n \tdf_test refs/df-ias-idl --sym-add --sym-del --add-del foo foo/bar\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents indirect delete long + indirect add short' '\n+test_expect_success 'D/F conflict prevents indirect delete long + indirect add short' - <<\\EOT\n \tdf_test refs/df-idl-ias --sym-add --sym-del --del-add foo/bar foo\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents indirect add long + delete short packed' '\n+test_expect_success 'D/F conflict prevents indirect add long + delete short packed' - <<\\EOT\n \tdf_test refs/df-ial-dsp --sym-add --pack --add-del foo/bar foo\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents indirect add long + indirect delete short packed' '\n+test_expect_success 'D/F conflict prevents indirect add long + indirect delete short packed' - <<\\EOT\n \tdf_test refs/df-ial-idsp --sym-add --sym-del --pack --add-del foo/bar foo\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents add long + indirect delete short packed' '\n+test_expect_success 'D/F conflict prevents add long + indirect delete short packed' - <<\\EOT\n \tdf_test refs/df-al-idsp --sym-del --pack --add-del foo/bar foo\n-'\n+EOT\n \n-test_expect_success 'D/F conflict prevents indirect delete long packed + indirect add short' '\n+test_expect_success 'D/F conflict prevents indirect delete long packed + indirect add short' - <<\\EOT\n \tdf_test refs/df-idlp-ias --sym-add --sym-del --pack --del-add foo/bar foo\n-'\n+EOT\n \n # Test various errors when reading the old values of references...\n \n-test_expect_success 'missing old value blocks update' '\n+test_expect_success 'missing old value blocks update' - <<\\EOT\n \tprefix=refs/missing-update &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ\n+\tfatal: cannot lock ref '$prefix/foo': unable to resolve reference '$prefix/foo'\n \tEOF\n \tprintf \"%s\\n\" \"update $prefix/foo $E $D\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'incorrect old value blocks update' '\n+test_expect_success 'incorrect old value blocks update' - <<\\EOT\n \tprefix=refs/incorrect-update &&\n \tgit update-ref $prefix/foo $C &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/foo$SQ: is at $C but expected $D\n+\tfatal: cannot lock ref '$prefix/foo': is at $C but expected $D\n \tEOF\n \tprintf \"%s\\n\" \"update $prefix/foo $E $D\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'existing old value blocks create' '\n+test_expect_success 'existing old value blocks create' - <<\\EOT\n \tprefix=refs/existing-create &&\n \tgit update-ref $prefix/foo $C &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/foo$SQ: reference already exists\n+\tfatal: cannot lock ref '$prefix/foo': reference already exists\n \tEOF\n \tprintf \"%s\\n\" \"create $prefix/foo $E\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'incorrect old value blocks delete' '\n+test_expect_success 'incorrect old value blocks delete' - <<\\EOT\n \tprefix=refs/incorrect-delete &&\n \tgit update-ref $prefix/foo $C &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/foo$SQ: is at $C but expected $D\n+\tfatal: cannot lock ref '$prefix/foo': is at $C but expected $D\n \tEOF\n \tprintf \"%s\\n\" \"delete $prefix/foo $D\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'missing old value blocks indirect update' '\n+test_expect_success 'missing old value blocks indirect update' - <<\\EOT\n \tprefix=refs/missing-indirect-update &&\n \tgit symbolic-ref $prefix/symref $prefix/foo &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ\n+\tfatal: cannot lock ref '$prefix/symref': unable to resolve reference '$prefix/foo'\n \tEOF\n \tprintf \"%s\\n\" \"update $prefix/symref $E $D\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'incorrect old value blocks indirect update' '\n+test_expect_success 'incorrect old value blocks indirect update' - <<\\EOT\n \tprefix=refs/incorrect-indirect-update &&\n \tgit symbolic-ref $prefix/symref $prefix/foo &&\n \tgit update-ref $prefix/foo $C &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/symref$SQ: is at $C but expected $D\n+\tfatal: cannot lock ref '$prefix/symref': is at $C but expected $D\n \tEOF\n \tprintf \"%s\\n\" \"update $prefix/symref $E $D\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'existing old value blocks indirect create' '\n+test_expect_success 'existing old value blocks indirect create' - <<\\EOT\n \tprefix=refs/existing-indirect-create &&\n \tgit symbolic-ref $prefix/symref $prefix/foo &&\n \tgit update-ref $prefix/foo $C &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/symref$SQ: reference already exists\n+\tfatal: cannot lock ref '$prefix/symref': reference already exists\n \tEOF\n \tprintf \"%s\\n\" \"create $prefix/symref $E\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'incorrect old value blocks indirect delete' '\n+test_expect_success 'incorrect old value blocks indirect delete' - <<\\EOT\n \tprefix=refs/incorrect-indirect-delete &&\n \tgit symbolic-ref $prefix/symref $prefix/foo &&\n \tgit update-ref $prefix/foo $C &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/symref$SQ: is at $C but expected $D\n+\tfatal: cannot lock ref '$prefix/symref': is at $C but expected $D\n \tEOF\n \tprintf \"%s\\n\" \"delete $prefix/symref $D\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'missing old value blocks indirect no-deref update' '\n+test_expect_success 'missing old value blocks indirect no-deref update' - <<\\EOT\n \tprefix=refs/missing-noderef-update &&\n \tgit symbolic-ref $prefix/symref $prefix/foo &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/symref$SQ: reference is missing but expected $D\n+\tfatal: cannot lock ref '$prefix/symref': reference is missing but expected $D\n \tEOF\n \tprintf \"%s\\n\" \"option no-deref\" \"update $prefix/symref $E $D\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'incorrect old value blocks indirect no-deref update' '\n+test_expect_success 'incorrect old value blocks indirect no-deref update' - <<\\EOT\n \tprefix=refs/incorrect-noderef-update &&\n \tgit symbolic-ref $prefix/symref $prefix/foo &&\n \tgit update-ref $prefix/foo $C &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/symref$SQ: is at $C but expected $D\n+\tfatal: cannot lock ref '$prefix/symref': is at $C but expected $D\n \tEOF\n \tprintf \"%s\\n\" \"option no-deref\" \"update $prefix/symref $E $D\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'existing old value blocks indirect no-deref create' '\n+test_expect_success 'existing old value blocks indirect no-deref create' - <<\\EOT\n \tprefix=refs/existing-noderef-create &&\n \tgit symbolic-ref $prefix/symref $prefix/foo &&\n \tgit update-ref $prefix/foo $C &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/symref$SQ: reference already exists\n+\tfatal: cannot lock ref '$prefix/symref': reference already exists\n \tEOF\n \tprintf \"%s\\n\" \"option no-deref\" \"create $prefix/symref $E\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'incorrect old value blocks indirect no-deref delete' '\n+test_expect_success 'incorrect old value blocks indirect no-deref delete' - <<\\EOT\n \tprefix=refs/incorrect-noderef-delete &&\n \tgit symbolic-ref $prefix/symref $prefix/foo &&\n \tgit update-ref $prefix/foo $C &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/symref$SQ: is at $C but expected $D\n+\tfatal: cannot lock ref '$prefix/symref': is at $C but expected $D\n \tEOF\n \tprintf \"%s\\n\" \"option no-deref\" \"delete $prefix/symref $D\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'non-empty directory blocks create' '\n+test_expect_success 'non-empty directory blocks create' - <<\\EOT\n \tprefix=refs/ne-create &&\n \tmkdir -p .git/$prefix/foo/bar &&\n \t: >.git/$prefix/foo/bar/baz.lock &&\n \ttest_when_finished \"rm -f .git/$prefix/foo/bar/baz.lock\" &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/foo$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ\n+\tfatal: cannot lock ref '$prefix/foo': there is a non-empty directory '.git/$prefix/foo' blocking reference '$prefix/foo'\n \tEOF\n \tprintf \"%s\\n\" \"update $prefix/foo $C\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ\n+\tfatal: cannot lock ref '$prefix/foo': unable to resolve reference '$prefix/foo'\n \tEOF\n \tprintf \"%s\\n\" \"update $prefix/foo $D $C\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'broken reference blocks create' '\n+test_expect_success 'broken reference blocks create' - <<\\EOT\n \tprefix=refs/broken-create &&\n \tmkdir -p .git/$prefix &&\n \techo \"gobbledigook\" >.git/$prefix/foo &&\n \ttest_when_finished \"rm -f .git/$prefix/foo\" &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken\n+\tfatal: cannot lock ref '$prefix/foo': unable to resolve reference '$prefix/foo': reference broken\n \tEOF\n \tprintf \"%s\\n\" \"update $prefix/foo $C\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/foo$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken\n+\tfatal: cannot lock ref '$prefix/foo': unable to resolve reference '$prefix/foo': reference broken\n \tEOF\n \tprintf \"%s\\n\" \"update $prefix/foo $D $C\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'non-empty directory blocks indirect create' '\n+test_expect_success 'non-empty directory blocks indirect create' - <<\\EOT\n \tprefix=refs/ne-indirect-create &&\n \tgit symbolic-ref $prefix/symref $prefix/foo &&\n \tmkdir -p .git/$prefix/foo/bar &&\n \t: >.git/$prefix/foo/bar/baz.lock &&\n \ttest_when_finished \"rm -f .git/$prefix/foo/bar/baz.lock\" &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/symref$SQ: there is a non-empty directory $SQ.git/$prefix/foo$SQ blocking reference $SQ$prefix/foo$SQ\n+\tfatal: cannot lock ref '$prefix/symref': there is a non-empty directory '.git/$prefix/foo' blocking reference '$prefix/foo'\n \tEOF\n \tprintf \"%s\\n\" \"update $prefix/symref $C\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ\n+\tfatal: cannot lock ref '$prefix/symref': unable to resolve reference '$prefix/foo'\n \tEOF\n \tprintf \"%s\\n\" \"update $prefix/symref $D $C\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'broken reference blocks indirect create' '\n+test_expect_success 'broken reference blocks indirect create' - <<\\EOT\n \tprefix=refs/broken-indirect-create &&\n \tgit symbolic-ref $prefix/symref $prefix/foo &&\n \techo \"gobbledigook\" >.git/$prefix/foo &&\n \ttest_when_finished \"rm -f .git/$prefix/foo\" &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken\n+\tfatal: cannot lock ref '$prefix/symref': unable to resolve reference '$prefix/foo': reference broken\n \tEOF\n \tprintf \"%s\\n\" \"update $prefix/symref $C\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err &&\n \tcat >expected <<-EOF &&\n-\tfatal: cannot lock ref $SQ$prefix/symref$SQ: unable to resolve reference $SQ$prefix/foo$SQ: reference broken\n+\tfatal: cannot lock ref '$prefix/symref': unable to resolve reference '$prefix/foo': reference broken\n \tEOF\n \tprintf \"%s\\n\" \"update $prefix/symref $D $C\" |\n \ttest_must_fail git update-ref --stdin 2>output.err &&\n \ttest_cmp expected output.err\n-'\n+EOT\n \n-test_expect_success 'no bogus intermediate values during delete' '\n+test_expect_success 'no bogus intermediate values during delete' - <<\\EOT\n \tprefix=refs/slow-transaction &&\n \t# Set up a reference with differing loose and packed versions:\n \tgit update-ref $prefix/foo $C &&\n@@ -598,9 +598,9 @@ test_expect_success 'no bogus intermediate values during delete' '\n \twait $pid2 &&\n \ttest_must_be_empty out &&\n \ttest_must_fail git rev-parse --verify --quiet $prefix/foo\n-'\n+EOT\n \n-test_expect_success 'delete fails cleanly if packed-refs file is locked' '\n+test_expect_success 'delete fails cleanly if packed-refs file is locked' - <<\\EOT\n \tprefix=refs/locked-packed-refs &&\n \t# Set up a reference with differing loose and packed versions:\n \tgit update-ref $prefix/foo $C &&\n@@ -612,11 +612,11 @@ test_expect_success 'delete fails cleanly if packed-refs file is locked' '\n \ttest_when_finished \"rm -f .git/packed-refs.lock\" &&\n \ttest_must_fail git update-ref -d $prefix/foo >out 2>err &&\n \tgit for-each-ref $prefix >actual &&\n-\ttest_i18ngrep \"Unable to create $SQ.*packed-refs.lock$SQ: \" err &&\n+\ttest_i18ngrep \"Unable to create '.*packed-refs.lock': \" err &&\n \ttest_cmp unchanged actual\n-'\n+EOT\n \n-test_expect_success 'delete fails cleanly if packed-refs.new write fails' '\n+test_expect_success 'delete fails cleanly if packed-refs.new write fails' - <<\\EOT\n \t# Setup and expectations are similar to the test above.\n \tprefix=refs/failed-packed-refs &&\n \tgit update-ref $prefix/foo $C &&\n@@ -630,6 +630,6 @@ test_expect_success 'delete fails cleanly if packed-refs.new write fails' '\n \ttest_must_fail git update-ref -d $prefix/foo &&\n \tgit for-each-ref $prefix >actual &&\n \ttest_cmp unchanged actual\n-'\n+EOT\n \n test_done\n-- \n2.31.1.629.gb61be4ec8d\n"},{"id":"421488","messageId":"YHDVhmveVlkHPBNb@coredump.intra.peff.net","threadId":"55468","inReplyTo":"YHDVAxxKDzfTlq3h@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] test-lib: allow test snippets as here-docs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-09T22:30:30Z","receivedAt":"2021-04-09T22:30:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 09, 2021 at 06:28:19PM -0400, Jeff King wrote:\n\n> +   If <script> is `-` (a single dash), then the script to run is read\n> +   from stdin. This lets you more easily use single quotes within the\n> +   script by using a here-doc. For example:\n> +\n> +        test_expect_success 'output contains expected string' <<-\\EOT\n> +\t        grep \"this string has 'quotes' in it\" output\n> +\tEOT\n\nWhoops, this should have an extra \"-\", of course (I got this wrong in\nthe cover letter, too). I.e.:\n\n  test_expect_success 'whatever' - <<\\EOT\n     ...\n  EOT\n\nIt would be nice to drop it, but then:\n\n  test_expect_success PREREQ 'whatever' <<\\EOT\n\nbecomes ambiguous (and I don't think there is a portable and reliable\nway to decide that our input is coming from a here-doc versus the\noriginal stdin).\n\n-Peff\n"},{"id":"421494","messageId":"xmqqtuofkqxu.fsf@gitster.g","threadId":"55468","inReplyTo":"YHDVAxxKDzfTlq3h@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] test-lib: allow test snippets as here-docs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-09T22:56:29Z","receivedAt":"2021-04-09T22:56:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> +   If <script> is `-` (a single dash), then the script to run is read\n> +   from stdin. This lets you more easily use single quotes within the\n> +   script by using a here-doc. For example:\n> +\n> +        test_expect_success 'output contains expected string' <<-\\EOT\n\nMissing '-'?\n\n> +\t        grep \"this string has 'quotes' in it\" output\n> +\tEOT\n> +\n> ...\n> +\t# start with a newline, to match hanging newline from open-quote style\n> +\teval \"$1=\\$LF\"\n> +\tlocal test_line\n> +\twhile IFS= read -r test_line\n> +\tdo\n> +\t\teval \"$1=\\${$1}\\${test_line}\\${LF}\"\n> +\tdone\n\nI wonder if we can do this without relying on \"read -r\" (which I\ndistrust, perhaps out of superstition)?  Perhaps by slurping the\nwhole thing with \"$(cat)\"?\n\nThanks.\n"},{"id":"421499","messageId":"xmqqh7kfklcp.fsf@gitster.g","threadId":"55468","inReplyTo":"xmqqtuofkqxu.fsf@gitster.g","subject":"Re: [PATCH 1/2] test-lib: allow test snippets as here-docs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-04-10T00:57:10Z","receivedAt":"2021-04-10T00:57:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> +\t# start with a newline, to match hanging newline from open-quote style\n>> +\teval \"$1=\\$LF\"\n>> +\tlocal test_line\n>> +\twhile IFS= read -r test_line\n>> +\tdo\n>> +\t\teval \"$1=\\${$1}\\${test_line}\\${LF}\"\n>> +\tdone\n>\n> I wonder if we can do this without relying on \"read -r\" (which I\n> distrust, perhaps out of superstition)?  Perhaps by slurping the\n> whole thing with \"$(cat)\"?\n\nMeaning, something along this line...\n\n----- >8 --------- >8 --------- >8 --------- >8 ----\n#!/bin/sh\nLF='\n'\nttt () {\n\teval \"$1\"='$LF$(cat)'\n}\nttt script <<\\EOT\n\ta\n\tb\nEOT\necho \"<<<$script>>>\"\n----- 8< --------- 8< --------- 8< --------- 8< ----\n"},{"id":"421501","messageId":"42b187cb-6417-0353-cf63-c6379be53208@gmail.com","threadId":"55468","inReplyTo":"YHDUg6ZR5vu93kGm@coredump.intra.peff.net","subject":"Re: [RFC/PATCH 0/2] here-doc test bodies","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-04-10T01:03:24Z","receivedAt":"2021-04-10T01:03:30Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/9/2021 6:26 PM, Jeff King wrote:\n> I've been wanting to do this for years, but after getting bitten by a\n> misplaced quote the other day, I finally did. This series allows you to\n> do:\n> \n>   test_expect_success <<\\EOT\n>           something 'with single quotes'\n>   EOT\n> \n> Thoughts?\n\nOdd. I _just_ hit this for the first time today. I didn't know\nabout the $SQ trick, so I just modified my 'sed' call to drop\nthe single quotes from the string I was trying to match [1].\n\n[1] https://lore.kernel.org/git/36aa6837-722c-9ef0-84cc-77e982db9f6e@gmail.com/\n\nI think this is a good pattern for this kind of thing. I\nimagine that if the test itself needed heredocs, then they\nwould be interpreted correctly when evaluating stdin (as\nlong as a different identifier is used).\n\nI also like the \"EOT\" pattern as an example.\n\nThanks,\n-Stolee\n"},{"id":"421506","messageId":"YHD+3mKoQmzIagjF@coredump.intra.peff.net","threadId":"55468","inReplyTo":"xmqqh7kfklcp.fsf@gitster.g","subject":"Re: [PATCH 1/2] test-lib: allow test snippets as here-docs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-10T01:26:54Z","receivedAt":"2021-04-10T01:26:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 09, 2021 at 05:57:10PM -0700, Junio C Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> >> +\t# start with a newline, to match hanging newline from open-quote style\n> >> +\teval \"$1=\\$LF\"\n> >> +\tlocal test_line\n> >> +\twhile IFS= read -r test_line\n> >> +\tdo\n> >> +\t\teval \"$1=\\${$1}\\${test_line}\\${LF}\"\n> >> +\tdone\n> >\n> > I wonder if we can do this without relying on \"read -r\" (which I\n> > distrust, perhaps out of superstition)?  Perhaps by slurping the\n> > whole thing with \"$(cat)\"?\n> \n> Meaning, something along this line...\n> \n> ----- >8 --------- >8 --------- >8 --------- >8 ----\n> #!/bin/sh\n> LF='\n> '\n> ttt () {\n> \teval \"$1\"='$LF$(cat)'\n> }\n> ttt script <<\\EOT\n> \ta\n> \tb\n> EOT\n> echo \"<<<$script>>>\"\n> ----- 8< --------- 8< --------- 8< --------- 8< ----\n\nI wrote it using cat initially. If you want to preserve trailing\nwhitespace, you have to do something like:\n\n  val=$(printf '\\n'\n        cat\n\tprintf '.')\n  val=${val%.}\n\nI was worried about adding the overhead of the extra subshell and\nprocess for the command substitution, but perhaps that is overblown.\nTBH, worrying about whitespace may be overblown, too. Some test snippets\nhave extra blank lines at the end, but possibly we should just not care.\n\nI imagine \"read -r\" does not work reliably for binary junk, but keep in\nmind that we are talking about text shell script snippets (that are\nalready in shell strings anyway).\n\n-Peff\n"},{"id":"421507","messageId":"YHEAn7u4SXlmZsfZ@coredump.intra.peff.net","threadId":"55468","inReplyTo":"42b187cb-6417-0353-cf63-c6379be53208@gmail.com","subject":"Re: [RFC/PATCH 0/2] here-doc test bodies","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-04-10T01:34:23Z","receivedAt":"2021-04-10T01:34:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 09, 2021 at 09:03:24PM -0400, Derrick Stolee wrote:\n\n> On 4/9/2021 6:26 PM, Jeff King wrote:\n> > I've been wanting to do this for years, but after getting bitten by a\n> > misplaced quote the other day, I finally did. This series allows you to\n> > do:\n> > \n> >   test_expect_success <<\\EOT\n> >           something 'with single quotes'\n> >   EOT\n> > \n> > Thoughts?\n> \n> Odd. I _just_ hit this for the first time today. I didn't know\n> about the $SQ trick, so I just modified my 'sed' call to drop\n> the single quotes from the string I was trying to match [1].\n> \n> [1] https://lore.kernel.org/git/36aa6837-722c-9ef0-84cc-77e982db9f6e@gmail.com/\n\nYep, that case would definitely benefit. We have a variety of little\nworkarounds now, but it would be nice to not even need them.\n\n> I think this is a good pattern for this kind of thing. I\n> imagine that if the test itself needed heredocs, then they\n> would be interpreted correctly when evaluating stdin (as\n> long as a different identifier is used).\n\nYeah, some of the tests I converted in t1404 have their own here-docs.\nIt works for the same reason that the single-quoted snippets work: the\nouter doc slurps the whole thing to the \"EOT\" marker. So we do not even\nconsider the inner here-docs until we are eval-ing the snippet.\n\n> I also like the \"EOT\" pattern as an example.\n\nI think it would make sense to have a convention here for readability.\nAlso, I added this to the sharness vim highlighter from [1]:\n\ndiff --git a/sharness.vim b/sharness.vim\nindex 6ffc64f..7dbcd18 100644\n--- a/sharness.vim\n+++ b/sharness.vim\n@@ -21,6 +21,9 @@ syn region shsTestBody contained transparent excludenl matchgroup=shQuote start=\n syn region shsTestBody contained oneline transparent excludenl keepend matchgroup=shQuote start=+ '+hs=s+1 end=+'$+ contains=@shSubShList\n syn region shsTestBody contained oneline transparent excludenl keepend matchgroup=shQuote start=+ \"+hs=s+1 end=+\"$+ contains=@shSubShList\n \n+\" here-doc body\n+syn region shsTestBody contained transparent excludenl matchgroup=shQuote start=+ <<\\\\EOT+ end=+EOT$+ contains=@shSubShList\n+\n syn match shsPrereq contained \"\\<[A-Z_,]\\+\\>\" nextgroup=shsTestTitle\n syn match shsPrereqLazy contained \"\\<[A-Z_,]\\+\\>\" nextgroup=shsTestBody\n \n\nwhich makes the innards look nice. :) I think it could be written to\nallow any marker, but there would probably need to be some magic with\nmatching the opening and closing markers.\n\n-Peff\n\n[1] https://lore.kernel.org/git/CAMP44s1D-Zp3KDS+Hi74a=Lkc7Nc_0qiEzQEF0Pmj=bD8i+=JQ@mail.gmail.com/\n"},{"id":"421522","messageId":"87a6q6h78y.fsf@evledraar.gmail.com","threadId":"55468","inReplyTo":"YHDVAxxKDzfTlq3h@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] test-lib: allow test snippets as here-docs","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-04-10T08:30:05Z","receivedAt":"2021-04-10T08:31:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Apr 10 2021, Jeff King wrote:\n\n> Most test snippets are wrapped in single quotes, like:\n>\n>   test_expect_success 'some description' '\n>           do_something\n>   '\n>\n> This sometimes makes the snippets awkward to write, because you can't\n> easily use single quotes. We sometimes work around this with $SQ, or by\n> loosening regexes to use \".\" instead of a literal quote, or by using\n> double quotes when we'd prefer to use single-quotes (and just adding\n> extra backslash-escapes to avoid interpolation).\n>\n> This commit adds another option: feeding the snippet on the function's\n> stdin. This doesn't conflict with anything the snippet would want to do,\n> because we always redirect its stdin from /dev/null anyway (which we'll\n> continue to do).\n\nI like this, and not having to write $SQ, '\"'\"' etc.\n\n> A few notes on the implementation:\n>\n>   - it would be nice to push this down into test_run_, but we can't, as\n>     test_expect_success and test_expect_failure want to see the actual\n>     script content to report it for verbose-mode. A helper function\n>     limits the amount of duplication in those callers here.\n\nI've got an unsubmitted series (a bigger part of the -V rewrite) which\nconflicted with this one, because I'd moved that message into those\nhelper functions.\n\nBut in that case I end up having to have this in\ntest_expect_{success,failure} anyway, because the change I had was to\nmove it into test_{ok,failure}_, i.e. to color the emitted body under\nverbose differently depending on test ok/failure (which means deferring\nthe \"this is our test body\" until after the run).\n\nIt got slightly awkward because before I could pass \"$@\" to those (they\npass \"$1\" now), but with your change there's a \"-\" left on the argument\nlist, so we need to pass \"$1\" and \"$test_body\".\n\nAnyway, it's no problem, just musings on having re-arranged this code\nyou're pointing out needs/could be re-arranged.\n\nMaybe it would be easier to pass test_run arguments saying whether we\nexpect failure or not, and then move the whole if/else after it into its\nown body. It already takes the \"expecting_failure\" parameter, so this on\ntop of master:\n\n\tdiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n\tindex 6348e8d733..9e20bd607d 100644\n\t--- a/t/test-lib-functions.sh\n\t+++ b/t/test-lib-functions.sh\n\t@@ -611,8 +611,7 @@ test_expect_failure () {\n\t \texport test_prereq\n\t \tif ! test_skip \"$@\"\n\t \tthen\n\t-\t\tsay >&3 \"checking known breakage of $TEST_NUMBER.$test_count '$1': $2\"\n\t-\t\tif test_run_ \"$2\" expecting_failure\n\t+\t\tif test_run_ \"$1\" \"$2\" expecting_failure\n\t \t\tthen\n\t \t\t\ttest_known_broken_ok_ \"$1\"\n\t \t\telse\n\t@@ -631,8 +630,7 @@ test_expect_success () {\n\t \texport test_prereq\n\t \tif ! test_skip \"$@\"\n\t \tthen\n\t-\t\tsay >&3 \"expecting success of $TEST_NUMBER.$test_count '$1': $2\"\n\t-\t\tif test_run_ \"$2\"\n\t+\t\tif test_run_ \"$1\" \"$2\"\n\t \t\tthen\n\t \t\t\ttest_ok_ \"$1\"\n\t \t\telse\n\tdiff --git a/t/test-lib.sh b/t/test-lib.sh\n\tindex d3f6af6a65..5a1192e80c 100644\n\t--- a/t/test-lib.sh\n\t+++ b/t/test-lib.sh\n\t@@ -935,9 +935,20 @@ test_eval_ () {\n\t }\n\t \n\t test_run_ () {\n\t+\tlocal description\n\t+\tdescription=\"$1\"\n\t+\tshift\n\t+\n\t \ttest_cleanup=:\n\t \texpecting_failure=$2\n\t \n\t+\tif test -n \"$expecting_failure\"\n\t+\tthen\n\t+\t\tsay >&3 \"checking known breakage of $TEST_NUMBER.$test_count '$description': $1\"\n\t+\telse\n\t+\t\tsay >&3 \"expecting success of $TEST_NUMBER.$test_count '$description': $1\"\n\t+\tfi\n\t+\n\t \tif test \"${GIT_TEST_CHAIN_LINT:-1}\" != 0; then\n\t \t\t# turn off tracing for this test-eval, as it simply creates\n\t \t\t# confusing noise in the \"-x\" output\n\n... or maybe not, but in any case, if the verbose mode was what was\nstopping you from moving this down to \"test_run_\" just that seems like\nan easy change.\n\nI like your current implementation better, i.e. to have the stdin\nconsumption happen ASAP and have the others be low-level utility\nfunctions, but I don't care much, but if you wanted it the other way\nmaybe the above diff helps.\n\t\n>   - The helper function is a little awkward to call, as you feed it the\n>     name of the variable you want to set. The more natural thing in\n>     shell would be command substitution like:\n>\n>       body=$(body_or_stdin \"$2\")\n>\n>     but that loses trailing whitespace. There are tricks around this,\n>     like:\n>\n>       body=$(body_or_stdin \"$2\"; printf '.)\n>       body=${body%.}\n>\n>     but we'd prefer to keep such tricks in the helper, not in each\n>     caller.\n\nI see why you did this, and for a narrow change it's a good thing.\n\nFWIW having spent some more time on making the TAP format more pruttah\nin a parallel WIP series I think this is ultimately a losing\ngame. You're inserting the extra LF because you don't want to have the\n\"checking...\" and the first line of the test body on the same line;\n\nBut we have all of:\n\n    test_expect_success 'foo' 'true'\n    test_expect_success 'foo' '\n        true\n    '\n\nAnd now:\n\n    test_expect_success 'foo' - <<\\EOT\n        true\n    '\n\nSo if (definitely not needed for your change) wanted to always make this\npretty/indented we'd need to push that logic down to the formatter,\nwhich would insert a leading LF and/or indentation as appropriate.\n\nI just declared that if you use the first form you don't get\nindentation :)\n\n>   - I implemented the helper using a sequence of \"read\" calls. Together\n>     with \"-r\" and unsetting the IFS, this preserves incoming whitespace.\n>     An alternative is to use \"cat\" (which then requires the gross \".\"\n>     trick above). But this saves us a process, which is probably a good\n>     thing. The \"read\" builtin does use more read() syscalls than\n>     necessary (one per byte), but that is almost certainly a win over a\n>     separate process.\n>\n>     Both are probably slower than passing a single-quoted string, but\n>     the difference is lost in the noise for a script that I converted as\n>     an experiment.\n>\n>   - I handle test_expect_success and test_expect_failure here. If we\n>     like this style, we could easily extend it to other spots (e.g.,\n>     lazy_prereq bodies) on top of this patch.\n>\n>   - even though we are using \"local\", we have to be careful about our\n>     variable names. Within test_expect_success, any variable we declare\n>     with local will be seen by the test snippets themselves (so it won't\n>     persist between tests like normal variables would).\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n"}]}