{"thread":{"id":"24640","subject":"[PATCH] rebase -i: add exec command to launch a shell command","startedAt":"2010-08-05T13:00:17Z","lastAt":"2010-08-10T11:18:48Z","messageCount":15,"participants":["Matthieu Moy","Ævar Arnfjörð Bjarmason","Erik Faye-Lund","Jacob Helwig","Junio C Hamano","Jonathan Nieder","Joshua Juran"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"147188","messageId":"1281013217-29577-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24640","inReplyTo":null,"subject":"[PATCH] rebase -i: add exec command to launch a shell command","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-08-05T13:00:17Z","receivedAt":"2010-08-05T13:00:17Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The typical usage pattern would be to run a test (or simply a compilation\ncommand) at given points in history.\n\nThe shell command is ran (from the worktree root), and the rebase is\nstopped when the command fails, to give the user an opportunity to fix\nthe problem before continuing with \"git rebase --continue\".\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n\nSo, back to the \"run from tree root\", but that't now properly\ndocumented and tested.\n\nOne notable difference with my first version is that the command is\nran in a subshell, defaulting to $SHELL (typically for users like me\nwith $SHELL=zsh who may want to take advantage of their shell's\nadvanced features)\n\n Documentation/git-rebase.txt  |   24 +++++++++++++++++++\n git-rebase--interactive.sh    |   20 ++++++++++++++++\n t/lib-rebase.sh               |    2 +\n t/t3404-rebase-interactive.sh |   50 +++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 96 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex be23ad2..4bd4b66 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -459,6 +459,30 @@ sure that the current HEAD is \"B\", and call\n $ git rebase -i -p --onto Q O\n -----------------------------\n \n+Reordering and editing commits usually creates untested intermediate\n+steps.  You may want to check that your history editing did not break\n+anything by running a test, or at least recompiling at intermediate\n+points in history by using the \"exec\" command (shortcut \"x\").  You may\n+do so by creating a todo list like this one:\n+\n+-------------------------------------------\n+pick deadbee Implement feature XXX\n+fixup f1a5c00 Fix to feature XXX\n+exec make\n+pick c0ffeee The oneline of the next commit\n+edit deadbab The oneline of the commit after\n+exec cd subdir; make test\n+...\n+-------------------------------------------\n+\n+The interactive rebase will stop when a command fails (i.e. exits with\n+non-0 status) to give you an opportunity to fix the problem. You can\n+continue with `git rebase --continue`.\n+\n+The \"exec\" command launches the command in a shell (the one specified\n+in `$SHELL`, or the default shell if `$SHELL` is not set), so you can\n+use usual shell commands like \"cd\". The command is run from the\n+root of the working tree.\n \n SPLITTING COMMITS\n -----------------\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex b94c2a0..33d3087 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -537,6 +537,25 @@ do_next () {\n \t\tesac\n \t\trecord_in_rewritten $sha1\n \t\t;;\n+\tx|\"exec\")\n+\t\tread -r command rest < \"$TODO\"\n+\t\tmark_action_done\n+\t\tprintf 'Executing: %s\\n' \"$rest\"\n+\t\t# \"exec\" command doesn't take a sha1 in the todo-list.\n+\t\t# => can't just use $sha1 here.\n+\t\tgit rev-parse --verify HEAD > \"$DOTEST\"/stopped-sha\n+\t\t${SHELL:-@SHELL_PATH@} -c \"$rest\" # Actual execution\n+\t\tstatus=$?\n+\t\tif test \"$status\" -ne 0\n+\t\tthen\n+\t\t\twarn \"Execution failed: $rest\"\n+\t\t\twarn \"You can fix the problem, and then run\"\n+\t\t\twarn\n+\t\t\twarn \"\tgit rebase --continue\"\n+\t\t\twarn\n+\t\t\texit \"$status\"\n+\t\tfi\n+\t\t;;\n \t*)\n \t\twarn \"Unknown command: $command $sha1 $rest\"\n \t\tif git rev-parse --verify -q \"$sha1\" >/dev/null\n@@ -957,6 +976,7 @@ first and then run 'git rebase --continue' again.\"\n #  e, edit = use commit, but stop for amending\n #  s, squash = use commit, but meld into previous commit\n #  f, fixup = like \"squash\", but discard this commit's log message\n+#  x <cmd>, exec <cmd> = Run a shell command <cmd>, and stop if it fails\n #\n # If you remove a line here THAT COMMIT WILL BE LOST.\n # However, if you remove everything, the rebase will be aborted.\ndiff --git a/t/lib-rebase.sh b/t/lib-rebase.sh\nindex 6aefe27..6ccf797 100644\n--- a/t/lib-rebase.sh\n+++ b/t/lib-rebase.sh\n@@ -47,6 +47,8 @@ for line in $FAKE_LINES; do\n \tcase $line in\n \tsquash|fixup|edit|reword)\n \t\taction=\"$line\";;\n+\texec*)\n+\t\techo \"$line\" | sed 's/_/ /g' >> \"$1\";;\n \t\"#\")\n \t\techo '# comment' >> \"$1\";;\n \t\">\")\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 9f03ce6..3b07850 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -64,6 +64,56 @@ test_expect_success 'setup' '\n \tdone\n '\n \n+# debugging-friendly alternatives to \"test -f\" and \"test ! -f\"\n+file_must_exist () {\n+    if ! [ -f \"$1\" ]; then\n+\techo \"file $1 not created.\"\n+\tfalse\n+    fi\n+}\n+\n+file_must_not_exist () {\n+    if [ -f \"$1\" ]; then\n+\techo \"file $1 created while it shouldn't have. $2\"\n+\tfalse\n+    fi\n+}\n+\n+test_expect_success 'rebase -i with the exec command' '\n+\tgit checkout master &&\n+\tFAKE_LINES=\"1 exec_touch_touch-one 2 exec_touch_touch-two exec_false exec_touch_touch-three 3 4\n+\t\texec_touch_\\\"touch-file__name_with_spaces\\\";_touch_touch-after-semicolon 5\" \\\n+\t\ttest_must_fail git rebase -i A &&\n+\tfile_must_exist touch-one &&\n+\tfile_must_exist touch-two &&\n+\tfile_must_not_exist touch-three \"(Should have stopped before)\" &&\n+\ttest $(git rev-parse C) = $(git rev-parse HEAD) || {\n+\t\techo \"Stopped at wrong revision:\"\n+\t\techo \"($(git describe --tags HEAD) instead of C)\"\n+\t\tfalse\n+\t} &&\n+\tgit rebase --continue &&\n+\tfile_must_exist touch-three &&\n+\tfile_must_exist \"touch-file  name with spaces\" &&\n+\tfile_must_exist touch-after-semicolon &&\n+\ttest $(git rev-parse master) = $(git rev-parse HEAD) || {\n+\t\techo \"Stopped at wrong revision:\"\n+\t\techo \"($(git describe --tags HEAD) instead of master)\"\n+\t\tfalse\n+\t} &&\n+\trm -f touch-*\n+'\n+\n+test_expect_success 'rebase -i with the exec command runs from tree root' '\n+\tgit checkout master &&\n+\tmkdir subdir && cd subdir &&\n+\tFAKE_LINES=\"1 exec_touch_touch-subdir\" \\\n+\t\tgit rebase -i HEAD^ &&\n+\tcd .. &&\n+\tfile_must_exist touch-subdir &&\n+\trm -fr subdir\n+'\n+\n test_expect_success 'no changes are a nop' '\n \tgit checkout branch2 &&\n \tgit rebase -i F &&\n-- \n1.7.2.1.30.g18195\n"},{"id":"147191","messageId":"AANLkTinWvJvNOj6Ga7LgTMmEF37GbZN=hQBFJz4EBry5@mail.gmail.com","threadId":"24640","inReplyTo":"1281013217-29577-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] rebase -i: add exec command to launch a shell command","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-05T13:31:18Z","receivedAt":"2010-08-05T13:31:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Aug 5, 2010 at 13:00, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:\n> The typical usage pattern would be to run a test (or simply a compilation\n> command) at given points in history.\n>\n> The shell command is ran (from the worktree root), and the rebase is\n> stopped when the command fails, to give the user an opportunity to fix\n> the problem before continuing with \"git rebase --continue\".\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n>\n> So, back to the \"run from tree root\", but that't now properly\n> documented and tested.\n>\n> One notable difference with my first version is that the command is\n> ran in a subshell, defaulting to $SHELL (typically for users like me\n> with $SHELL=zsh who may want to take advantage of their shell's\n> advanced features)\n>\n>  Documentation/git-rebase.txt  |   24 +++++++++++++++++++\n>  git-rebase--interactive.sh    |   20 ++++++++++++++++\n>  t/lib-rebase.sh               |    2 +\n>  t/t3404-rebase-interactive.sh |   50 +++++++++++++++++++++++++++++++++++++++++\n>  4 files changed, 96 insertions(+), 0 deletions(-)\n>\n> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\n> index be23ad2..4bd4b66 100644\n> --- a/Documentation/git-rebase.txt\n> +++ b/Documentation/git-rebase.txt\n> @@ -459,6 +459,30 @@ sure that the current HEAD is \"B\", and call\n>  $ git rebase -i -p --onto Q O\n>  -----------------------------\n>\n> +Reordering and editing commits usually creates untested intermediate\n> +steps.  You may want to check that your history editing did not break\n> +anything by running a test, or at least recompiling at intermediate\n> +points in history by using the \"exec\" command (shortcut \"x\").  You may\n> +do so by creating a todo list like this one:\n> +\n> +-------------------------------------------\n> +pick deadbee Implement feature XXX\n> +fixup f1a5c00 Fix to feature XXX\n> +exec make\n> +pick c0ffeee The oneline of the next commit\n> +edit deadbab The oneline of the commit after\n> +exec cd subdir; make test\n> +...\n> +-------------------------------------------\n> +\n> +The interactive rebase will stop when a command fails (i.e. exits with\n> +non-0 status) to give you an opportunity to fix the problem. You can\n> +continue with `git rebase --continue`.\n> +\n> +The \"exec\" command launches the command in a shell (the one specified\n> +in `$SHELL`, or the default shell if `$SHELL` is not set), so you can\n> +use usual shell commands like \"cd\". The command is run from the\n\nI think that needs a definite article: \".. use the usual ..\".\n\n> +# debugging-friendly alternatives to \"test -f\" and \"test ! -f\"\n> +file_must_exist () {\n> +    if ! [ -f \"$1\" ]; then\n> +       echo \"file $1 not created.\"\n> +       false\n> +    fi\n> +}\n> +\n> +file_must_not_exist () {\n> +    if [ -f \"$1\" ]; then\n> +       echo \"file $1 created while it shouldn't have. $2\"\n> +       false\n> +    fi\n> +}\n\nAs I pointed out in a previous comment to the series this sort of\ndebug code should either be converted to use \"test\" or we should\nincorporate it into test-lib.sh and use it everywhere.\n\nI somewhat like the latter. It's sometimes hard to see what's going\nwrong with our tests. It'd also translate to TAP subtests.\n\nOtherwise this all looks good. Especially without the fragile mkdir/chdir\npart present in a previous submission.\n"},{"id":"147221","messageId":"vpqfwytnh0m.fsf@bauges.imag.fr","threadId":"24640","inReplyTo":"AANLkTinWvJvNOj6Ga7LgTMmEF37GbZN=hQBFJz4EBry5@mail.gmail.com","subject":"Re: [PATCH] rebase -i: add exec command to launch a shell command","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2010-08-05T16:47:21Z","receivedAt":"2010-08-05T16:47:21Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>> +in `$SHELL`, or the default shell if `$SHELL` is not set), so you can\n>> +use usual shell commands like \"cd\". The command is run from the\n>\n> I think that needs a definite article: \".. use the usual ..\".\n\nI don't think so, especially with a plural \"commands\" after.\nGooglefight agrees with me (\"use the usual commands\" => 350 results,\n\"use usual commands\" => 4900), but that's not a proof. Perhaps a\nnative speaker could help?\n\n> As I pointed out in a previous comment to the series this sort of\n> debug code should either be converted to use \"test\" or we should\n> incorporate it into test-lib.sh and use it everywhere.\n>\n> I somewhat like the latter. It's sometimes hard to see what's going\n> wrong with our tests. It'd also translate to TAP subtests.\n\nI'll do both: use test -f in a first patch, and propose alternative in\nthe second. New patch serie follows.\n\nI don't know TAP and TAP subtests, but if the functions exist, other\npatches can be added on top to improve them.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"147222","messageId":"1281027281-21055-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24640","inReplyTo":"vpqfwytnh0m.fsf@bauges.imag.fr","subject":"[PATCH 1/2] rebase -i: add exec command to launch a shell command","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-08-05T16:54:40Z","receivedAt":"2010-08-05T16:54:40Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The typical usage pattern would be to run a test (or simply a compilation\ncommand) at given points in history.\n\nThe shell command is ran (from the worktree root), and the rebase is\nstopped when the command fails, to give the user an opportunity to fix\nthe problem before continuing with \"git rebase --continue\".\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n Documentation/git-rebase.txt  |   24 ++++++++++++++++++++++++\n git-rebase--interactive.sh    |   20 ++++++++++++++++++++\n t/lib-rebase.sh               |    2 ++\n t/t3404-rebase-interactive.sh |   35 +++++++++++++++++++++++++++++++++++\n 4 files changed, 81 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex be23ad2..4bd4b66 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -459,6 +459,30 @@ sure that the current HEAD is \"B\", and call\n $ git rebase -i -p --onto Q O\n -----------------------------\n \n+Reordering and editing commits usually creates untested intermediate\n+steps.  You may want to check that your history editing did not break\n+anything by running a test, or at least recompiling at intermediate\n+points in history by using the \"exec\" command (shortcut \"x\").  You may\n+do so by creating a todo list like this one:\n+\n+-------------------------------------------\n+pick deadbee Implement feature XXX\n+fixup f1a5c00 Fix to feature XXX\n+exec make\n+pick c0ffeee The oneline of the next commit\n+edit deadbab The oneline of the commit after\n+exec cd subdir; make test\n+...\n+-------------------------------------------\n+\n+The interactive rebase will stop when a command fails (i.e. exits with\n+non-0 status) to give you an opportunity to fix the problem. You can\n+continue with `git rebase --continue`.\n+\n+The \"exec\" command launches the command in a shell (the one specified\n+in `$SHELL`, or the default shell if `$SHELL` is not set), so you can\n+use usual shell commands like \"cd\". The command is run from the\n+root of the working tree.\n \n SPLITTING COMMITS\n -----------------\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex b94c2a0..33d3087 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -537,6 +537,25 @@ do_next () {\n \t\tesac\n \t\trecord_in_rewritten $sha1\n \t\t;;\n+\tx|\"exec\")\n+\t\tread -r command rest < \"$TODO\"\n+\t\tmark_action_done\n+\t\tprintf 'Executing: %s\\n' \"$rest\"\n+\t\t# \"exec\" command doesn't take a sha1 in the todo-list.\n+\t\t# => can't just use $sha1 here.\n+\t\tgit rev-parse --verify HEAD > \"$DOTEST\"/stopped-sha\n+\t\t${SHELL:-@SHELL_PATH@} -c \"$rest\" # Actual execution\n+\t\tstatus=$?\n+\t\tif test \"$status\" -ne 0\n+\t\tthen\n+\t\t\twarn \"Execution failed: $rest\"\n+\t\t\twarn \"You can fix the problem, and then run\"\n+\t\t\twarn\n+\t\t\twarn \"\tgit rebase --continue\"\n+\t\t\twarn\n+\t\t\texit \"$status\"\n+\t\tfi\n+\t\t;;\n \t*)\n \t\twarn \"Unknown command: $command $sha1 $rest\"\n \t\tif git rev-parse --verify -q \"$sha1\" >/dev/null\n@@ -957,6 +976,7 @@ first and then run 'git rebase --continue' again.\"\n #  e, edit = use commit, but stop for amending\n #  s, squash = use commit, but meld into previous commit\n #  f, fixup = like \"squash\", but discard this commit's log message\n+#  x <cmd>, exec <cmd> = Run a shell command <cmd>, and stop if it fails\n #\n # If you remove a line here THAT COMMIT WILL BE LOST.\n # However, if you remove everything, the rebase will be aborted.\ndiff --git a/t/lib-rebase.sh b/t/lib-rebase.sh\nindex 6aefe27..6ccf797 100644\n--- a/t/lib-rebase.sh\n+++ b/t/lib-rebase.sh\n@@ -47,6 +47,8 @@ for line in $FAKE_LINES; do\n \tcase $line in\n \tsquash|fixup|edit|reword)\n \t\taction=\"$line\";;\n+\texec*)\n+\t\techo \"$line\" | sed 's/_/ /g' >> \"$1\";;\n \t\"#\")\n \t\techo '# comment' >> \"$1\";;\n \t\">\")\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 9f03ce6..bba220a 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -64,6 +64,41 @@ test_expect_success 'setup' '\n \tdone\n '\n \n+test_expect_success 'rebase -i with the exec command' '\n+\tgit checkout master &&\n+\tFAKE_LINES=\"1 exec_touch_touch-one 2 exec_touch_touch-two exec_false exec_touch_touch-three 3 4\n+\t\texec_touch_\\\"touch-file__name_with_spaces\\\";_touch_touch-after-semicolon 5\" \\\n+\t\ttest_must_fail git rebase -i A &&\n+\ttest -f touch-one &&\n+\ttest -f touch-two &&\n+\t! test -f touch-three &&\n+\ttest $(git rev-parse C) = $(git rev-parse HEAD) || {\n+\t\techo \"Stopped at wrong revision:\"\n+\t\techo \"($(git describe --tags HEAD) instead of C)\"\n+\t\tfalse\n+\t} &&\n+\tgit rebase --continue &&\n+\ttest -f touch-three &&\n+\ttest -f \"touch-file  name with spaces\" &&\n+\ttest -f touch-after-semicolon &&\n+\ttest $(git rev-parse master) = $(git rev-parse HEAD) || {\n+\t\techo \"Stopped at wrong revision:\"\n+\t\techo \"($(git describe --tags HEAD) instead of master)\"\n+\t\tfalse\n+\t} &&\n+\trm -f touch-*\n+'\n+\n+test_expect_success 'rebase -i with the exec command runs from tree root' '\n+\tgit checkout master &&\n+\tmkdir subdir && cd subdir &&\n+\tFAKE_LINES=\"1 exec_touch_touch-subdir\" \\\n+\t\tgit rebase -i HEAD^ &&\n+\tcd .. &&\n+\ttest -f touch-subdir &&\n+\trm -fr subdir\n+'\n+\n test_expect_success 'no changes are a nop' '\n \tgit checkout branch2 &&\n \tgit rebase -i F &&\n-- \n1.7.2.1.30.g18195\n"},{"id":"147223","messageId":"1281027281-21055-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"24640","inReplyTo":"vpqfwytnh0m.fsf@bauges.imag.fr","subject":"[PATCH 2/2] test-lib: user-friendly alternatives to test [!] [-d|-f]","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-08-05T16:54:41Z","receivedAt":"2010-08-05T16:54:41Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The helper functions are implemented, documented, and used in a few\nplaces to validate them, but not everywhere to avoid useless code churn.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n t/README                      |    8 ++++++++\n t/t3404-rebase-interactive.sh |   18 +++++++++---------\n t/t3407-rebase-abort.sh       |    6 +++---\n t/test-lib.sh                 |   32 ++++++++++++++++++++++++++++++++\n 4 files changed, 52 insertions(+), 12 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 0d1183c..be760b9 100644\n--- a/t/README\n+++ b/t/README\n@@ -467,6 +467,14 @@ library for your script to use.\n    <expected> file.  This behaves like \"cmp\" but produces more\n    helpful output when the test is run with \"-v\" option.\n \n+ - test_file_must_exist <file> [<diagnosis>]\n+   test_file_must_not_exist <file> [<diagnosis>]\n+   test_dir_must_exist <dir> [<diagnosis>]\n+   test_dir_must_not_exist <dir> [<diagnosis>]\n+\n+   check whether a file/directory exists or doesn't. <diagnosis> will\n+   be displayed if the test fails.\n+\n  - test_when_finished <script>\n \n    Prepend <script> to a list of commands to run to clean up\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex bba220a..50787c2 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -69,18 +69,18 @@ test_expect_success 'rebase -i with the exec command' '\n \tFAKE_LINES=\"1 exec_touch_touch-one 2 exec_touch_touch-two exec_false exec_touch_touch-three 3 4\n \t\texec_touch_\\\"touch-file__name_with_spaces\\\";_touch_touch-after-semicolon 5\" \\\n \t\ttest_must_fail git rebase -i A &&\n-\ttest -f touch-one &&\n-\ttest -f touch-two &&\n-\t! test -f touch-three &&\n+\ttest_file_must_exist touch-one &&\n+\ttest_file_must_exist touch-two &&\n+\ttest_file_must_not_exist touch-three \"(Rebase should have stopped before)\" &&\n \ttest $(git rev-parse C) = $(git rev-parse HEAD) || {\n \t\techo \"Stopped at wrong revision:\"\n \t\techo \"($(git describe --tags HEAD) instead of C)\"\n \t\tfalse\n \t} &&\n \tgit rebase --continue &&\n-\ttest -f touch-three &&\n-\ttest -f \"touch-file  name with spaces\" &&\n-\ttest -f touch-after-semicolon &&\n+\ttest_file_must_exist touch-three &&\n+\ttest_file_must_exist \"touch-file  name with spaces\" &&\n+\ttest_file_must_exist touch-after-semicolon &&\n \ttest $(git rev-parse master) = $(git rev-parse HEAD) || {\n \t\techo \"Stopped at wrong revision:\"\n \t\techo \"($(git describe --tags HEAD) instead of master)\"\n@@ -95,7 +95,7 @@ test_expect_success 'rebase -i with the exec command runs from tree root' '\n \tFAKE_LINES=\"1 exec_touch_touch-subdir\" \\\n \t\tgit rebase -i HEAD^ &&\n \tcd .. &&\n-\ttest -f touch-subdir &&\n+\ttest_file_must_exist touch-subdir &&\n \trm -fr subdir\n '\n \n@@ -178,7 +178,7 @@ test_expect_success 'abort' '\n \tgit rebase --abort &&\n \ttest $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&\n \ttest \"$(git symbolic-ref -q HEAD)\" = \"refs/heads/branch1\" &&\n-\t! test -d .git/rebase-merge\n+\ttest_dir_must_not_exist .git/rebase-merge\n '\n \n test_expect_success 'abort with error when new base cannot be checked out' '\n@@ -187,7 +187,7 @@ test_expect_success 'abort with error when new base cannot be checked out' '\n \ttest_must_fail git rebase -i master > output 2>&1 &&\n \tgrep \"Untracked working tree file .file1. would be overwritten\" \\\n \t\toutput &&\n-\t! test -d .git/rebase-merge &&\n+\ttest_dir_must_not_exist .git/rebase-merge &&\n \tgit reset --hard HEAD^\n '\n \ndiff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh\nindex 2999e78..a1615b8 100755\n--- a/t/t3407-rebase-abort.sh\n+++ b/t/t3407-rebase-abort.sh\n@@ -38,7 +38,7 @@ testrebase() {\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n \t\ttest_must_fail git rebase$type master &&\n-\t\ttest -d \"$dotest\" &&\n+\t\ttest_dir_must_not_exist \"$dotest\" &&\n \t\tgit rebase --abort &&\n \t\ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase) &&\n \t\ttest ! -d \"$dotest\"\n@@ -49,7 +49,7 @@ testrebase() {\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n \t\ttest_must_fail git rebase$type master &&\n-\t\ttest -d \"$dotest\" &&\n+\t\ttest_dir_must_not_exist \"$dotest\" &&\n \t\ttest_must_fail git rebase --skip &&\n \t\ttest $(git rev-parse HEAD) = $(git rev-parse master) &&\n \t\tgit rebase --abort &&\n@@ -62,7 +62,7 @@ testrebase() {\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n \t\ttest_must_fail git rebase$type master &&\n-\t\ttest -d \"$dotest\" &&\n+\t\ttest_dir_must_not_exist \"$dotest\" &&\n \t\techo c > a &&\n \t\techo d >> a &&\n \t\tgit add a &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex e8f21d5..3701f2d 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -541,6 +541,38 @@ test_external_without_stderr () {\n \tfi\n }\n \n+# debugging-friendly alternatives to \"test [!] [-f|-d]\"\n+# The commands test the existence or non-existance of $1. $2 can be\n+# given to provide a more precise diagnosis.\n+test_file_must_exist () {\n+    if ! [ -f \"$1\" ]; then\n+\techo \"file $1 doesn't exist. $*\"\n+\tfalse\n+    fi\n+}\n+\n+test_file_must_not_exist () {\n+    if [ -f \"$1\" ]; then\n+\techo \"file $1 exists. $*\"\n+\tfalse\n+    fi\n+}\n+\n+test_dir_must_exist () {\n+    if ! [ -d \"$1\" ]; then\n+\techo \"directory $1 doesn't exist. $*\"\n+\tfalse\n+    fi\n+}\n+\n+test_file_must_not_exist () {\n+    if [ -d \"$1\" ]; then\n+\techo \"directory $1 exists. $*\"\n+\tfalse\n+    fi\n+}\n+\n+\n # This is not among top-level (test_expect_success | test_expect_failure)\n # but is a prefix that can be used in the test script, like:\n #\n-- \n1.7.2.1.30.g18195\n"},{"id":"147224","messageId":"1281027831-22739-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24640","inReplyTo":"1281027281-21055-2-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH v2] test-lib: user-friendly alternatives to test [!] [-d|-f]","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-08-05T17:03:51Z","receivedAt":"2010-08-05T17:03:51Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The helper functions are implemented, documented, and used in a few\nplaces to validate them, but not everywhere to avoid useless code churn.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nSooooory, the first version was plain broken (the tests did not even\npass anymore), forget it. This should be better.\n\n t/README                      |    8 ++++++++\n t/t3404-rebase-interactive.sh |   18 +++++++++---------\n t/t3407-rebase-abort.sh       |    6 +++---\n t/test-lib.sh                 |   32 ++++++++++++++++++++++++++++++++\n 4 files changed, 52 insertions(+), 12 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 0d1183c..be760b9 100644\n--- a/t/README\n+++ b/t/README\n@@ -467,6 +467,14 @@ library for your script to use.\n    <expected> file.  This behaves like \"cmp\" but produces more\n    helpful output when the test is run with \"-v\" option.\n \n+ - test_file_must_exist <file> [<diagnosis>]\n+   test_file_must_not_exist <file> [<diagnosis>]\n+   test_dir_must_exist <dir> [<diagnosis>]\n+   test_dir_must_not_exist <dir> [<diagnosis>]\n+\n+   check whether a file/directory exists or doesn't. <diagnosis> will\n+   be displayed if the test fails.\n+\n  - test_when_finished <script>\n \n    Prepend <script> to a list of commands to run to clean up\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex bba220a..50787c2 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -69,18 +69,18 @@ test_expect_success 'rebase -i with the exec command' '\n \tFAKE_LINES=\"1 exec_touch_touch-one 2 exec_touch_touch-two exec_false exec_touch_touch-three 3 4\n \t\texec_touch_\\\"touch-file__name_with_spaces\\\";_touch_touch-after-semicolon 5\" \\\n \t\ttest_must_fail git rebase -i A &&\n-\ttest -f touch-one &&\n-\ttest -f touch-two &&\n-\t! test -f touch-three &&\n+\ttest_file_must_exist touch-one &&\n+\ttest_file_must_exist touch-two &&\n+\ttest_file_must_not_exist touch-three \"(Rebase should have stopped before)\" &&\n \ttest $(git rev-parse C) = $(git rev-parse HEAD) || {\n \t\techo \"Stopped at wrong revision:\"\n \t\techo \"($(git describe --tags HEAD) instead of C)\"\n \t\tfalse\n \t} &&\n \tgit rebase --continue &&\n-\ttest -f touch-three &&\n-\ttest -f \"touch-file  name with spaces\" &&\n-\ttest -f touch-after-semicolon &&\n+\ttest_file_must_exist touch-three &&\n+\ttest_file_must_exist \"touch-file  name with spaces\" &&\n+\ttest_file_must_exist touch-after-semicolon &&\n \ttest $(git rev-parse master) = $(git rev-parse HEAD) || {\n \t\techo \"Stopped at wrong revision:\"\n \t\techo \"($(git describe --tags HEAD) instead of master)\"\n@@ -95,7 +95,7 @@ test_expect_success 'rebase -i with the exec command runs from tree root' '\n \tFAKE_LINES=\"1 exec_touch_touch-subdir\" \\\n \t\tgit rebase -i HEAD^ &&\n \tcd .. &&\n-\ttest -f touch-subdir &&\n+\ttest_file_must_exist touch-subdir &&\n \trm -fr subdir\n '\n \n@@ -178,7 +178,7 @@ test_expect_success 'abort' '\n \tgit rebase --abort &&\n \ttest $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&\n \ttest \"$(git symbolic-ref -q HEAD)\" = \"refs/heads/branch1\" &&\n-\t! test -d .git/rebase-merge\n+\ttest_dir_must_not_exist .git/rebase-merge\n '\n \n test_expect_success 'abort with error when new base cannot be checked out' '\n@@ -187,7 +187,7 @@ test_expect_success 'abort with error when new base cannot be checked out' '\n \ttest_must_fail git rebase -i master > output 2>&1 &&\n \tgrep \"Untracked working tree file .file1. would be overwritten\" \\\n \t\toutput &&\n-\t! test -d .git/rebase-merge &&\n+\ttest_dir_must_not_exist .git/rebase-merge &&\n \tgit reset --hard HEAD^\n '\n \ndiff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh\nindex 2999e78..0ca81fe 100755\n--- a/t/t3407-rebase-abort.sh\n+++ b/t/t3407-rebase-abort.sh\n@@ -38,7 +38,7 @@ testrebase() {\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n \t\ttest_must_fail git rebase$type master &&\n-\t\ttest -d \"$dotest\" &&\n+\t\ttest_dir_must_exist \"$dotest\" &&\n \t\tgit rebase --abort &&\n \t\ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase) &&\n \t\ttest ! -d \"$dotest\"\n@@ -49,7 +49,7 @@ testrebase() {\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n \t\ttest_must_fail git rebase$type master &&\n-\t\ttest -d \"$dotest\" &&\n+\t\ttest_dir_must_exist \"$dotest\" &&\n \t\ttest_must_fail git rebase --skip &&\n \t\ttest $(git rev-parse HEAD) = $(git rev-parse master) &&\n \t\tgit rebase --abort &&\n@@ -62,7 +62,7 @@ testrebase() {\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n \t\ttest_must_fail git rebase$type master &&\n-\t\ttest -d \"$dotest\" &&\n+\t\ttest_dir_must_exist \"$dotest\" &&\n \t\techo c > a &&\n \t\techo d >> a &&\n \t\tgit add a &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex e8f21d5..694bbe8 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -541,6 +541,38 @@ test_external_without_stderr () {\n \tfi\n }\n \n+# debugging-friendly alternatives to \"test [!] [-f|-d]\"\n+# The commands test the existence or non-existance of $1. $2 can be\n+# given to provide a more precise diagnosis.\n+test_file_must_exist () {\n+\tif ! [ -f \"$1\" ]; then\n+\t\techo \"file $1 doesn't exist. $*\"\n+\t\tfalse\n+\tfi\n+}\n+\n+test_file_must_not_exist () {\n+\tif [ -f \"$1\" ]; then\n+\t\techo \"file $1 exists. $*\"\n+\t\tfalse\n+\tfi\n+}\n+\n+test_dir_must_exist () {\n+\tif ! [ -d \"$1\" ]; then\n+\t\techo \"directory $1 doesn't exist. $*\"\n+\t\tfalse\n+\tfi\n+}\n+\n+test_dir_must_not_exist () {\n+\tif [ -d \"$1\" ]; then\n+\t\techo \"directory $1 exists. $*\"\n+\t\tfalse\n+\tfi\n+}\n+\n+\n # This is not among top-level (test_expect_success | test_expect_failure)\n # but is a prefix that can be used in the test script, like:\n #\n-- \n1.7.2.1.30.g18195\n"},{"id":"147231","messageId":"AANLkTinF51h8s8Q8dpyY7aZioenWLpOY4qVLGusN2fOX@mail.gmail.com","threadId":"24640","inReplyTo":"vpqfwytnh0m.fsf@bauges.imag.fr","subject":"Re: [PATCH] rebase -i: add exec command to launch a shell command","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-08-05T18:24:41Z","receivedAt":"2010-08-05T18:24:41Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Aug 5, 2010 at 6:47 PM, Matthieu Moy\n<Matthieu.Moy@grenoble-inp.fr> wrote:\n> Ęvar Arnfjörš Bjarmason <avarab@gmail.com> writes:\n>\n>>> +in `$SHELL`, or the default shell if `$SHELL` is not set), so you can\n>>> +use usual shell commands like \"cd\". The command is run from the\n>>\n>> I think that needs a definite article: \".. use the usual ..\".\n>\n> I don't think so, especially with a plural \"commands\" after.\n> Googlefight agrees with me (\"use the usual commands\" => 350 results,\n> \"use usual commands\" => 4900), but that's not a proof.\n\nBeing grammatically correct doesn't automatically make a sentence\ngood. \"Use ususal\" is a bit of a tongue-twister, so I'd rewrite that\nto \"use normal\" for that purpose alone.\n\nBut I'm not a native English speaker, and the natives might disagree with me.\n"},{"id":"147232","messageId":"AANLkTi=P4iinacNXgPN8ZCtjiggBEj-OzF8TkKG5pZgU@mail.gmail.com","threadId":"24640","inReplyTo":"vpqfwytnh0m.fsf@bauges.imag.fr","subject":"Re: [PATCH] rebase -i: add exec command to launch a shell command","fromName":"Jacob Helwig","fromEmail":"jacob.helwig@gmail.com","sentAt":"2010-08-05T18:37:40Z","receivedAt":"2010-08-05T18:37:40Z","isPatch":true,"sender":{"key":"jacob.helwig@gmail.com","avatar":"https://avatars.githubusercontent.com/u/14557?v=4"},"body":"On Thu, Aug 5, 2010 at 09:47, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:\n> Ęvar Arnfjörš Bjarmason <avarab@gmail.com> writes:\n>\n>>> +in `$SHELL`, or the default shell if `$SHELL` is not set), so you can\n>>> +use usual shell commands like \"cd\". The command is run from the\n>>\n>> I think that needs a definite article: \".. use the usual ..\".\n>\n> I don't think so, especially with a plural \"commands\" after.\n> Googlefight agrees with me (\"use the usual commands\" => 350 results,\n> \"use usual commands\" => 4900), but that's not a proof. Perhaps a\n> native speaker could help?\n>\n\nYou could probably just drop \"usual\" entirely: ..., so you can use\nshell commands like \"cd\".\n\nI'm not sure that \"usual\" really adds anything there, and might\nactually be confusing.  Does it mean that \"unusual\" ones won't work?\nWhat are \"unusual\" ones?\n\nPossibly make 'like \"cd\"', parenthetical to further show that it's an\nexample, and not saying that only commands along the lines of \"cd\"\nwill work?  ..., so you can use shell commands (for example: cd).\n\n-Jacob\n"},{"id":"147242","messageId":"7vpqxwddd2.fsf@alter.siamese.dyndns.org","threadId":"24640","inReplyTo":"AANLkTi=P4iinacNXgPN8ZCtjiggBEj-OzF8TkKG5pZgU@mail.gmail.com","subject":"Re: [PATCH] rebase -i: add exec command to launch a shell command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-05T20:16:25Z","receivedAt":"2010-08-05T20:16:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Helwig <jacob.helwig@gmail.com> writes:\n\n> On Thu, Aug 5, 2010 at 09:47, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:\n>> Ęvar Arnfjörš Bjarmason <avarab@gmail.com> writes:\n>>\n>>>> +in `$SHELL`, or the default shell if `$SHELL` is not set), so you can\n>>>> +use usual shell commands like \"cd\". The command is run from the\n>>>\n>>> I think that needs a definite article: \".. use the usual ..\".\n>> ...\n> You could probably just drop \"usual\" entirely: ..., so you can use\n> shell commands like \"cd\".\n\nSounds sane.  Will do, unless a native speaker stops me from doing so.\n"},{"id":"147355","messageId":"20100806225705.GA2534@burratino","threadId":"24640","inReplyTo":"1281027831-22739-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH v2] test-lib: user-friendly alternatives to test [!] [-d|-f]","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-08-06T22:57:05Z","receivedAt":"2010-08-06T22:57:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Matthieu Moy wrote:\n\n> The helper functions are implemented, documented, and used in a few\n> places to validate them\n\nWhen I first read this, I thought you were saying these helpers\nalready existed.  This is where the rationale goes, anyway, so maybe:\n\n\tAdd new test_file_must_not_exist et al helpers for\n\tuse by tests to more loudly diagnose failures that\n\tmanifest themselves by the existence or nonexistence\n\tof a file or directory.\n\n\tSo now you can use\n\n\t\ttest_file_must_exist foo \"so there\"\n\n\tfrom your test, and when it fails due to foo being\n\tabsent or being a symlink instead, instead of silence\n\tyou will get (if debugging with \"-v\") the helpful message\n\n\t\tfile foo does not exist. so there.\n\n> +++ b/t/README\n> @@ -467,6 +467,14 @@ library for your script to use.\n>     <expected> file.  This behaves like \"cmp\" but produces more\n>     helpful output when the test is run with \"-v\" option.\n>  \n> + - test_file_must_exist <file> [<diagnosis>]\n> +   test_file_must_not_exist <file> [<diagnosis>]\n> +   test_dir_must_exist <dir> [<diagnosis>]\n> +   test_dir_must_not_exist <dir> [<diagnosis>]\n> +\n> +   check whether a file/directory exists or doesn't. <diagnosis> will\n> +   be displayed if the test fails.\n\nMaybe:\n\n\t- test_file_exists <name> [<diagnosis>]\n\t- test_dir_exists <name> [<diagnosis>]\n\n\t  Check that <name> exists and is a file or directory,\n\t  printing a diagnostic if it does not.  The <diagnosis>\n\t  if present will be used to give some added context to\n\t  the diagnostic.\n\n\t- test_does_not_exist <name> [<diagnosis>]\n\n\t  Check that <name> does not exist, printing a\n\t  diagnostic if it does.  The <diagnosis> will be\n\t  printed on failure as added context if present.\n\nI think the ..._must_exist names put the emphasis in the\nwrong place, and they look funny in \"if\" statements.\n\n> +++ b/t/t3404-rebase-interactive.sh\n> +++ b/t/t3407-rebase-abort.sh\n[examples]\n\nMakes sense.\n\n> +++ b/t/test-lib.sh\n> @@ -541,6 +541,38 @@ test_external_without_stderr () {\n>  \tfi\n>  }\n>  \n> +# debugging-friendly alternatives to \"test [!] [-f|-d]\"\n> +# The commands test the existence or non-existance of $1. $2 can be\n> +# given to provide a more precise diagnosis.\n> +test_file_must_exist () {\n> +\tif ! [ -f \"$1\" ]; then\n> +\t\techo \"file $1 doesn't exist. $*\"\n> +\t\tfalse\n> +\tfi\n> +}\n\nStyle nitpick: if statementss in the test-lib have tended to look like\n\n if [ foo ]\n then\n\tbar\n fi\n\nso far.  Here the whole function is a glorified \"test -f\", so I wonder\nif\n\n\t[ -f \"$1\" ] ||\n\t{\n\t\techo >&2 \"file $1 doesn't exist. $*\"\n\t\tfalse\n\t}\n\nwould not be clearer.  I dunno.\n\n> +test_file_must_not_exist () {\n> +\tif [ -f \"$1\" ]; then\n> +\t\techo \"file $1 exists. $*\"\n> +\t\tfalse\n> +\tfi\n> +}\n\nWhat should happen if $1 exists and is not a file?\n\nI have often run into silent test failures of the sort your patch\nis designed to avoid.  Thanks for tackling it.\n"},{"id":"147364","messageId":"AANLkTimiSJQPcZRZ06BamJPkd8PBkm7CaMcsKRSdEeP_@mail.gmail.com","threadId":"24640","inReplyTo":"20100806225705.GA2534@burratino","subject":"Re: [PATCH v2] test-lib: user-friendly alternatives to test [!] [-d|-f]","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-08-07T00:21:34Z","receivedAt":"2010-08-07T00:21:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, Aug 6, 2010 at 22:57, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Matthieu Moy wrote:\n>        - test_file_exists <name> [<diagnosis>]\n>        - test_dir_exists <name> [<diagnosis>]\n>\n>          Check that <name> exists and is a file or directory,\n>          printing a diagnostic if it does not.  The <diagnosis>\n>          if present will be used to give some added context to\n>          the diagnostic.\n>\n>        - test_does_not_exist <name> [<diagnosis>]\n>\n>          Check that <name> does not exist, printing a\n>          diagnostic if it does.  The <diagnosis> will be\n>          printed on failure as added context if present.\n>\n> I think the ..._must_exist names put the emphasis in the\n> wrong place, and they look funny in \"if\" statements.\n\nPersonally I'd prefer something where I can just think in the \"test\"\nbultin terms and still get debug info, something like (pseduocode)\n\n    gittest() {\n        test \"$#\" = 3 && { bool='!'; shift; } || bool=\n        test=$1; shift\n        args=\"$@\"; shift\n        case \"$test\" in\n        -f)\n            # Handle the common case of -f with a custom message\n        ;;\n        -d)\n            # Same for -d\n        ;;\n        *)\n            # Just pass a switch to test and say \"test with the -X\nswitch\", or something\nesac\n    }\n\n    gittest ! -f ~/.gitconfig\n    gittest -f ~/.gitconfig\n    gittest -d /tmp\n    gittest ! -d /tmp\n    gittest -s /tmp\n\nI'll never be able to fit more test_* functions in my brain, and I\nwrote the docs :)\n\n> Style nitpick: if statementss in the test-lib have tended to look like\n>\n>  if [ foo ]\n>  then\n>        bar\n>  fi\n>\n> so far.  Here the whole function is a glorified \"test -f\", so I wonder\n> if\n>\n>        [ -f \"$1\" ] ||\n>        {\n>                echo >&2 \"file $1 doesn't exist. $*\"\n>                false\n>        }\n>\n> would not be clearer.  I dunno.\n\nThis is the style we usually use:\n\n    if ! test -f \"$1\"\n    then\n        echo >&2 \"file $1 doesn't exist. $*\"\n        false\n    fi\n\n> I have often run into silent test failures of the sort your patch\n> is designed to avoid.  Thanks for tackling it.\n\nYeah, having more intra-test progress is definitely good. Right now I\njust remove things from the tests in an ad-hoc fashion until they\nstart passing if they fail when I debug them.\n\nI mentioned that we could emit these test progress reports as TAP in a\nprevious E-Mail. Here's how that could look like:\n\n    $ perl -MTest::More=no_plan -E '\n        subtest \"A git test\" => sub {\n            pass(\"doing test -f file\");\n            pass(\"git commit ...\");\n            pass(\"test_tick...\");\n            done_testing();\n        } for 1 .. 2\n    '\n        ok 1 - doing test -f file\n        ok 2 - git commit ...\n        ok 3 - test_tick...\n        1..3\n    ok 1 - A git test\n        ok 1 - doing test -f file\n        ok 2 - git commit ...\n        ok 3 - test_tick...\n        1..3\n    ok 2 - A git test\n    1..2\n\nI.e. we could make these intra-test progress reports machine readable.\n\nAs the example shows the obvious next step would be to make other\nutility functions like test_commit() emit a progress status as well.\n"},{"id":"147512","messageId":"7vvd7j7nys.fsf@alter.siamese.dyndns.org","threadId":"24640","inReplyTo":"1281027281-21055-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 2/2] test-lib: user-friendly alternatives to test [!] [-d|-f]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-09T16:25:15Z","receivedAt":"2010-08-09T16:25:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> +   test_file_must_not_exist <file> [<diagnosis>]\n> +   test_dir_must_not_exist <dir> [<diagnosis>]\n\nShould either of these pass?\n\n    mkdir foo && test_file_must_not_exist foo\n    rm -fr foo && >foo && test_dir_must_not_exist foo\n\nI think in most of the test cases we want to write \"must not exist\" to\nmake sure what we are supposed to remove is gone, which would mean that\n(1) we know what that thing is, and (2) we not only just do not want a\nfile \"foo\" when we say \"file-must-not-exist foo\", but we don't expect it\nto be a directory either.\n\nI'd say we would probably want three primitives instead of four:\n\n    test_path_is_file        <path>\n    test_path_is_directory   <path>\n    test_path_is_missing     <path>\n\nThanks.\n"},{"id":"147621","messageId":"1281438055-6971-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24640","inReplyTo":"7vvd7j7nys.fsf@alter.siamese.dyndns.org","subject":"[PATCH] test-lib: user-friendly alternatives to test [-d|-f|-e]","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-08-10T11:00:55Z","receivedAt":"2010-08-10T11:00:55Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The helper functions are implemented, documented, and used in a few\nplaces to validate them, but not everywhere to avoid useless code churn.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n> I'd say we would probably want three primitives instead of four:\n> \n>     test_path_is_file        <path>\n>     test_path_is_directory   <path>\n>     test_path_is_missing     <path>\n\nI buy this. Thanks,\n\n t/README                      |    7 +++++++\n t/t3404-rebase-interactive.sh |   18 +++++++++---------\n t/t3407-rebase-abort.sh       |    6 +++---\n t/test-lib.sh                 |   32 ++++++++++++++++++++++++++++++++\n 4 files changed, 51 insertions(+), 12 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 0d1183c..410499a 100644\n--- a/t/README\n+++ b/t/README\n@@ -467,6 +467,13 @@ library for your script to use.\n    <expected> file.  This behaves like \"cmp\" but produces more\n    helpful output when the test is run with \"-v\" option.\n \n+ - test_path_is_file <file> [<diagnosis>]\n+   test_path_is_dir <dir> [<diagnosis>]\n+   test_path_is_missing <path> [<diagnosis>]\n+\n+   Check whether a file/directory exists or doesn't. <diagnosis> will\n+   be displayed if the test fails.\n+\n  - test_when_finished <script>\n \n    Prepend <script> to a list of commands to run to clean up\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 7b0026e..f78c364 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -69,18 +69,18 @@ test_expect_success 'rebase -i with the exec command' '\n \tFAKE_LINES=\"1 exec_touch_touch-one 2 exec_touch_touch-two exec_false exec_touch_touch-three 3 4\n \t\texec_touch_\\\"touch-file__name_with_spaces\\\";_touch_touch-after-semicolon 5\" \\\n \t\ttest_must_fail git rebase -i A &&\n-\ttest -f touch-one &&\n-\ttest -f touch-two &&\n-\t! test -f touch-three &&\n+\ttest_path_is_file touch-one &&\n+\ttest_path_is_file touch-two &&\n+\ttest_path_is_missing touch-three \"(Rebase should have stopped before)\" &&\n \ttest $(git rev-parse C) = $(git rev-parse HEAD) || {\n \t\techo \"Stopped at wrong revision:\"\n \t\techo \"($(git describe --tags HEAD) instead of C)\"\n \t\tfalse\n \t} &&\n \tgit rebase --continue &&\n-\ttest -f touch-three &&\n-\ttest -f \"touch-file  name with spaces\" &&\n-\ttest -f touch-after-semicolon &&\n+\ttest_path_is_file touch-three &&\n+\ttest_path_is_file \"touch-file  name with spaces\" &&\n+\ttest_path_is_file touch-after-semicolon &&\n \ttest $(git rev-parse master) = $(git rev-parse HEAD) || {\n \t\techo \"Stopped at wrong revision:\"\n \t\techo \"($(git describe --tags HEAD) instead of master)\"\n@@ -95,7 +95,7 @@ test_expect_success 'rebase -i with the exec command runs from tree root' '\n \tFAKE_LINES=\"1 exec_touch_touch-subdir\" \\\n \t\tgit rebase -i HEAD^ &&\n \tcd .. &&\n-\ttest -f touch-subdir &&\n+\ttest_path_is_file touch-subdir &&\n \trm -fr subdir\n '\n \n@@ -191,7 +191,7 @@ test_expect_success 'abort' '\n \tgit rebase --abort &&\n \ttest $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&\n \ttest \"$(git symbolic-ref -q HEAD)\" = \"refs/heads/branch1\" &&\n-\t! test -d .git/rebase-merge\n+\ttest_path_is_missing .git/rebase-merge\n '\n \n test_expect_success 'abort with error when new base cannot be checked out' '\n@@ -200,7 +200,7 @@ test_expect_success 'abort with error when new base cannot be checked out' '\n \ttest_must_fail git rebase -i master > output 2>&1 &&\n \tgrep \"Untracked working tree file .file1. would be overwritten\" \\\n \t\toutput &&\n-\t! test -d .git/rebase-merge &&\n+\ttest_path_is_missing .git/rebase-merge &&\n \tgit reset --hard HEAD^\n '\n \ndiff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh\nindex 2999e78..fbb3f2e 100755\n--- a/t/t3407-rebase-abort.sh\n+++ b/t/t3407-rebase-abort.sh\n@@ -38,7 +38,7 @@ testrebase() {\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n \t\ttest_must_fail git rebase$type master &&\n-\t\ttest -d \"$dotest\" &&\n+\t\ttest_path_is_dir \"$dotest\" &&\n \t\tgit rebase --abort &&\n \t\ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase) &&\n \t\ttest ! -d \"$dotest\"\n@@ -49,7 +49,7 @@ testrebase() {\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n \t\ttest_must_fail git rebase$type master &&\n-\t\ttest -d \"$dotest\" &&\n+\t\ttest_path_is_dir \"$dotest\" &&\n \t\ttest_must_fail git rebase --skip &&\n \t\ttest $(git rev-parse HEAD) = $(git rev-parse master) &&\n \t\tgit rebase --abort &&\n@@ -62,7 +62,7 @@ testrebase() {\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n \t\ttest_must_fail git rebase$type master &&\n-\t\ttest -d \"$dotest\" &&\n+\t\ttest_path_is_dir \"$dotest\" &&\n \t\techo c > a &&\n \t\techo d >> a &&\n \t\tgit add a &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex e8f21d5..a2173dd 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -541,6 +541,38 @@ test_external_without_stderr () {\n \tfi\n }\n \n+# debugging-friendly alternatives to \"test [-f|-d|-e]\"\n+# The commands test the existence or non-existance of $1. $2 can be\n+# given to provide a more precise diagnosis.\n+test_path_is_file () {\n+\tif ! [ -f \"$1\" ]\n+\tthen\n+\t\techo \"File $1 doesn't exist. $*\"\n+\t\tfalse\n+\tfi\n+}\n+\n+test_path_is_dir () {\n+\tif ! [ -d \"$1\" ]\n+\tthen\n+\t\techo \"Directory $1 doesn't exist. $*\"\n+\t\tfalse\n+\tfi\n+}\n+\n+test_path_is_missing () {\n+\tif [ -e \"$1\" ]\n+\tthen\n+\t\techo \"Path exists:\"\n+\t\tls -ld \"$1\"\n+\t\tif [ $# -ge 1 ]; then\n+\t\t\techo \"$*\"\n+\t\tfi\n+\t\tfalse\n+\tfi\n+}\n+\n+\n # This is not among top-level (test_expect_success | test_expect_failure)\n # but is a prefix that can be used in the test script, like:\n #\n-- \n1.7.2.1.52.g95e25.dirty\n"},{"id":"147622","messageId":"E0E79EC3-DC41-40C5-AF38-53C73759EFAE@gmail.com","threadId":"24640","inReplyTo":"1281438055-6971-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] test-lib: user-friendly alternatives to test [-d|-f|-e]","fromName":"Joshua Juran","fromEmail":"jjuran@gmail.com","sentAt":"2010-08-10T11:11:08Z","receivedAt":"2010-08-10T11:11:08Z","isPatch":true,"sender":{"key":"jjuran@gmail.com","avatar":null},"body":"On Aug 10, 2010, at 4:00 AM, Matthieu Moy wrote:\n\n> +# debugging-friendly alternatives to \"test [-f|-d|-e]\"\n> +# The commands test the existence or non-existance of $1. $2 can be\n\ns/existance/existence/\n\nJosh\n"},{"id":"147623","messageId":"1281439128-12910-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24640","inReplyTo":"E0E79EC3-DC41-40C5-AF38-53C73759EFAE@gmail.com","subject":"[PATCH v3] test-lib: user-friendly alternatives to test [-d|-f|-e]","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-08-10T11:18:48Z","receivedAt":"2010-08-10T11:18:48Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"The helper functions are implemented, documented, and used in a few\nplaces to validate them, but not everywhere to avoid useless code churn.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nWith Junio's comment and a typo noticed by Joshua Juran.\n\n t/README                      |    7 +++++++\n t/t3404-rebase-interactive.sh |   18 +++++++++---------\n t/t3407-rebase-abort.sh       |    6 +++---\n t/test-lib.sh                 |   32 ++++++++++++++++++++++++++++++++\n 4 files changed, 51 insertions(+), 12 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 0d1183c..410499a 100644\n--- a/t/README\n+++ b/t/README\n@@ -467,6 +467,13 @@ library for your script to use.\n    <expected> file.  This behaves like \"cmp\" but produces more\n    helpful output when the test is run with \"-v\" option.\n \n+ - test_path_is_file <file> [<diagnosis>]\n+   test_path_is_dir <dir> [<diagnosis>]\n+   test_path_is_missing <path> [<diagnosis>]\n+\n+   Check whether a file/directory exists or doesn't. <diagnosis> will\n+   be displayed if the test fails.\n+\n  - test_when_finished <script>\n \n    Prepend <script> to a list of commands to run to clean up\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 7b0026e..f78c364 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -69,18 +69,18 @@ test_expect_success 'rebase -i with the exec command' '\n \tFAKE_LINES=\"1 exec_touch_touch-one 2 exec_touch_touch-two exec_false exec_touch_touch-three 3 4\n \t\texec_touch_\\\"touch-file__name_with_spaces\\\";_touch_touch-after-semicolon 5\" \\\n \t\ttest_must_fail git rebase -i A &&\n-\ttest -f touch-one &&\n-\ttest -f touch-two &&\n-\t! test -f touch-three &&\n+\ttest_path_is_file touch-one &&\n+\ttest_path_is_file touch-two &&\n+\ttest_path_is_missing touch-three \"(Rebase should have stopped before)\" &&\n \ttest $(git rev-parse C) = $(git rev-parse HEAD) || {\n \t\techo \"Stopped at wrong revision:\"\n \t\techo \"($(git describe --tags HEAD) instead of C)\"\n \t\tfalse\n \t} &&\n \tgit rebase --continue &&\n-\ttest -f touch-three &&\n-\ttest -f \"touch-file  name with spaces\" &&\n-\ttest -f touch-after-semicolon &&\n+\ttest_path_is_file touch-three &&\n+\ttest_path_is_file \"touch-file  name with spaces\" &&\n+\ttest_path_is_file touch-after-semicolon &&\n \ttest $(git rev-parse master) = $(git rev-parse HEAD) || {\n \t\techo \"Stopped at wrong revision:\"\n \t\techo \"($(git describe --tags HEAD) instead of master)\"\n@@ -95,7 +95,7 @@ test_expect_success 'rebase -i with the exec command runs from tree root' '\n \tFAKE_LINES=\"1 exec_touch_touch-subdir\" \\\n \t\tgit rebase -i HEAD^ &&\n \tcd .. &&\n-\ttest -f touch-subdir &&\n+\ttest_path_is_file touch-subdir &&\n \trm -fr subdir\n '\n \n@@ -191,7 +191,7 @@ test_expect_success 'abort' '\n \tgit rebase --abort &&\n \ttest $(git rev-parse new-branch1) = $(git rev-parse HEAD) &&\n \ttest \"$(git symbolic-ref -q HEAD)\" = \"refs/heads/branch1\" &&\n-\t! test -d .git/rebase-merge\n+\ttest_path_is_missing .git/rebase-merge\n '\n \n test_expect_success 'abort with error when new base cannot be checked out' '\n@@ -200,7 +200,7 @@ test_expect_success 'abort with error when new base cannot be checked out' '\n \ttest_must_fail git rebase -i master > output 2>&1 &&\n \tgrep \"Untracked working tree file .file1. would be overwritten\" \\\n \t\toutput &&\n-\t! test -d .git/rebase-merge &&\n+\ttest_path_is_missing .git/rebase-merge &&\n \tgit reset --hard HEAD^\n '\n \ndiff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh\nindex 2999e78..fbb3f2e 100755\n--- a/t/t3407-rebase-abort.sh\n+++ b/t/t3407-rebase-abort.sh\n@@ -38,7 +38,7 @@ testrebase() {\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n \t\ttest_must_fail git rebase$type master &&\n-\t\ttest -d \"$dotest\" &&\n+\t\ttest_path_is_dir \"$dotest\" &&\n \t\tgit rebase --abort &&\n \t\ttest $(git rev-parse to-rebase) = $(git rev-parse pre-rebase) &&\n \t\ttest ! -d \"$dotest\"\n@@ -49,7 +49,7 @@ testrebase() {\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n \t\ttest_must_fail git rebase$type master &&\n-\t\ttest -d \"$dotest\" &&\n+\t\ttest_path_is_dir \"$dotest\" &&\n \t\ttest_must_fail git rebase --skip &&\n \t\ttest $(git rev-parse HEAD) = $(git rev-parse master) &&\n \t\tgit rebase --abort &&\n@@ -62,7 +62,7 @@ testrebase() {\n \t\t# Clean up the state from the previous one\n \t\tgit reset --hard pre-rebase &&\n \t\ttest_must_fail git rebase$type master &&\n-\t\ttest -d \"$dotest\" &&\n+\t\ttest_path_is_dir \"$dotest\" &&\n \t\techo c > a &&\n \t\techo d >> a &&\n \t\tgit add a &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex e8f21d5..d584194 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -541,6 +541,38 @@ test_external_without_stderr () {\n \tfi\n }\n \n+# debugging-friendly alternatives to \"test [-f|-d|-e]\"\n+# The commands test the existence or non-existence of $1. $2 can be\n+# given to provide a more precise diagnosis.\n+test_path_is_file () {\n+\tif ! [ -f \"$1\" ]\n+\tthen\n+\t\techo \"File $1 doesn't exist. $*\"\n+\t\tfalse\n+\tfi\n+}\n+\n+test_path_is_dir () {\n+\tif ! [ -d \"$1\" ]\n+\tthen\n+\t\techo \"Directory $1 doesn't exist. $*\"\n+\t\tfalse\n+\tfi\n+}\n+\n+test_path_is_missing () {\n+\tif [ -e \"$1\" ]\n+\tthen\n+\t\techo \"Path exists:\"\n+\t\tls -ld \"$1\"\n+\t\tif [ $# -ge 1 ]; then\n+\t\t\techo \"$*\"\n+\t\tfi\n+\t\tfalse\n+\tfi\n+}\n+\n+\n # This is not among top-level (test_expect_success | test_expect_failure)\n # but is a prefix that can be used in the test script, like:\n #\n-- \n1.7.2.1.52.g95e25.dirty\n"}]}