{"thread":{"id":"42047","subject":"[PATCH v2 0/2] t1500-rev-parse: re-write t1500","startedAt":"2016-04-16T16:13:48Z","lastAt":"2016-04-17T16:22:23Z","messageCount":13,"participants":["Michael Rappazzo","Eric Sunshine","Jeff King","SZEDER Gábor","Johannes Sixt"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"283652","messageId":"1460823230-45692-1-git-send-email-rappazzo@gmail.com","threadId":"42047","inReplyTo":null,"subject":"[PATCH v2 0/2] t1500-rev-parse: re-write t1500","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-04-16T16:13:48Z","receivedAt":"2016-04-16T16:13:48Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"Differences between v1[1]:\n\t- Rebased the change on master.\n\t- Added a test-lib function `test_stdout` which is similar to `test_cmp`.\n\t  This addition is based on a patch from Jeff King[2] found the same \n\t  discussion.\n\t- Cleaned up the use of subshells as recommended in the discussion.\n\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/291087\n[2] http://thread.gmane.org/gmane.comp.version-control.git/291087/focus=291475\n\nMichael Rappazzo (2):\n  test-lib: add a function to compare an expection with stdout from a\n    command\n  t1500-rev-parse: rewrite each test to run in isolation\n\n t/t1500-rev-parse.sh    | 355 ++++++++++++++++++++++++++++++++++++++++--------\n t/test-lib-functions.sh |  34 +++++\n 2 files changed, 329 insertions(+), 60 deletions(-)\n\n-- \n2.8.0\n"},{"id":"283653","messageId":"1460823230-45692-2-git-send-email-rappazzo@gmail.com","threadId":"42047","inReplyTo":"1460823230-45692-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH v2 1/2] test-lib: add a function to compare an expection with stdout from a command","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-04-16T16:13:49Z","receivedAt":"2016-04-16T16:13:49Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"test_stdout accepts an expection and a command to execute.  It will execute\nthe command and then compare the stdout from that command to an expectation.\nIf the expectation is not met, a mock diff output is written to stderr.\n\nBased-on-a-patch-by: Jeff King <peff@peff.net>\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n t/test-lib-functions.sh | 34 ++++++++++++++++++++++++++++++++++\n 1 file changed, 34 insertions(+)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 8d99eb3..95e54b2 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -941,3 +941,37 @@ mingw_read_file_strip_cr_ () {\n \t\teval \"$1=\\$$1\\$line\"\n \tdone\n }\n+\n+#\ttest_stdout is a helper function to compare expected output with\n+#\tthe standard output of a command execution\n+#\n+#\tArgs:\n+#\t\t1: The expected output\n+#\t\t2: The command to run\n+#\n+#\tYou can use it like:\n+#\n+#\ttest_expect_success 'foo works' '\n+#\t\ttest_cmp \"This is expected\" cmd_to_run arg1 arg2 ... argN\n+#\t'\n+#\n+#\tThe output when there is a mismatch mimics diff output, but this\n+#\tcan break down for a multi-line result\n+test_stdout () {\n+\texpect=$1\n+\tshift\n+\tif ! actual=$(\"$@\")\n+\tthen\n+\t\techo \"test_stdout: command failed: '$*'\" >&2\n+\t\treturn 1\n+\tfi\n+\tif test \"$expect\" != \"$actual\"\n+\tthen\n+\t\techo \"test_stdout: unexpected output for '$*'\" >&2\n+\t\techo \"@@ -N +N @@\" >&2\n+\t\techo \"-$expect\" >&2\n+\t\techo \"+$actual\" >&2\n+\t\treturn 1\n+\tfi\n+\treturn 0\n+}\n-- \n2.8.0\n"},{"id":"283654","messageId":"1460823230-45692-3-git-send-email-rappazzo@gmail.com","threadId":"42047","inReplyTo":"1460823230-45692-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH v2 2/2] t1500-rev-parse: rewrite each test to run in isolation","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-04-16T16:13:50Z","receivedAt":"2016-04-16T16:13:50Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"t1500-rev-parse has many tests which change directories and leak\nenvironment variables.  This makes it difficult to add new tests without\nminding the environment variables and current directory.\n\nEach test is now setup, executed, and cleaned up without leaving anything\nbehind.  Test comparisons been converted to use test_cmp or test_stdout.\n\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n t/t1500-rev-parse.sh | 355 ++++++++++++++++++++++++++++++++++++++++++---------\n 1 file changed, 295 insertions(+), 60 deletions(-)\n\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex 48ee077..e2c2a06 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -3,85 +3,320 @@\n test_description='test git rev-parse'\n . ./test-lib.sh\n \n-test_rev_parse() {\n-\tname=$1\n-\tshift\n+test_expect_success 'toplevel: is-bare-repository' '\n+\ttest_stdout false git rev-parse --is-bare-repository\n+'\n \n-\ttest_expect_success \"$name: is-bare-repository\" \\\n-\t\"test '$1' = \\\"\\$(git rev-parse --is-bare-repository)\\\"\"\n-\tshift\n-\t[ $# -eq 0 ] && return\n+test_expect_success 'toplevel: is-inside-git-dir' '\n+\ttest_stdout false git rev-parse --is-inside-git-dir\n+'\n \n-\ttest_expect_success \"$name: is-inside-git-dir\" \\\n-\t\"test '$1' = \\\"\\$(git rev-parse --is-inside-git-dir)\\\"\"\n-\tshift\n-\t[ $# -eq 0 ] && return\n+test_expect_success 'toplevel: is-inside-work-tree' '\n+\ttest_stdout true git rev-parse --is-inside-work-tree\n+'\n \n-\ttest_expect_success \"$name: is-inside-work-tree\" \\\n-\t\"test '$1' = \\\"\\$(git rev-parse --is-inside-work-tree)\\\"\"\n-\tshift\n-\t[ $# -eq 0 ] && return\n+test_expect_success 'toplevel: prefix' '\n+\ttest_stdout \"\" git rev-parse --show-prefix\n+'\n \n-\ttest_expect_success \"$name: prefix\" \\\n-\t\"test '$1' = \\\"\\$(git rev-parse --show-prefix)\\\"\"\n-\tshift\n-\t[ $# -eq 0 ] && return\n+test_expect_success 'toplevel: git-dir' '\n+\ttest_stdout .git git rev-parse --git-dir\n+'\n \n-\ttest_expect_success \"$name: git-dir\" \\\n-\t\"test '$1' = \\\"\\$(git rev-parse --git-dir)\\\"\"\n-\tshift\n-\t[ $# -eq 0 ] && return\n-}\n+test_expect_success '.git/: is-bare-repository' '\n+\ttest_stdout false git -C .git rev-parse --is-bare-repository\n+'\n \n-# label is-bare is-inside-git is-inside-work prefix git-dir\n+test_expect_success '.git/: is-inside-git-dir' '\n+\ttest_stdout true git -C .git rev-parse --is-inside-git-dir\n+'\n \n-ROOT=$(pwd)\n+test_expect_success '.git/: is-inside-work-tree' '\n+\ttest_stdout false git -C .git rev-parse --is-inside-work-tree\n+'\n \n-test_rev_parse toplevel false false true '' .git\n+test_expect_success '.git/: prefix' '\n+\ttest_stdout \"\" git -C .git rev-parse --show-prefix\n+'\n \n-cd .git || exit 1\n-test_rev_parse .git/ false true false '' .\n-cd objects || exit 1\n-test_rev_parse .git/objects/ false true false '' \"$ROOT/.git\"\n-cd ../.. || exit 1\n+test_expect_success '.git/: git-dir' '\n+\ttest_stdout . git -C .git rev-parse --git-dir\n+'\n \n-mkdir -p sub/dir || exit 1\n-cd sub/dir || exit 1\n-test_rev_parse subdirectory false false true sub/dir/ \"$ROOT/.git\"\n-cd ../.. || exit 1\n+test_expect_success '.git/objects/: is-bare-repository' '\n+\ttest_stdout false git -C .git/objects rev-parse --is-bare-repository\n+'\n \n-git config core.bare true\n-test_rev_parse 'core.bare = true' true false false\n+test_expect_success '.git/objects/: is-inside-git-dir' '\n+\ttest_stdout true git -C .git/objects rev-parse --is-inside-git-dir\n+'\n \n-git config --unset core.bare\n-test_rev_parse 'core.bare undefined' false false true\n+test_expect_success '.git/objects/: is-inside-work-tree' '\n+\ttest_stdout false git -C .git/objects rev-parse --is-inside-work-tree\n+'\n \n-mkdir work || exit 1\n-cd work || exit 1\n-GIT_DIR=../.git\n-GIT_CONFIG=\"$(pwd)\"/../.git/config\n-export GIT_DIR GIT_CONFIG\n+test_expect_success '.git/objects/: prefix' '\n+\ttest_stdout \"\" git -C .git/objects rev-parse --show-prefix\n+'\n \n-git config core.bare false\n-test_rev_parse 'GIT_DIR=../.git, core.bare = false' false false true ''\n+test_expect_success '.git/objects/: git-dir' '\n+\techo $(pwd)/.git >expect &&\n+\tgit -C .git/objects rev-parse --git-dir >actual &&\n+\ttest_cmp expect actual\n+'\n \n-git config core.bare true\n-test_rev_parse 'GIT_DIR=../.git, core.bare = true' true false false ''\n+test_expect_success 'subdirectory: is-bare-repository' '\n+\tmkdir -p sub/dir &&\n+\ttest_when_finished \"rm -rf sub\" &&\n+\ttest_stdout false git -C sub/dir rev-parse --is-bare-repository\n+'\n \n-git config --unset core.bare\n-test_rev_parse 'GIT_DIR=../.git, core.bare undefined' false false true ''\n+test_expect_success 'subdirectory: is-inside-git-dir' '\n+\tmkdir -p sub/dir &&\n+\ttest_when_finished \"rm -rf sub\" &&\n+\ttest_stdout false git -C sub/dir rev-parse --is-inside-git-dir\n+'\n \n-mv ../.git ../repo.git || exit 1\n-GIT_DIR=../repo.git\n-GIT_CONFIG=\"$(pwd)\"/../repo.git/config\n+test_expect_success 'subdirectory: is-inside-work-tree' '\n+\tmkdir -p sub/dir &&\n+\ttest_when_finished \"rm -rf sub\" &&\n+\ttest_stdout true git -C sub/dir rev-parse --is-inside-work-tree\n+'\n \n-git config core.bare false\n-test_rev_parse 'GIT_DIR=../repo.git, core.bare = false' false false true ''\n+test_expect_success 'subdirectory: prefix' '\n+\tmkdir -p sub/dir &&\n+\ttest_when_finished \"rm -rf sub\" &&\n+\ttest sub/dir/ = \"$(git -C sub/dir rev-parse --show-prefix)\"\n+'\n \n-git config core.bare true\n-test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true false false ''\n+test_expect_success 'subdirectory: git-dir' '\n+\tmkdir -p sub/dir &&\n+\ttest_when_finished \"rm -rf sub\" &&\n+\techo $(pwd)/.git >expect &&\n+\tgit -C sub/dir rev-parse --git-dir >actual &&\n+\ttest_cmp expect actual\n+'\n \n-git config --unset core.bare\n-test_rev_parse 'GIT_DIR=../repo.git, core.bare undefined' false false true ''\n+test_expect_success 'core.bare = true: is-bare-repository' '\n+\ttest_config core.bare true &&\n+\ttest_stdout true git rev-parse --is-bare-repository\n+'\n+\n+test_expect_success 'core.bare = true: is-inside-git-dir' '\n+\ttest_config core.bare true &&\n+\ttest_stdout false git rev-parse --is-inside-git-dir\n+'\n+\n+test_expect_success 'core.bare = true: is-inside-work-tree' '\n+\ttest_config core.bare true &&\n+\ttest_stdout false git rev-parse --is-inside-work-tree\n+'\n+\n+test_expect_success 'core.bare undefined: is-bare-repository' '\n+\ttest_config core.bare \"\" &&\n+\ttest_stdout false git rev-parse --is-bare-repository\n+'\n+\n+test_expect_success 'core.bare undefined: is-inside-git-dir' '\n+\ttest_config core.bare \"\" &&\n+\ttest_stdout false git rev-parse --is-inside-git-dir\n+'\n+\n+test_expect_success 'core.bare undefined: is-inside-work-tree' '\n+\ttest_config core.bare \"\" &&\n+\ttest_stdout true git rev-parse --is-inside-work-tree\n+'\n+\n+test_expect_success 'GIT_DIR=../.git, core.bare = false: is-bare-repository' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\ttest_config -C \"$(pwd)\"/.git core.bare false &&\n+\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-bare-repository\n+'\n+\n+test_expect_success 'GIT_DIR=../.git, core.bare = false: is-inside-git-dir' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\ttest_config -C \"$(pwd)\"/.git core.bare false &&\n+\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-inside-git-dir\n+'\n+\n+test_expect_success 'GIT_DIR=../.git, core.bare = false: is-inside-work-tree' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\ttest_config -C \"$(pwd)\"/.git core.bare false &&\n+\tGIT_DIR=../.git test_stdout true git -C work rev-parse --is-inside-work-tree\n+'\n+\n+test_expect_success 'GIT_DIR=../.git, core.bare = false: prefix' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\ttest_config -C \"$(pwd)\"/.git core.bare false &&\n+\tGIT_DIR=../.git test_stdout \"\" git -C work rev-parse --show-prefix >actual\n+'\n+\n+test_expect_success 'GIT_DIR=../.git, core.bare = true: is-bare-repository' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\ttest_config -C \"$(pwd)\"/.git core.bare true &&\n+\tGIT_DIR=../.git test_stdout true git -C work rev-parse --is-bare-repository\n+'\n+\n+test_expect_success 'GIT_DIR=../.git, core.bare = true: is-inside-git-dir' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\ttest_config -C \"$(pwd)\"/.git core.bare true &&\n+\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-inside-git-dir\n+'\n+\n+test_expect_success 'GIT_DIR=../.git, core.bare = true: is-inside-work-tree' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\ttest_config -C \"$(pwd)\"/.git core.bare true &&\n+\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-inside-work-tree\n+'\n+\n+test_expect_success 'GIT_DIR=../.git, core.bare = true: prefix' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\ttest_config -C \"$(pwd)\"/.git core.bare true &&\n+\tGIT_DIR=../.git test_stdout \"\" git -C work rev-parse --show-prefix\n+'\n+\n+test_expect_success 'GIT_DIR=../.git, core.bare undefined: is-bare-repository' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\ttest_config -C \"$(pwd)\"/.git core.bar = &&\n+\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-bare-repository\n+'\n+\n+test_expect_success 'GIT_DIR=../.git, core.bare undefined: is-inside-git-dir' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\ttest_config -C \"$(pwd)\"/.git core.bar = &&\n+\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-inside-git-dir\n+'\n+\n+test_expect_success 'GIT_DIR=../.git, core.bare undefined: is-inside-work-tree' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\ttest_config -C \"$(pwd)\"/.git core.bar = &&\n+\tGIT_DIR=../.git test_stdout true git -C work rev-parse --is-inside-work-tree\n+'\n+\n+test_expect_success 'GIT_DIR=../.git, core.bare undefined: prefix' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\ttest_config -C \"$(pwd)\"/.git core.bar = &&\n+\tGIT_DIR=../.git test_stdout \"\" git -C work rev-parse --show-prefix\n+'\n+\n+test_expect_success 'GIT_DIR=../repo.git, core.bare = false: is-bare-repository' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\tcp -r .git repo.git &&\n+\ttest_when_finished \"rm -r repo.git\" &&\n+\ttest_config -C \"$(pwd)\"/repo.git core.bare false &&\n+\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-bare-repository\n+'\n+\n+test_expect_success 'GIT_DIR=../repo.git, core.bare = false: is-inside-git-dir' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\tcp -r .git repo.git &&\n+\ttest_when_finished \"rm -r repo.git\" &&\n+\ttest_config -C \"$(pwd)\"/repo.git core.bare false &&\n+\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-inside-git-dir\n+'\n+\n+test_expect_success 'GIT_DIR=../repo.git, core.bare = false: is-inside-work-tree' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\tcp -r .git repo.git &&\n+\ttest_when_finished \"rm -r repo.git\" &&\n+\ttest_config -C \"$(pwd)\"/repo.git core.bare false &&\n+\tGIT_DIR=../repo.git test_stdout true git -C work rev-parse --is-inside-work-tree\n+'\n+\n+test_expect_success 'GIT_DIR=../repo.git, core.bare = false: prefix' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\tcp -r .git repo.git &&\n+\ttest_when_finished \"rm -r repo.git\" &&\n+\ttest_config -C \"$(pwd)\"/repo.git core.bare false &&\n+\tGIT_DIR=../repo.git test_stdout \"\" git -C work rev-parse --show-prefix\n+'\n+\n+test_expect_success 'GIT_DIR=../repo.git, core.bare = true: is-bare-repository' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\tcp -r .git repo.git &&\n+\ttest_when_finished \"rm -r repo.git\" &&\n+\ttest_config -C \"$(pwd)\"/repo.git core.bare true &&\n+\tGIT_DIR=../repo.git test_stdout true git -C work rev-parse --is-bare-repository\n+'\n+\n+test_expect_success 'GIT_DIR=../repo.git, core.bare = true: is-inside-git-dir' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\tcp -r .git repo.git &&\n+\ttest_when_finished \"rm -r repo.git\" &&\n+\ttest_config -C \"$(pwd)\"/repo.git core.bare true &&\n+\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-inside-git-dir\n+'\n+\n+test_expect_success 'GIT_DIR=../repo.git, core.bare = true: is-inside-work-tree' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\tcp -r .git repo.git &&\n+\ttest_when_finished \"rm -r repo.git\" &&\n+\ttest_config -C \"$(pwd)\"/repo.git core.bare true &&\n+\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-inside-work-tree\n+'\n+\n+test_expect_success 'GIT_DIR=../repo.git, core.bare = true: prefix' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\tcp -r .git repo.git &&\n+\ttest_when_finished \"rm -r repo.git\" &&\n+\ttest_config -C \"$(pwd)\"/repo.git core.bare true &&\n+\tGIT_DIR=../repo.git test_stdout \"\" git -C work rev-parse --show-prefix\n+'\n+\n+test_expect_success 'GIT_DIR=../repo.git, core.bare undefined: is-bare-repository' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\tcp -r .git repo.git &&\n+\ttest_when_finished \"rm -r repo.git\" &&\n+\ttest_config -C \"$(pwd)\"/repo.git core.bare \"\" &&\n+\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-bare-repository\n+'\n+\n+test_expect_success 'GIT_DIR=../repo.git, core.bare undefined: is-inside-git-dir' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\tcp -r .git repo.git &&\n+\ttest_when_finished \"rm -r repo.git\" &&\n+\ttest_config -C \"$(pwd)\"/repo.git core.bare \"\" &&\n+\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-inside-git-dir\n+'\n+\n+test_expect_success 'GIT_DIR=../repo.git, core.bare undefined: is-inside-work-tree' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\tcp -r .git repo.git &&\n+\ttest_when_finished \"rm -r repo.git\" &&\n+\ttest_config -C \"$(pwd)\"/repo.git core.bare \"\" &&\n+\tGIT_DIR=../repo.git test_stdout true git -C work rev-parse --is-inside-work-tree\n+'\n+\n+test_expect_success 'GIT_DIR=../repo.git, core.bare undefined: prefix' '\n+\tmkdir work &&\n+\ttest_when_finished \"rm -rf work\" &&\n+\tcp -r .git repo.git &&\n+\ttest_when_finished \"rm -r repo.git\" &&\n+\ttest_config -C \"$(pwd)\"/repo.git core.bare \"\" &&\n+\tGIT_DIR=../repo.git test_stdout \"\" git -C work rev-parse --show-prefix\n+'\n \n test_done\n-- \n2.8.0\n"},{"id":"283663","messageId":"CAPig+cSOuFygsScGn_Nu0_d8mvRik1hQJuanrb-Nvw3ozyt7JQ@mail.gmail.com","threadId":"42047","inReplyTo":"1460823230-45692-2-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v2 1/2] test-lib: add a function to compare an expection with stdout from a command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-17T03:07:02Z","receivedAt":"2016-04-17T03:07:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 16, 2016 at 12:13 PM, Michael Rappazzo <rappazzo@gmail.com> wrote:\n> test-lib: add a function to compare an expection with stdout from a command\n\nRather long subject. Perhaps:\n\n    test-lib: add convenience function to check command output\n\n> test_stdout accepts an expection and a command to execute.  It will execute\n> the command and then compare the stdout from that command to an expectation.\n> If the expectation is not met, a mock diff output is written to stderr.\n\nI wonder if this deserves more flexibility by accepting a comparison\noperator, such as = and !=, similar to test_line_count()? Although, I\nsuppose such functionality could be added later if deemed useful.\n\n> Based-on-a-patch-by: Jeff King <peff@peff.net>\n\nSince Peff wrote the actual code[1], it might be worthwhile to give\nhim authorship by prepending the commit message with a \"From: Jeff\nKing <peff@peff.net>\" header.\n\n> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>\n> ---\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> @@ -941,3 +941,37 @@ mingw_read_file_strip_cr_ () {\n> +#      test_stdout is a helper function to compare expected output with\n> +#      the standard output of a command execution\n> +#\n> +#      Args:\n> +#              1: The expected output\n> +#              2: The command to run\n> +#\n> +#      You can use it like:\n> +#\n> +#      test_expect_success 'foo works' '\n> +#              test_cmp \"This is expected\" cmd_to_run arg1 arg2 ... argN\n> +#      '\n\ntest_cmp?\n\n> +#      The output when there is a mismatch mimics diff output, but this\n> +#      can break down for a multi-line result\n\nHmm, considering that $(...) collapses each whitespace run (including\nnewlines) down to a single space, I don't see how you could get a\nmulti-line result.\n\nBy the way, either the documentation should mention this limitation\n(\"not possible to check multi-line output\") or the implementation\nshould be upgraded to support it.\n\n> +test_stdout () {\n> +       expect=$1\n> +       shift\n> +       if ! actual=$(\"$@\")\n> +       then\n> +               echo \"test_stdout: command failed: '$*'\" >&2\n> +               return 1\n> +       fi\n> +       if test \"$expect\" != \"$actual\"\n> +       then\n> +               echo \"test_stdout: unexpected output for '$*'\" >&2\n> +               echo \"@@ -N +N @@\" >&2\n> +               echo \"-$expect\" >&2\n> +               echo \"+$actual\" >&2\n\nThis faux diff output is quite a bit more noisy than the simple error\nmessage emitted by Peff's original[1] and it doesn't provide any\nadditional useful information, so it doesn't feel like an improvement.\nAdding quotes around $expect and $actual in Peff's error message would\nprobably be an improvement, though.\n\n> +               return 1\n> +       fi\n> +       return 0\n> +}\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/291475\n"},{"id":"283665","messageId":"20160417035414.GA30002@sigill.intra.peff.net","threadId":"42047","inReplyTo":"CAPig+cSOuFygsScGn_Nu0_d8mvRik1hQJuanrb-Nvw3ozyt7JQ@mail.gmail.com","subject":"Re: [PATCH v2 1/2] test-lib: add a function to compare an expection with stdout from a command","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-17T03:54:15Z","receivedAt":"2016-04-17T03:54:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 16, 2016 at 11:07:02PM -0400, Eric Sunshine wrote:\n\n> > test_stdout accepts an expection and a command to execute.  It will execute\n> > the command and then compare the stdout from that command to an expectation.\n> > If the expectation is not met, a mock diff output is written to stderr.\n> \n> I wonder if this deserves more flexibility by accepting a comparison\n> operator, such as = and !=, similar to test_line_count()? Although, I\n> suppose such functionality could be added later if deemed useful.\n\nIMHO the funny syntax would outweigh the readability benefits. Unlike\ntest_line_count(), which is abstracting a portability solution, this is\nmostly just about trying to save a few lines.\n\nThough I do actually find that:\n\n  test_stdout false git rev-parse --whatever\n\nisn't great, because there's no syntactic separator between the expected\noutput and the actual command to run. So I dunno, maybe it would be\nbetter as:\n\n  test_stdout false = git rev-parse --whatever\n\nand then you get \"!=\" for free later on if you want it.\n\nWe could also do:\n\n  test_stdout git rev-parse --whatever <<-\\EOF\n  false\n  EOF\n\nwhich is more robust for multi-line output, but I think part of the\npoint is to keep these as simple one-liners. You're not buying all that\nmuch over:\n\n  cat >expect <<-\\EOF &&\n  false\n  EOF\n  git rev-parse --whatever >actual &&\n  test_cmp expect actual\n\nThough I do admit I've considered such a helper for some tests where\nthat pattern is repeated ad nauseam.\n\n> > Based-on-a-patch-by: Jeff King <peff@peff.net>\n> \n> Since Peff wrote the actual code[1], it might be worthwhile to give\n> him authorship by prepending the commit message with a \"From: Jeff\n> King <peff@peff.net>\" header.\n\nMichael contacted me offline asking how to credit, and I actually\nsuggested the \"Based-on\" route. I'm OK with it either way.\n\nAnd for the record, my contribution is:\n\n  Signed-off-by: Jeff King <peff@peff.net>\n\nin case there are any DCO questions.\n\n-Peff\n"},{"id":"283668","messageId":"20160417055955.GA13384@flurp.local","threadId":"42047","inReplyTo":"1460823230-45692-3-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v2 2/2] t1500-rev-parse: rewrite each test to run in isolation","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-17T05:59:55Z","receivedAt":"2016-04-17T05:59:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 16, 2016 at 12:13:50PM -0400, Michael Rappazzo wrote:\n> t1500-rev-parse has many tests which change directories and leak\n> environment variables.  This makes it difficult to add new tests without\n> minding the environment variables and current directory.\n> \n> Each test is now setup, executed, and cleaned up without leaving anything\n> behind.  Test comparisons been converted to use test_cmp or test_stdout.\n\nOverall, I'm not enthused about how this patch unrolls the systematic\nfunction-driven approach taken by the original code and turns it into\na series of highly repetitive individual tests. Not only does it make\nthe patch mind-numbing to review, but it is far too easy for errors\nto creep into the conversion which simply wouldn't exist if a\nsystematic approach was used (whether via function, table, or\nfor-loops).\n\nThe very fact that you missed several test_stdout conversion\nopportunities and didn't notice a bit of gunk (an unnecessary\n\">actual\") or several bogus and misspelled \"test_config care.bar =\"\ninvocations, makes a good argument that this patch's approach is\nundesirable.\n\nThe fact that I also missed these problems when reading through the\npatch hammers the point home. It wasn't until I started actually\nchanging the patch in order to present you with a \"here's a diff atop\nyour patch which fixes the issues\" as a convenience, that I noticed\nthe more serious problems.\n\n> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>\n> ---\n> diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\n> @@ -3,85 +3,320 @@\n> +test_expect_success '.git/objects/: git-dir' '\n> +\techo $(pwd)/.git >expect &&\n> +\tgit -C .git/objects rev-parse --git-dir >actual &&\n> +\ttest_cmp expect actual\n> +'\n\nYou forgot to convert this test_cmp to test_stdout.\n\n> +test_expect_success 'subdirectory: prefix' '\n> +\tmkdir -p sub/dir &&\n> +\ttest_when_finished \"rm -rf sub\" &&\n> +\ttest sub/dir/ = \"$(git -C sub/dir rev-parse --show-prefix)\"\n> +'\n\nYou forgot to convert this 'test' to test_stdout.\n\n> -git config core.bare true\n> -test_rev_parse 'GIT_DIR=../repo.git, core.bare = true' true false false ''\n> +test_expect_success 'subdirectory: git-dir' '\n> +\tmkdir -p sub/dir &&\n> +\ttest_when_finished \"rm -rf sub\" &&\n> +\techo $(pwd)/.git >expect &&\n\nNit: Here and one other place, you could quote this: \"$(pwd)/.git\"\n\n> +\tgit -C sub/dir rev-parse --git-dir >actual &&\n> +\ttest_cmp expect actual\n> +'\n\nDitto: test_cmp => test_stdout\n\n> +test_expect_success 'core.bare = true: is-bare-repository' '\n> +\ttest_config core.bare true &&\n> +\ttest_stdout true git rev-parse --is-bare-repository\n> +'\n\nIs there a reason you chose to use test_config rather than the more\nconcise '-c foo=bar' as suggested by the review[1]?\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/291368\n\n> +test_expect_success 'core.bare undefined: is-bare-repository' '\n> +\ttest_config core.bare \"\" &&\n\nIs setting core.bare to \"\" really the same as undefining it, or is\nthe effect the same only as an accident of implementation? (Genuine\nquestion; I haven't checked.)\n\n> +\ttest_stdout false git rev-parse --is-bare-repository\n> +'\n> +test_expect_success 'GIT_DIR=../.git, core.bare = false: is-bare-repository' '\n> +\tmkdir work &&\n> +\ttest_when_finished \"rm -rf work\" &&\n> +\ttest_config -C \"$(pwd)\"/.git core.bare false &&\n\nDrop the unnecessary \"$(pwd)\"/ here and elsewhere.\n\n> +\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-bare-repository\n> +'\n> +\n> +test_expect_success 'GIT_DIR=../.git, core.bare = false: prefix' '\n> +\tmkdir work &&\n> +\ttest_when_finished \"rm -rf work\" &&\n> +\ttest_config -C \"$(pwd)\"/.git core.bare false &&\n> +\tGIT_DIR=../.git test_stdout \"\" git -C work rev-parse --show-prefix >actual\n\nDrop the unnecessary '>actual' redirection.\n\n> +'\n> +\n> +test_expect_success 'GIT_DIR=../.git, core.bare undefined: is-bare-repository' '\n> +\tmkdir work &&\n> +\ttest_when_finished \"rm -rf work\" &&\n> +\ttest_config -C \"$(pwd)\"/.git core.bar = &&\n\nWhat is \"core.bar =\" (here and elsewhere)?\n\n> +\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-bare-repository\n> +'\n> +\n> +test_expect_success 'GIT_DIR=../repo.git, core.bare = false: is-bare-repository' '\n> +\tmkdir work &&\n> +\ttest_when_finished \"rm -rf work\" &&\n> +\tcp -r .git repo.git &&\n> +\ttest_when_finished \"rm -r repo.git\" &&\n\nYou could coalesce these two test_when_finished invocations:\n\n    test_when_finished \"rm -rf work repo.git\" &&\n\nWindows folk might appreciate it since process spawning is expensive\nand slow there.\n\n> +\ttest_config -C \"$(pwd)\"/repo.git core.bare false &&\n> +\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-bare-repository\n> +'\n\nFor convenience, here's a diff atop your patch which addresses the\nabove issues (except the question about core.bare set to \"\" versus\nbeing undefined). However, as noted above, I think this patch's\napproach is going in the wrong direction.\n\n--- 8< ---\ndiff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh\nindex e2c2a06..beaf6e3 100755\n--- a/t/t1500-rev-parse.sh\n+++ b/t/t1500-rev-parse.sh\n@@ -60,9 +60,7 @@ test_expect_success '.git/objects/: prefix' '\n '\n \n test_expect_success '.git/objects/: git-dir' '\n-\techo $(pwd)/.git >expect &&\n-\tgit -C .git/objects rev-parse --git-dir >actual &&\n-\ttest_cmp expect actual\n+\ttest_stdout \"$(pwd)/.git\" git -C .git/objects rev-parse --git-dir\n '\n \n test_expect_success 'subdirectory: is-bare-repository' '\n@@ -86,237 +84,193 @@ test_expect_success 'subdirectory: is-inside-work-tree' '\n test_expect_success 'subdirectory: prefix' '\n \tmkdir -p sub/dir &&\n \ttest_when_finished \"rm -rf sub\" &&\n-\ttest sub/dir/ = \"$(git -C sub/dir rev-parse --show-prefix)\"\n+\ttest_stdout sub/dir/ git -C sub/dir rev-parse --show-prefix\n '\n \n test_expect_success 'subdirectory: git-dir' '\n \tmkdir -p sub/dir &&\n \ttest_when_finished \"rm -rf sub\" &&\n-\techo $(pwd)/.git >expect &&\n-\tgit -C sub/dir rev-parse --git-dir >actual &&\n-\ttest_cmp expect actual\n+\ttest_stdout \"$(pwd)/.git\" git -C sub/dir rev-parse --git-dir\n '\n \n test_expect_success 'core.bare = true: is-bare-repository' '\n-\ttest_config core.bare true &&\n-\ttest_stdout true git rev-parse --is-bare-repository\n+\ttest_stdout true git -c core.bare=true rev-parse --is-bare-repository\n '\n \n test_expect_success 'core.bare = true: is-inside-git-dir' '\n-\ttest_config core.bare true &&\n-\ttest_stdout false git rev-parse --is-inside-git-dir\n+\ttest_stdout false git -c core.bare=true rev-parse --is-inside-git-dir\n '\n \n test_expect_success 'core.bare = true: is-inside-work-tree' '\n-\ttest_config core.bare true &&\n-\ttest_stdout false git rev-parse --is-inside-work-tree\n+\ttest_stdout false git -c core.bare=true rev-parse --is-inside-work-tree\n '\n \n test_expect_success 'core.bare undefined: is-bare-repository' '\n-\ttest_config core.bare \"\" &&\n-\ttest_stdout false git rev-parse --is-bare-repository\n+\ttest_stdout false git -c core.bare= rev-parse --is-bare-repository\n '\n \n test_expect_success 'core.bare undefined: is-inside-git-dir' '\n-\ttest_config core.bare \"\" &&\n-\ttest_stdout false git rev-parse --is-inside-git-dir\n+\ttest_stdout false git -c core.bare= rev-parse --is-inside-git-dir\n '\n \n test_expect_success 'core.bare undefined: is-inside-work-tree' '\n-\ttest_config core.bare \"\" &&\n-\ttest_stdout true git rev-parse --is-inside-work-tree\n+\ttest_stdout true git -c core.bare= rev-parse --is-inside-work-tree\n '\n \n test_expect_success 'GIT_DIR=../.git, core.bare = false: is-bare-repository' '\n-\tmkdir work &&\n \ttest_when_finished \"rm -rf work\" &&\n-\ttest_config -C \"$(pwd)\"/.git core.bare false &&\n-\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-bare-repository\n+\tmkdir work &&\n+\tGIT_DIR=../.git test_stdout false git -C work -c core.bare=false rev-parse --is-bare-repository\n '\n \n test_expect_success 'GIT_DIR=../.git, core.bare = false: is-inside-git-dir' '\n-\tmkdir work &&\n \ttest_when_finished \"rm -rf work\" &&\n-\ttest_config -C \"$(pwd)\"/.git core.bare false &&\n-\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-inside-git-dir\n+\tmkdir work &&\n+\tGIT_DIR=../.git test_stdout false git -C work -c core.bare=false rev-parse --is-inside-git-dir\n '\n \n test_expect_success 'GIT_DIR=../.git, core.bare = false: is-inside-work-tree' '\n-\tmkdir work &&\n \ttest_when_finished \"rm -rf work\" &&\n-\ttest_config -C \"$(pwd)\"/.git core.bare false &&\n-\tGIT_DIR=../.git test_stdout true git -C work rev-parse --is-inside-work-tree\n+\tmkdir work &&\n+\tGIT_DIR=../.git test_stdout true git -C work -c core.bare=false rev-parse --is-inside-work-tree\n '\n \n test_expect_success 'GIT_DIR=../.git, core.bare = false: prefix' '\n-\tmkdir work &&\n \ttest_when_finished \"rm -rf work\" &&\n-\ttest_config -C \"$(pwd)\"/.git core.bare false &&\n-\tGIT_DIR=../.git test_stdout \"\" git -C work rev-parse --show-prefix >actual\n+\tmkdir work &&\n+\tGIT_DIR=../.git test_stdout \"\" git -C work -c core.bare=false rev-parse --show-prefix\n '\n \n test_expect_success 'GIT_DIR=../.git, core.bare = true: is-bare-repository' '\n-\tmkdir work &&\n \ttest_when_finished \"rm -rf work\" &&\n-\ttest_config -C \"$(pwd)\"/.git core.bare true &&\n-\tGIT_DIR=../.git test_stdout true git -C work rev-parse --is-bare-repository\n+\tmkdir work &&\n+\tGIT_DIR=../.git test_stdout true git -C work -c core.bare=true rev-parse --is-bare-repository\n '\n \n test_expect_success 'GIT_DIR=../.git, core.bare = true: is-inside-git-dir' '\n-\tmkdir work &&\n \ttest_when_finished \"rm -rf work\" &&\n-\ttest_config -C \"$(pwd)\"/.git core.bare true &&\n-\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-inside-git-dir\n+\tmkdir work &&\n+\tGIT_DIR=../.git test_stdout false git -C work -c core.bare=true rev-parse --is-inside-git-dir\n '\n \n test_expect_success 'GIT_DIR=../.git, core.bare = true: is-inside-work-tree' '\n-\tmkdir work &&\n \ttest_when_finished \"rm -rf work\" &&\n-\ttest_config -C \"$(pwd)\"/.git core.bare true &&\n-\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-inside-work-tree\n+\tmkdir work &&\n+\tGIT_DIR=../.git test_stdout false git -C work -c core.bare=true rev-parse --is-inside-work-tree\n '\n \n test_expect_success 'GIT_DIR=../.git, core.bare = true: prefix' '\n-\tmkdir work &&\n \ttest_when_finished \"rm -rf work\" &&\n-\ttest_config -C \"$(pwd)\"/.git core.bare true &&\n-\tGIT_DIR=../.git test_stdout \"\" git -C work rev-parse --show-prefix\n+\tmkdir work &&\n+\tGIT_DIR=../.git test_stdout \"\" git -C work -c core.bare=true rev-parse --show-prefix\n '\n \n test_expect_success 'GIT_DIR=../.git, core.bare undefined: is-bare-repository' '\n-\tmkdir work &&\n \ttest_when_finished \"rm -rf work\" &&\n-\ttest_config -C \"$(pwd)\"/.git core.bar = &&\n-\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-bare-repository\n+\tmkdir work &&\n+\tGIT_DIR=../.git test_stdout false git -C work -c core.bare= rev-parse --is-bare-repository\n '\n \n test_expect_success 'GIT_DIR=../.git, core.bare undefined: is-inside-git-dir' '\n-\tmkdir work &&\n \ttest_when_finished \"rm -rf work\" &&\n-\ttest_config -C \"$(pwd)\"/.git core.bar = &&\n-\tGIT_DIR=../.git test_stdout false git -C work rev-parse --is-inside-git-dir\n+\tmkdir work &&\n+\tGIT_DIR=../.git test_stdout false git -C work -c core.bare= rev-parse --is-inside-git-dir\n '\n \n test_expect_success 'GIT_DIR=../.git, core.bare undefined: is-inside-work-tree' '\n-\tmkdir work &&\n \ttest_when_finished \"rm -rf work\" &&\n-\ttest_config -C \"$(pwd)\"/.git core.bar = &&\n-\tGIT_DIR=../.git test_stdout true git -C work rev-parse --is-inside-work-tree\n+\tmkdir work &&\n+\tGIT_DIR=../.git test_stdout true git -C work -c core.bare= rev-parse --is-inside-work-tree\n '\n \n test_expect_success 'GIT_DIR=../.git, core.bare undefined: prefix' '\n-\tmkdir work &&\n \ttest_when_finished \"rm -rf work\" &&\n-\ttest_config -C \"$(pwd)\"/.git core.bar = &&\n-\tGIT_DIR=../.git test_stdout \"\" git -C work rev-parse --show-prefix\n+\tmkdir work &&\n+\tGIT_DIR=../.git test_stdout \"\" git -C work -c core.bare= rev-parse --show-prefix\n '\n \n test_expect_success 'GIT_DIR=../repo.git, core.bare = false: is-bare-repository' '\n+\ttest_when_finished \"rm -rf work repo.git\" &&\n \tmkdir work &&\n-\ttest_when_finished \"rm -rf work\" &&\n \tcp -r .git repo.git &&\n-\ttest_when_finished \"rm -r repo.git\" &&\n-\ttest_config -C \"$(pwd)\"/repo.git core.bare false &&\n-\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-bare-repository\n+\tGIT_DIR=../repo.git test_stdout false git -C work -c core.bare=false rev-parse --is-bare-repository\n '\n \n test_expect_success 'GIT_DIR=../repo.git, core.bare = false: is-inside-git-dir' '\n+\ttest_when_finished \"rm -rf work repo.git\" &&\n \tmkdir work &&\n-\ttest_when_finished \"rm -rf work\" &&\n \tcp -r .git repo.git &&\n-\ttest_when_finished \"rm -r repo.git\" &&\n-\ttest_config -C \"$(pwd)\"/repo.git core.bare false &&\n-\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-inside-git-dir\n+\tGIT_DIR=../repo.git test_stdout false git -C work -c core.bare=false rev-parse --is-inside-git-dir\n '\n \n test_expect_success 'GIT_DIR=../repo.git, core.bare = false: is-inside-work-tree' '\n+\ttest_when_finished \"rm -rf work repo.git\" &&\n \tmkdir work &&\n-\ttest_when_finished \"rm -rf work\" &&\n \tcp -r .git repo.git &&\n-\ttest_when_finished \"rm -r repo.git\" &&\n-\ttest_config -C \"$(pwd)\"/repo.git core.bare false &&\n-\tGIT_DIR=../repo.git test_stdout true git -C work rev-parse --is-inside-work-tree\n+\tGIT_DIR=../repo.git test_stdout true git -C work -c core.bare=false rev-parse --is-inside-work-tree\n '\n \n test_expect_success 'GIT_DIR=../repo.git, core.bare = false: prefix' '\n+\ttest_when_finished \"rm -rf work repo.git\" &&\n \tmkdir work &&\n-\ttest_when_finished \"rm -rf work\" &&\n \tcp -r .git repo.git &&\n-\ttest_when_finished \"rm -r repo.git\" &&\n-\ttest_config -C \"$(pwd)\"/repo.git core.bare false &&\n-\tGIT_DIR=../repo.git test_stdout \"\" git -C work rev-parse --show-prefix\n+\tGIT_DIR=../repo.git test_stdout \"\" git -C work -c core.bare=false rev-parse --show-prefix\n '\n \n test_expect_success 'GIT_DIR=../repo.git, core.bare = true: is-bare-repository' '\n+\ttest_when_finished \"rm -rf work repo.git\" &&\n \tmkdir work &&\n-\ttest_when_finished \"rm -rf work\" &&\n \tcp -r .git repo.git &&\n-\ttest_when_finished \"rm -r repo.git\" &&\n-\ttest_config -C \"$(pwd)\"/repo.git core.bare true &&\n-\tGIT_DIR=../repo.git test_stdout true git -C work rev-parse --is-bare-repository\n+\tGIT_DIR=../repo.git test_stdout true git -C work -c core.bare=true rev-parse --is-bare-repository\n '\n \n test_expect_success 'GIT_DIR=../repo.git, core.bare = true: is-inside-git-dir' '\n+\ttest_when_finished \"rm -rf work repo.git\" &&\n \tmkdir work &&\n-\ttest_when_finished \"rm -rf work\" &&\n \tcp -r .git repo.git &&\n-\ttest_when_finished \"rm -r repo.git\" &&\n-\ttest_config -C \"$(pwd)\"/repo.git core.bare true &&\n-\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-inside-git-dir\n+\tGIT_DIR=../repo.git test_stdout false git -C work -c core.bare=true rev-parse --is-inside-git-dir\n '\n \n test_expect_success 'GIT_DIR=../repo.git, core.bare = true: is-inside-work-tree' '\n+\ttest_when_finished \"rm -rf work repo.git\" &&\n \tmkdir work &&\n-\ttest_when_finished \"rm -rf work\" &&\n \tcp -r .git repo.git &&\n-\ttest_when_finished \"rm -r repo.git\" &&\n-\ttest_config -C \"$(pwd)\"/repo.git core.bare true &&\n-\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-inside-work-tree\n+\tGIT_DIR=../repo.git test_stdout false git -C work -c core.bare=true rev-parse --is-inside-work-tree\n '\n \n test_expect_success 'GIT_DIR=../repo.git, core.bare = true: prefix' '\n+\ttest_when_finished \"rm -rf work repo.git\" &&\n \tmkdir work &&\n-\ttest_when_finished \"rm -rf work\" &&\n \tcp -r .git repo.git &&\n-\ttest_when_finished \"rm -r repo.git\" &&\n-\ttest_config -C \"$(pwd)\"/repo.git core.bare true &&\n-\tGIT_DIR=../repo.git test_stdout \"\" git -C work rev-parse --show-prefix\n+\tGIT_DIR=../repo.git test_stdout \"\" git -C work -c core.bare=true rev-parse --show-prefix\n '\n \n test_expect_success 'GIT_DIR=../repo.git, core.bare undefined: is-bare-repository' '\n+\ttest_when_finished \"rm -rf work repo.git\" &&\n \tmkdir work &&\n-\ttest_when_finished \"rm -rf work\" &&\n \tcp -r .git repo.git &&\n-\ttest_when_finished \"rm -r repo.git\" &&\n-\ttest_config -C \"$(pwd)\"/repo.git core.bare \"\" &&\n-\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-bare-repository\n+\tGIT_DIR=../repo.git test_stdout false git -C work -c core.bare= rev-parse --is-bare-repository\n '\n \n test_expect_success 'GIT_DIR=../repo.git, core.bare undefined: is-inside-git-dir' '\n+\ttest_when_finished \"rm -rf work repo.git\" &&\n \tmkdir work &&\n-\ttest_when_finished \"rm -rf work\" &&\n \tcp -r .git repo.git &&\n-\ttest_when_finished \"rm -r repo.git\" &&\n-\ttest_config -C \"$(pwd)\"/repo.git core.bare \"\" &&\n-\tGIT_DIR=../repo.git test_stdout false git -C work rev-parse --is-inside-git-dir\n+\tGIT_DIR=../repo.git test_stdout false git -C work -c core.bare= rev-parse --is-inside-git-dir\n '\n \n test_expect_success 'GIT_DIR=../repo.git, core.bare undefined: is-inside-work-tree' '\n+\ttest_when_finished \"rm -rf work repo.git\" &&\n \tmkdir work &&\n-\ttest_when_finished \"rm -rf work\" &&\n \tcp -r .git repo.git &&\n-\ttest_when_finished \"rm -r repo.git\" &&\n-\ttest_config -C \"$(pwd)\"/repo.git core.bare \"\" &&\n-\tGIT_DIR=../repo.git test_stdout true git -C work rev-parse --is-inside-work-tree\n+\tGIT_DIR=../repo.git test_stdout true git -C work -c core.bare= rev-parse --is-inside-work-tree\n '\n \n test_expect_success 'GIT_DIR=../repo.git, core.bare undefined: prefix' '\n+\ttest_when_finished \"rm -rf work repo.git\" &&\n \tmkdir work &&\n-\ttest_when_finished \"rm -rf work\" &&\n \tcp -r .git repo.git &&\n-\ttest_when_finished \"rm -r repo.git\" &&\n-\ttest_config -C \"$(pwd)\"/repo.git core.bare \"\" &&\n-\tGIT_DIR=../repo.git test_stdout \"\" git -C work rev-parse --show-prefix\n+\tGIT_DIR=../repo.git test_stdout \"\" git -C work -c core.bare= rev-parse --show-prefix\n '\n \n test_done\n-- \n2.8.1.217.gcab2cda\n"},{"id":"283670","messageId":"CAPig+cTOa2yaMikOJHQXpSjY_EtyUXaqVz4KobQwO2xn=Q6h_w@mail.gmail.com","threadId":"42047","inReplyTo":"20160417035414.GA30002@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/2] test-lib: add a function to compare an expection with stdout from a command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-17T06:36:24Z","receivedAt":"2016-04-17T06:36:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 16, 2016 at 11:54 PM, Jeff King <peff@peff.net> wrote:\n> On Sat, Apr 16, 2016 at 11:07:02PM -0400, Eric Sunshine wrote:\n>> > test_stdout accepts an expection and a command to execute.  It will execute\n>> > the command and then compare the stdout from that command to an expectation.\n>> > If the expectation is not met, a mock diff output is written to stderr.\n>>\n>> I wonder if this deserves more flexibility by accepting a comparison\n>> operator, such as = and !=, similar to test_line_count()? Although, I\n>> suppose such functionality could be added later if deemed useful.\n>\n> [...] Though I do actually find that:\n>\n>   test_stdout false git rev-parse --whatever\n>\n> isn't great, because there's no syntactic separator between the expected\n> output and the actual command to run. So I dunno, maybe it would be\n> better as:\n>\n>   test_stdout false = git rev-parse --whatever\n>\n> [...] We could also do:\n>\n>   test_stdout git rev-parse --whatever <<-\\EOF\n>   false\n>   EOF\n>\n> which is more robust for multi-line output, but I think part of the\n> point is to keep these as simple one-liners. You're not buying all that\n> much over:\n>\n>   cat >expect <<-\\EOF &&\n>   false\n>   EOF\n>   git rev-parse --whatever >actual &&\n>   test_cmp expect actual\n>\n> Though I do admit I've considered such a helper for some tests where\n> that pattern is repeated ad nauseam.\n\nAgreed. I wouldn't mind the version where test_stdout grabs \"expected\"\nfrom <<EOF, but, as you say, it doesn't buy much over the manually\nprepared test_cmp version.\n\nI suppose that the one-liner form of test_stdout could have its uses,\nhowever, it bothers me for a couple reasons: (1) it's not generally\nuseful like the version which grabs \"expected\" from <<EOF, (2) it\nsquats on a nice concise name which would better suit the <<EOF\nversion.\n\nAnyhow, this may all be moot (for now) since I think this patch series\nis going in the wrong direction entirely by abandoning the systematic\napproach taken by the original t1500 code, as explained in my\nreview[1]. If modernization of t1500 retains a systematic approach,\nthen the repetitive code which prompted the suggestion of test_stdout\nwon't exist in the first place.\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/291745\n"},{"id":"283671","messageId":"20160417064140.GA31993@sigill.intra.peff.net","threadId":"42047","inReplyTo":"CAPig+cTOa2yaMikOJHQXpSjY_EtyUXaqVz4KobQwO2xn=Q6h_w@mail.gmail.com","subject":"Re: [PATCH v2 1/2] test-lib: add a function to compare an expection with stdout from a command","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-17T06:41:40Z","receivedAt":"2016-04-17T06:41:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Apr 17, 2016 at 02:36:24AM -0400, Eric Sunshine wrote:\n\n> Agreed. I wouldn't mind the version where test_stdout grabs \"expected\"\n> from <<EOF, but, as you say, it doesn't buy much over the manually\n> prepared test_cmp version.\n> \n> I suppose that the one-liner form of test_stdout could have its uses,\n> however, it bothers me for a couple reasons: (1) it's not generally\n> useful like the version which grabs \"expected\" from <<EOF, (2) it\n> squats on a nice concise name which would better suit the <<EOF\n> version.\n\nI think you could get around your second objection by making \"-\" a magic\ntoken, like:\n\n  test_stdout - = git rev-parse ... <<-\\EOF\n  false\n  EOF\n\nThough I admit the combination of \"-\" and \"=\" is pretty ugly to read.\n\nI'm OK with abandoning this line of inquiry, too. This may be a case\nwhere a little repetition makes things a lot less magical to a reader,\nand it's not worth trying to devise the perfect helper.\n\n> Anyhow, this may all be moot (for now) since I think this patch series\n> is going in the wrong direction entirely by abandoning the systematic\n> approach taken by the original t1500 code, as explained in my\n> review[1]. If modernization of t1500 retains a systematic approach,\n> then the repetitive code which prompted the suggestion of test_stdout\n> won't exist in the first place.\n\nFair enough. I haven't really followed the other part of the series very\nclosely.\n\n-Peff\n"},{"id":"283672","messageId":"20160417114253.Horde.giIo57RkUzhAe6GP-RahIrw@webmail.informatik.kit.edu","threadId":"42047","inReplyTo":"1460823230-45692-3-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v2 2/2] t1500-rev-parse: rewrite each test to run in isolation","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2016-04-17T09:42:53Z","receivedAt":"2016-04-17T09:42:53Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Michael Rappazzo <rappazzo@gmail.com>:\n\n> +test_expect_success 'GIT_DIR=../.git, core.bare = false:  \n> is-bare-repository' '\n> +\tmkdir work &&\n> +\ttest_when_finished \"rm -rf work\" &&\n> +\ttest_config -C \"$(pwd)\"/.git core.bare false &&\n> +\tGIT_DIR=../.git test_stdout false git -C work rev-parse  \n> --is-bare-repository\n> +'\n\nHere and in the following tests as well: some shells don't cope that well\nwith a one-shot environmental variable set in front of a shell function.\nSee commit 512477b17528:\n\n     tests: use \"env\" to run commands with temporary env-var settings\n\n     Ordinarily, we would say \"VAR=VAL command\" to execute a tested\n     command with environment variable(s) set only for that command.\n     This however does not work if 'command' is a shell function (most\n     notably 'test_must_fail'); the result of the assignment is retained\n     and affects later commands.\n\n     To avoid this, we used to assign and export environment variables\n     and run such a test in a subshell, like so:\n\n             (\n                     VAR=VAL && export VAR &&\n                     test_must_fail git command to be tested\n             )\n\n     But with \"env\" utility, we should be able to say:\n\n             test_must_fail env VAR=VAL git command to be tested\n\n     which is much shorter and easier to read.\n"},{"id":"283684","messageId":"5713A63D.3060200@kdbg.org","threadId":"42047","inReplyTo":"20160417055955.GA13384@flurp.local","subject":"Re: [PATCH v2 2/2] t1500-rev-parse: rewrite each test to run in isolation","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-04-17T15:05:33Z","receivedAt":"2016-04-17T15:05:33Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 17.04.2016 um 07:59 schrieb Eric Sunshine:\n> On Sat, Apr 16, 2016 at 12:13:50PM -0400, Michael Rappazzo wrote:\n>> +test_expect_success 'GIT_DIR=../.git, core.bare = false: prefix' '\n>> +\tmkdir work &&\n>> +\ttest_when_finished \"rm -rf work\" &&\n>> +\ttest_config -C \"$(pwd)\"/.git core.bare false &&\n>> +\tGIT_DIR=../.git test_stdout \"\" git -C work rev-parse --show-prefix >actual\n>\n> Drop the unnecessary '>actual' redirection.\n\nNot only that: setting an environment variable in front of a shell \nfunction invocation keeps the variable's value in some (most?) shells. \nThis occurs frequently in the new code. I don't know whether we have a \nshorter pattern than\n\n\t(\n\t\tGIT_DIR=../.git &&\n\t\texport GIT_DIR &&\n\t\ttest_stdout \"\" git -C work rev-parse --show-prefix\n\t)\n\n-- Hannes\n"},{"id":"283685","messageId":"5713A979.6030404@kdbg.org","threadId":"42047","inReplyTo":"CAPig+cSOuFygsScGn_Nu0_d8mvRik1hQJuanrb-Nvw3ozyt7JQ@mail.gmail.com","subject":"Re: [PATCH v2 1/2] test-lib: add a function to compare an expection with stdout from a command","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-04-17T15:19:21Z","receivedAt":"2016-04-17T15:19:21Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 17.04.2016 um 05:07 schrieb Eric Sunshine:\n> Hmm, considering that $(...) collapses each whitespace run (including\n> newlines) down to a single space, I don't see how you could get a\n> multi-line result.\n\nNo, it doesn't. It only removes trailing newlines:\n\n~:1004> frotz=$(echo 1; echo; echo 2; echo; echo; echo); echo \"$frotz\"\n1\n\n2\n~:1005>\n\n-- Hannes\n"},{"id":"283687","messageId":"CAPig+cQ+iqteAdEQR0PLZXnOLVuOT8Onbk3DDPujVvCmgnu=OA@mail.gmail.com","threadId":"42047","inReplyTo":"20160417114253.Horde.giIo57RkUzhAe6GP-RahIrw@webmail.informatik.kit.edu","subject":"Re: [PATCH v2 2/2] t1500-rev-parse: rewrite each test to run in isolation","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-17T16:15:19Z","receivedAt":"2016-04-17T16:15:19Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Apr 17, 2016 at 5:42 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> Quoting Michael Rappazzo <rappazzo@gmail.com>:\n>> +test_expect_success 'GIT_DIR=../.git, core.bare = false:\n>> is-bare-repository' '\n>> +       mkdir work &&\n>> +       test_when_finished \"rm -rf work\" &&\n>> +       test_config -C \"$(pwd)\"/.git core.bare false &&\n>> +       GIT_DIR=../.git test_stdout false git -C work rev-parse\n>> --is-bare-repository\n>> +'\n>\n> Here and in the following tests as well: some shells don't cope that well\n> with a one-shot environmental variable set in front of a shell function.\n> See commit 512477b17528:\n>\n>     tests: use \"env\" to run commands with temporary env-var settings\n\nWhile reviewing the patch, I stared at that code for a good while\nthinking that there was something about it I ought to remember but\ncouldn't, so thanks for the reminder (and j6t's too).\n\nConsidering that this patch is probably going in the wrong direction\nand that if, when re-rolled, it takes a systematic approach testing\nthat the original code uses, then the \"need\" for test_stdout\neffectively disappears, so this issue should go away too (but it's\ngood to remember, nevertheless).\n"},{"id":"283688","messageId":"CAPig+cT8SSeYPJe8A2DJcxeVW5KJrnDWEJ1VMWDNy5vRkYj0AA@mail.gmail.com","threadId":"42047","inReplyTo":"5713A979.6030404@kdbg.org","subject":"Re: [PATCH v2 1/2] test-lib: add a function to compare an expection with stdout from a command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-04-17T16:22:23Z","receivedAt":"2016-04-17T16:22:23Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Apr 17, 2016 at 11:19 AM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 17.04.2016 um 05:07 schrieb Eric Sunshine:\n>> Hmm, considering that $(...) collapses each whitespace run (including\n>> newlines) down to a single space, I don't see how you could get a\n>> multi-line result.\n>\n> No, it doesn't. It only removes trailing newlines:\n>\n> ~:1004> frotz=$(echo 1; echo; echo 2; echo; echo; echo); echo \"$frotz\"\n> 1\n>\n> 2\n> ~:1005>\n\nThanks for the correction.\n"}]}