{"thread":{"id":"24553","subject":"[PATCH v2 2/5] t/t4004-diff-rename-symlink.sh: use three-arg <prereq>","startedAt":"2010-07-28T10:34:54Z","lastAt":"2010-07-29T02:03:32Z","messageCount":11,"participants":["Ævar Arnfjörð Bjarmason","Jonathan Nieder"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"146608","messageId":"1280313299-721-1-git-send-email-avarab@gmail.com","threadId":"24553","inReplyTo":null,"subject":"[PATCH v2 0/5] Skip tests using 3-arg test_*","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-28T10:34:54Z","receivedAt":"2010-07-28T10:34:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is just like v1 aside from:\n    \n    diff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh\n    index 85a77b3..92a65f4 100755\n    --- a/t/t4004-diff-rename-symlink.sh\n    +++ b/t/t4004-diff-rename-symlink.sh\n    @@ -38 +38 @@ test_expect_success SYMLINKS 'setup diff output' \"\n    -    GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current\n    +    GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current &&\n\nBut I also split the change I made to t/t4004-diff-rename-symlink.sh\nfrom the \"tests: implicitly skip SYMLINKS tests using <prereq>\" patch\nas suggested by Jonathan Nieder.\n\nIt was the only one in the aforementioned commit that didn't just nuke\nskip_all=* + do 's/success /success SYMLINKS /', so it deserved a\nseparate patch.\n    \nÆvar Arnfjörð Bjarmason (5):\n  tests: implicitly skip SYMLINKS tests using <prereq>\n  t/t4004-diff-rename-symlink.sh: use three-arg <prereq>\n  t/t5800-remote-helpers.sh: Skip with prereq on python <2.4\n  t/t7800-difftool.sh: Skip with prereq on no PERL\n  t/README: Update \"Skipping tests\" to align with best practices\n\n t/README                          |   21 +++++++++++++--\n t/t2007-checkout-symlink.sh       |   14 +++-------\n t/t4004-diff-rename-symlink.sh    |   18 +++++--------\n t/t4011-diff-symlink.sh           |   18 ++++---------\n t/t4023-diff-rename-typechange.sh |   14 +++-------\n t/t4114-apply-typechange.sh       |   28 ++++++++-------------\n t/t4115-apply-symlink.sh          |   12 ++-------\n t/t4122-apply-symlink-inside.sh   |   12 ++-------\n t/t5522-pull-symlink.sh           |   14 +++-------\n t/t5800-remote-helpers.sh         |   24 ++++++++----------\n t/t6035-merge-dir-to-symlink.sh   |   20 +++++----------\n t/t7800-difftool.sh               |   49 ++++++++++++++++--------------------\n 12 files changed, 100 insertions(+), 144 deletions(-)\n"},{"id":"146612","messageId":"1280313299-721-2-git-send-email-avarab@gmail.com","threadId":"24553","inReplyTo":"1280313299-721-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 1/5] tests: implicitly skip SYMLINKS tests using <prereq>","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-28T10:34:55Z","receivedAt":"2010-07-28T10:34:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the tests that skipped due to unavailable SYMLINKS support to\nuse the three-arg prereq form of test_expect_success.\n\nNow we get an indication of how many tests that need symlinks are\nbeing skipped on platforms that don't support them.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/t2007-checkout-symlink.sh       |   14 ++++----------\n t/t4011-diff-symlink.sh           |   18 ++++++------------\n t/t4023-diff-rename-typechange.sh |   14 ++++----------\n t/t4114-apply-typechange.sh       |   28 +++++++++++-----------------\n t/t4115-apply-symlink.sh          |   12 +++---------\n t/t4122-apply-symlink-inside.sh   |   12 +++---------\n t/t5522-pull-symlink.sh           |   14 ++++----------\n t/t6035-merge-dir-to-symlink.sh   |   20 +++++++-------------\n 8 files changed, 42 insertions(+), 90 deletions(-)\n\ndiff --git a/t/t2007-checkout-symlink.sh b/t/t2007-checkout-symlink.sh\nindex 05cc8fd..a74ee22 100755\n--- a/t/t2007-checkout-symlink.sh\n+++ b/t/t2007-checkout-symlink.sh\n@@ -6,13 +6,7 @@ test_description='git checkout to switch between branches with symlink<->dir'\n \n . ./test-lib.sh\n \n-if ! test_have_prereq SYMLINKS\n-then\n-\tskip_all=\"symbolic links not supported - skipping tests\"\n-\ttest_done\n-fi\n-\n-test_expect_success setup '\n+test_expect_success SYMLINKS setup '\n \n \tmkdir frotz &&\n \techo hello >frotz/filfre &&\n@@ -38,18 +32,18 @@ test_expect_success setup '\n \n '\n \n-test_expect_success 'switch from symlink to dir' '\n+test_expect_success SYMLINKS 'switch from symlink to dir' '\n \n \tgit checkout master\n \n '\n \n-test_expect_success 'Remove temporary directories & switch to master' '\n+test_expect_success SYMLINKS 'Remove temporary directories & switch to master' '\n \trm -fr frotz xyzzy nitfol &&\n \tgit checkout -f master\n '\n \n-test_expect_success 'switch from dir to symlink' '\n+test_expect_success SYMLINKS 'switch from dir to symlink' '\n \n \tgit checkout side\n \ndiff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh\nindex 918a21a..6f69489 100755\n--- a/t/t4011-diff-symlink.sh\n+++ b/t/t4011-diff-symlink.sh\n@@ -9,12 +9,6 @@ test_description='Test diff of symlinks.\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/diff-lib.sh\n \n-if ! test_have_prereq SYMLINKS\n-then\n-\tskip_all='Symbolic links not supported, skipping tests.'\n-\ttest_done\n-fi\n-\n cat > expected << EOF\n diff --git a/frotz b/frotz\n new file mode 120000\n@@ -26,7 +20,7 @@ index 0000000..7c465af\n \\ No newline at end of file\n EOF\n \n-test_expect_success \\\n+test_expect_success SYMLINKS \\\n     'diff new symlink' \\\n     'ln -s xyzzy frotz &&\n     git update-index &&\n@@ -35,7 +29,7 @@ test_expect_success \\\n     GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree > current &&\n     compare_diff_patch current expected'\n \n-test_expect_success \\\n+test_expect_success SYMLINKS \\\n     'diff unchanged symlink' \\\n     'tree=$(git write-tree) &&\n     git update-index frotz &&\n@@ -52,7 +46,7 @@ index 7c465af..0000000\n \\ No newline at end of file\n EOF\n \n-test_expect_success \\\n+test_expect_success SYMLINKS \\\n     'diff removed symlink' \\\n     'mv frotz frotz2 &&\n     git diff-index -M -p $tree > current &&\n@@ -62,7 +56,7 @@ cat > expected << EOF\n diff --git a/frotz b/frotz\n EOF\n \n-test_expect_success \\\n+test_expect_success SYMLINKS \\\n     'diff identical, but newly created symlink' \\\n     'ln -s xyzzy frotz &&\n     git diff-index -M -p $tree > current &&\n@@ -80,14 +74,14 @@ index 7c465af..df1db54 120000\n \\ No newline at end of file\n EOF\n \n-test_expect_success \\\n+test_expect_success SYMLINKS \\\n     'diff different symlink' \\\n     'rm frotz &&\n     ln -s yxyyz frotz &&\n     git diff-index -M -p $tree > current &&\n     compare_diff_patch current expected'\n \n-test_expect_success \\\n+test_expect_success SYMLINKS \\\n     'diff symlinks with non-existing targets' \\\n     'ln -s narf pinky &&\n     ln -s take\\ over brain &&\ndiff --git a/t/t4023-diff-rename-typechange.sh b/t/t4023-diff-rename-typechange.sh\nindex 40a95a1..5d20acf 100755\n--- a/t/t4023-diff-rename-typechange.sh\n+++ b/t/t4023-diff-rename-typechange.sh\n@@ -4,13 +4,7 @@ test_description='typechange rename detection'\n \n . ./test-lib.sh\n \n-if ! test_have_prereq SYMLINKS\n-then\n-\tskip_all='Symbolic links not supported, skipping tests.'\n-\ttest_done\n-fi\n-\n-test_expect_success setup '\n+test_expect_success SYMLINKS setup '\n \n \trm -f foo bar &&\n \tcat \"$TEST_DIRECTORY\"/../COPYING >foo &&\n@@ -56,7 +50,7 @@ test_expect_success setup '\n \n '\n \n-test_expect_success 'cross renames to be detected for regular files' '\n+test_expect_success SYMLINKS 'cross renames to be detected for regular files' '\n \n \tgit diff-tree five six -r --name-status -B -M | sort >actual &&\n \t{\n@@ -67,7 +61,7 @@ test_expect_success 'cross renames to be detected for regular files' '\n \n '\n \n-test_expect_success 'cross renames to be detected for typechange' '\n+test_expect_success SYMLINKS 'cross renames to be detected for typechange' '\n \n \tgit diff-tree one two -r --name-status -B -M | sort >actual &&\n \t{\n@@ -78,7 +72,7 @@ test_expect_success 'cross renames to be detected for typechange' '\n \n '\n \n-test_expect_success 'moves and renames' '\n+test_expect_success SYMLINKS 'moves and renames' '\n \n \tgit diff-tree three four -r --name-status -B -M | sort >actual &&\n \t{\ndiff --git a/t/t4114-apply-typechange.sh b/t/t4114-apply-typechange.sh\nindex 164d58c..f12826f 100755\n--- a/t/t4114-apply-typechange.sh\n+++ b/t/t4114-apply-typechange.sh\n@@ -9,13 +9,7 @@ test_description='git apply should not get confused with type changes.\n \n . ./test-lib.sh\n \n-if ! test_have_prereq SYMLINKS\n-then\n-\tskip_all='Symbolic links not supported, skipping tests.'\n-\ttest_done\n-fi\n-\n-test_expect_success 'setup repository and commits' '\n+test_expect_success SYMLINKS 'setup repository and commits' '\n \techo \"hello world\" > foo &&\n \techo \"hi planet\" > bar &&\n \tgit update-index --add foo bar &&\n@@ -48,7 +42,7 @@ test_expect_success 'setup repository and commits' '\n \tgit branch foo-baz-renamed-from-foo\n \t'\n \n-test_expect_success 'file renamed from foo to foo/baz' '\n+test_expect_success SYMLINKS 'file renamed from foo to foo/baz' '\n \tgit checkout -f initial &&\n \tgit diff-tree -M -p HEAD foo-baz-renamed-from-foo > patch &&\n \tgit apply --index < patch\n@@ -56,7 +50,7 @@ test_expect_success 'file renamed from foo to foo/baz' '\n test_debug 'cat patch'\n \n \n-test_expect_success 'file renamed from foo/baz to foo' '\n+test_expect_success SYMLINKS 'file renamed from foo/baz to foo' '\n \tgit checkout -f foo-baz-renamed-from-foo &&\n \tgit diff-tree -M -p HEAD initial > patch &&\n \tgit apply --index < patch\n@@ -64,7 +58,7 @@ test_expect_success 'file renamed from foo/baz to foo' '\n test_debug 'cat patch'\n \n \n-test_expect_success 'directory becomes file' '\n+test_expect_success SYMLINKS 'directory becomes file' '\n \tgit checkout -f foo-becomes-a-directory &&\n \tgit diff-tree -p HEAD initial > patch &&\n \tgit apply --index < patch\n@@ -72,7 +66,7 @@ test_expect_success 'directory becomes file' '\n test_debug 'cat patch'\n \n \n-test_expect_success 'file becomes directory' '\n+test_expect_success SYMLINKS 'file becomes directory' '\n \tgit checkout -f initial &&\n \tgit diff-tree -p HEAD foo-becomes-a-directory > patch &&\n \tgit apply --index < patch\n@@ -80,7 +74,7 @@ test_expect_success 'file becomes directory' '\n test_debug 'cat patch'\n \n \n-test_expect_success 'file becomes symlink' '\n+test_expect_success SYMLINKS 'file becomes symlink' '\n \tgit checkout -f initial &&\n \tgit diff-tree -p HEAD foo-symlinked-to-bar > patch &&\n \tgit apply --index < patch\n@@ -88,21 +82,21 @@ test_expect_success 'file becomes symlink' '\n test_debug 'cat patch'\n \n \n-test_expect_success 'symlink becomes file' '\n+test_expect_success SYMLINKS 'symlink becomes file' '\n \tgit checkout -f foo-symlinked-to-bar &&\n \tgit diff-tree -p HEAD foo-back-to-file > patch &&\n \tgit apply --index < patch\n \t'\n test_debug 'cat patch'\n \n-test_expect_success 'binary file becomes symlink' '\n+test_expect_success SYMLINKS 'binary file becomes symlink' '\n \tgit checkout -f foo-becomes-binary &&\n \tgit diff-tree -p --binary HEAD foo-symlinked-to-bar > patch &&\n \tgit apply --index < patch\n \t'\n test_debug 'cat patch'\n \n-test_expect_success 'symlink becomes binary file' '\n+test_expect_success SYMLINKS 'symlink becomes binary file' '\n \tgit checkout -f foo-symlinked-to-bar &&\n \tgit diff-tree -p --binary HEAD foo-becomes-binary > patch &&\n \tgit apply --index < patch\n@@ -110,7 +104,7 @@ test_expect_success 'symlink becomes binary file' '\n test_debug 'cat patch'\n \n \n-test_expect_success 'symlink becomes directory' '\n+test_expect_success SYMLINKS 'symlink becomes directory' '\n \tgit checkout -f foo-symlinked-to-bar &&\n \tgit diff-tree -p HEAD foo-becomes-a-directory > patch &&\n \tgit apply --index < patch\n@@ -118,7 +112,7 @@ test_expect_success 'symlink becomes directory' '\n test_debug 'cat patch'\n \n \n-test_expect_success 'directory becomes symlink' '\n+test_expect_success SYMLINKS 'directory becomes symlink' '\n \tgit checkout -f foo-becomes-a-directory &&\n \tgit diff-tree -p HEAD foo-symlinked-to-bar > patch &&\n \tgit apply --index < patch\ndiff --git a/t/t4115-apply-symlink.sh b/t/t4115-apply-symlink.sh\nindex aff4348..7674dd2 100755\n--- a/t/t4115-apply-symlink.sh\n+++ b/t/t4115-apply-symlink.sh\n@@ -9,13 +9,7 @@ test_description='git apply symlinks and partial files\n \n . ./test-lib.sh\n \n-if ! test_have_prereq SYMLINKS\n-then\n-\tskip_all='Symbolic links not supported, skipping tests.'\n-\ttest_done\n-fi\n-\n-test_expect_success setup '\n+test_expect_success SYMLINKS setup '\n \n \tln -s path1/path2/path3/path4/path5 link1 &&\n \tgit add link? &&\n@@ -34,7 +28,7 @@ test_expect_success setup '\n \n '\n \n-test_expect_success 'apply symlink patch' '\n+test_expect_success SYMLINKS 'apply symlink patch' '\n \n \tgit checkout side &&\n \tgit apply patch &&\n@@ -43,7 +37,7 @@ test_expect_success 'apply symlink patch' '\n \n '\n \n-test_expect_success 'apply --index symlink patch' '\n+test_expect_success SYMLINKS 'apply --index symlink patch' '\n \n \tgit checkout -f side &&\n \tgit apply --index patch &&\ndiff --git a/t/t4122-apply-symlink-inside.sh b/t/t4122-apply-symlink-inside.sh\nindex 923fcab..3940737 100755\n--- a/t/t4122-apply-symlink-inside.sh\n+++ b/t/t4122-apply-symlink-inside.sh\n@@ -3,12 +3,6 @@\n test_description='apply to deeper directory without getting fooled with symlink'\n . ./test-lib.sh\n \n-if ! test_have_prereq SYMLINKS\n-then\n-\tskip_all='Symbolic links not supported, skipping tests.'\n-\ttest_done\n-fi\n-\n lecho () {\n \tfor l_\n \tdo\n@@ -16,7 +10,7 @@ lecho () {\n \tdone\n }\n \n-test_expect_success setup '\n+test_expect_success SYMLINKS setup '\n \n \tmkdir -p arch/i386/boot arch/x86_64 &&\n \tlecho 1 2 3 4 5 >arch/i386/boot/Makefile &&\n@@ -37,7 +31,7 @@ test_expect_success setup '\n \n '\n \n-test_expect_success apply '\n+test_expect_success SYMLINKS apply '\n \n \tgit checkout test &&\n \tgit diff --exit-code test &&\n@@ -46,7 +40,7 @@ test_expect_success apply '\n \n '\n \n-test_expect_success 'check result' '\n+test_expect_success SYMLINKS 'check result' '\n \n \tgit diff --exit-code master &&\n \tgit diff --exit-code --cached master &&\ndiff --git a/t/t5522-pull-symlink.sh b/t/t5522-pull-symlink.sh\nindex 298200f..8e9b204 100755\n--- a/t/t5522-pull-symlink.sh\n+++ b/t/t5522-pull-symlink.sh\n@@ -4,12 +4,6 @@ test_description='pulling from symlinked subdir'\n \n . ./test-lib.sh\n \n-if ! test_have_prereq SYMLINKS\n-then\n-\tskip_all='Symbolic links not supported, skipping tests.'\n-\ttest_done\n-fi\n-\n # The scenario we are building:\n #\n #   trash\\ directory/\n@@ -20,7 +14,7 @@ fi\n #\n # The working directory is subdir-link.\n \n-test_expect_success setup '\n+test_expect_success SYMLINKS setup '\n \tmkdir subdir &&\n \techo file >subdir/file &&\n \tgit add subdir/file &&\n@@ -36,7 +30,7 @@ test_expect_success setup '\n \n # Demonstrate that things work if we just avoid the symlink\n #\n-test_expect_success 'pulling from real subdir' '\n+test_expect_success SYMLINKS 'pulling from real subdir' '\n \t(\n \t\techo real >subdir/file &&\n \t\tgit commit -m real subdir/file &&\n@@ -64,7 +58,7 @@ test_expect_success 'pulling from real subdir' '\n # directory.  A POSIX shell's \"cd\" works a little differently\n # than chdir() in C; \"cd -P\" is much closer to chdir().\n #\n-test_expect_success 'pulling from symlinked subdir' '\n+test_expect_success SYMLINKS 'pulling from symlinked subdir' '\n \t(\n \t\techo link >subdir/file &&\n \t\tgit commit -m link subdir/file &&\n@@ -77,7 +71,7 @@ test_expect_success 'pulling from symlinked subdir' '\n # Prove that the remote end really is a repo, and other commands\n # work fine in this context.  It's just that \"git pull\" breaks.\n #\n-test_expect_success 'pushing from symlinked subdir' '\n+test_expect_success SYMLINKS 'pushing from symlinked subdir' '\n \t(\n \t\tcd subdir-link/ &&\n \t\techo push >file &&\ndiff --git a/t/t6035-merge-dir-to-symlink.sh b/t/t6035-merge-dir-to-symlink.sh\nindex cd3190c..28e8f2a 100755\n--- a/t/t6035-merge-dir-to-symlink.sh\n+++ b/t/t6035-merge-dir-to-symlink.sh\n@@ -3,13 +3,7 @@\n test_description='merging when a directory was replaced with a symlink'\n . ./test-lib.sh\n \n-if ! test_have_prereq SYMLINKS\n-then\n-\tskip_all='Symbolic links not supported, skipping tests.'\n-\ttest_done\n-fi\n-\n-test_expect_success 'create a commit where dir a/b changed to symlink' '\n+test_expect_success SYMLINKS 'create a commit where dir a/b changed to symlink' '\n \tmkdir -p a/b/c a/b-2/c &&\n \t> a/b/c/d &&\n \t> a/b-2/c/d &&\n@@ -23,7 +17,7 @@ test_expect_success 'create a commit where dir a/b changed to symlink' '\n \tgit commit -m \"dir to symlink\"\n '\n \n-test_expect_success 'keep a/b-2/c/d across checkout' '\n+test_expect_success SYMLINKS 'keep a/b-2/c/d across checkout' '\n \tgit checkout HEAD^0 &&\n \tgit reset --hard master &&\n \tgit rm --cached a/b &&\n@@ -32,14 +26,14 @@ test_expect_success 'keep a/b-2/c/d across checkout' '\n \t test -f a/b-2/c/d\n '\n \n-test_expect_success 'checkout should not have deleted a/b-2/c/d' '\n+test_expect_success SYMLINKS 'checkout should not have deleted a/b-2/c/d' '\n \tgit checkout HEAD^0 &&\n \tgit reset --hard master &&\n \t git checkout start^0 &&\n \t test -f a/b-2/c/d\n '\n \n-test_expect_success 'setup for merge test' '\n+test_expect_success SYMLINKS 'setup for merge test' '\n \tgit reset --hard &&\n \ttest -f a/b-2/c/d &&\n \techo x > a/x &&\n@@ -48,7 +42,7 @@ test_expect_success 'setup for merge test' '\n \tgit tag baseline\n '\n \n-test_expect_success 'do not lose a/b-2/c/d in merge (resolve)' '\n+test_expect_success SYMLINKS 'do not lose a/b-2/c/d in merge (resolve)' '\n \tgit reset --hard &&\n \tgit checkout baseline^0 &&\n \tgit merge -s resolve master &&\n@@ -64,7 +58,7 @@ test_expect_failure 'do not lose a/b-2/c/d in merge (recursive)' '\n \ttest -f a/b-2/c/d\n '\n \n-test_expect_success 'setup a merge where dir a/b-2 changed to symlink' '\n+test_expect_success SYMLINKS 'setup a merge where dir a/b-2 changed to symlink' '\n \tgit reset --hard &&\n \tgit checkout start^0 &&\n \trm -rf a/b-2 &&\n@@ -74,7 +68,7 @@ test_expect_success 'setup a merge where dir a/b-2 changed to symlink' '\n \tgit tag test2\n '\n \n-test_expect_success 'merge should not have conflicts (resolve)' '\n+test_expect_success SYMLINKS 'merge should not have conflicts (resolve)' '\n \tgit reset --hard &&\n \tgit checkout baseline^0 &&\n \tgit merge -s resolve test2 &&\n-- \n1.7.0.4\n"},{"id":"146607","messageId":"1280313299-721-3-git-send-email-avarab@gmail.com","threadId":"24553","inReplyTo":"1280313299-721-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 2/5] t/t4004-diff-rename-symlink.sh: use three-arg <prereq>","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-28T10:34:56Z","receivedAt":"2010-07-28T10:34:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the tests that skipped due to unavailable SYMLINKS support to\nuse the three-arg prereq form of test_expect_success.\n\nThis is like the \"tests: implicitly skip SYMLINKS tests using\n<prereq>\" change, but I needed to create an additional test for some\nsetup code. It's in a separate change as suggested by Jonathan Nieder\nfor ease of reviewing.\n\n    Date: Tue, 27 Jul 2010 16:17:37 -0500\n    From: Jonathan Nieder <jrnieder@gmail.com>\n    Message-ID: <20100727211737.GA11768@burratino>\n    In-Reply-To: <1280265254-19642-2-git-send-email-avarab@gmail.com>\n    Subject: Re: [PATCH 1/4] tests: implicitly skip SYMLINKS tests using <prereq>\n\n    Ævar Arnfjörð Bjarmason wrote:\n\n    > +++ b/t/t4004-diff-rename-symlink.sh\n    > @@ -40,8 +34,9 @@ test_expect_success \\\n    >  # rezrov and nitfol are rename/copy of frotz and bozbar should be\n    >  # a new creation.\n    >\n    > -GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current\n    > -cat >expected <<\\EOF\n    > +test_expect_success SYMLINKS 'setup diff output' \"\n    > +    GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current\n    > +    cat >expected <<\\EOF\n    >  diff --git a/bozbar b/bozbar\n    >  new file mode 120000\n    >  --- /dev/null\n\n    Probably belongs in a separate patch.  More importantly, it is missing\n    &&-chaining (not a regression, but it is best to set a good example).\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/t4004-diff-rename-symlink.sh |   18 +++++++-----------\n 1 files changed, 7 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh\nindex 1a09e8d..92a65f4 100755\n--- a/t/t4004-diff-rename-symlink.sh\n+++ b/t/t4004-diff-rename-symlink.sh\n@@ -12,13 +12,7 @@ by an edit for them.\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/diff-lib.sh\n \n-if ! test_have_prereq SYMLINKS\n-then\n-\tskip_all='Symbolic links not supported, skipping tests.'\n-\ttest_done\n-fi\n-\n-test_expect_success \\\n+test_expect_success SYMLINKS \\\n     'prepare reference tree' \\\n     'echo xyzzy | tr -d '\\\\\\\\'012 >yomin &&\n      ln -s xyzzy frotz &&\n@@ -26,7 +20,7 @@ test_expect_success \\\n     tree=$(git write-tree) &&\n     echo $tree'\n \n-test_expect_success \\\n+test_expect_success SYMLINKS \\\n     'prepare work tree' \\\n     'mv frotz rezrov &&\n      rm -f yomin &&\n@@ -40,8 +34,9 @@ test_expect_success \\\n # rezrov and nitfol are rename/copy of frotz and bozbar should be\n # a new creation.\n \n-GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current\n-cat >expected <<\\EOF\n+test_expect_success SYMLINKS 'setup diff output' \"\n+    GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current &&\n+    cat >expected <<\\EOF\n diff --git a/bozbar b/bozbar\n new file mode 120000\n --- /dev/null\n@@ -65,8 +60,9 @@ deleted file mode 100644\n -xyzzy\n \\ No newline at end of file\n EOF\n+\"\n \n-test_expect_success \\\n+test_expect_success SYMLINKS \\\n     'validate diff output' \\\n     'compare_diff_patch current expected'\n \n-- \n1.7.0.4\n"},{"id":"146610","messageId":"1280313299-721-4-git-send-email-avarab@gmail.com","threadId":"24553","inReplyTo":"1280313299-721-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 3/5] t/t5800-remote-helpers.sh: Skip with prereq on python <2.4","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-28T10:34:57Z","receivedAt":"2010-07-28T10:34:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the t/t5800-remote-helpers.sh test to skip with the the\nthree-arg prereq form of test_expect_success instead of bailing out.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/t5800-remote-helpers.sh |   24 +++++++++++-------------\n 1 files changed, 11 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t5800-remote-helpers.sh b/t/t5800-remote-helpers.sh\nindex 637d8e9..1fb6380 100755\n--- a/t/t5800-remote-helpers.sh\n+++ b/t/t5800-remote-helpers.sh\n@@ -13,13 +13,11 @@ if sys.hexversion < 0x02040000:\n     sys.exit(1)\n '\n then\n-\t:\n-else\n-\tskip_all='skipping git remote-testgit tests: requires Python 2.4 or newer'\n-\ttest_done\n+    # Requires Python 2.4 or newer\n+\ttest_set_prereq PYTHON_24\n fi\n \n-test_expect_success 'setup repository' '\n+test_expect_success PYTHON_24 'setup repository' '\n \tgit init --bare server/.git &&\n \tgit clone server public &&\n \t(cd public &&\n@@ -29,34 +27,34 @@ test_expect_success 'setup repository' '\n \t git push origin master)\n '\n \n-test_expect_success 'cloning from local repo' '\n+test_expect_success PYTHON_24 'cloning from local repo' '\n \tgit clone \"testgit::${PWD}/server\" localclone &&\n \ttest_cmp public/file localclone/file\n '\n \n-test_expect_success 'cloning from remote repo' '\n+test_expect_success PYTHON_24 'cloning from remote repo' '\n \tgit clone \"testgit::file://${PWD}/server\" clone &&\n \ttest_cmp public/file clone/file\n '\n \n-test_expect_success 'create new commit on remote' '\n+test_expect_success PYTHON_24 'create new commit on remote' '\n \t(cd public &&\n \t echo content >>file &&\n \t git commit -a -m two &&\n \t git push)\n '\n \n-test_expect_success 'pulling from local repo' '\n+test_expect_success PYTHON_24 'pulling from local repo' '\n \t(cd localclone && git pull) &&\n \ttest_cmp public/file localclone/file\n '\n \n-test_expect_success 'pulling from remote remote' '\n+test_expect_success PYTHON_24 'pulling from remote remote' '\n \t(cd clone && git pull) &&\n \ttest_cmp public/file clone/file\n '\n \n-test_expect_success 'pushing to local repo' '\n+test_expect_success PYTHON_24 'pushing to local repo' '\n \t(cd localclone &&\n \techo content >>file &&\n \tgit commit -a -m three &&\n@@ -65,12 +63,12 @@ test_expect_success 'pushing to local repo' '\n \ttest $HEAD = $(git --git-dir=server/.git rev-parse --verify HEAD)\n '\n \n-test_expect_success 'synch with changes from localclone' '\n+test_expect_success PYTHON_24 'synch with changes from localclone' '\n \t(cd clone &&\n \t git pull)\n '\n \n-test_expect_success 'pushing remote local repo' '\n+test_expect_success PYTHON_24 'pushing remote local repo' '\n \t(cd clone &&\n \techo content >>file &&\n \tgit commit -a -m four &&\n-- \n1.7.0.4\n"},{"id":"146611","messageId":"1280313299-721-5-git-send-email-avarab@gmail.com","threadId":"24553","inReplyTo":"1280313299-721-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 4/5] t/t7800-difftool.sh: Skip with prereq on no PERL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-28T10:34:58Z","receivedAt":"2010-07-28T10:34:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change t/t7800-difftool.sh to to skip with the the three-arg prereq\nform of test_expect_success instead of bailing out.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/t7800-difftool.sh |   49 ++++++++++++++++++++++---------------------------\n 1 files changed, 22 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 196827e..58dc6f6 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -10,11 +10,6 @@ Testing basic diff tool invocation\n \n . ./test-lib.sh\n \n-if ! test_have_prereq PERL; then\n-\tskip_all='skipping difftool tests, perl not available'\n-\ttest_done\n-fi\n-\n LF='\n '\n \n@@ -50,7 +45,7 @@ prompt_given()\n }\n \n # Create a file on master and change it on branch\n-test_expect_success 'setup' '\n+test_expect_success PERL 'setup' '\n \techo master >file &&\n \tgit add file &&\n \tgit commit -m \"added file\" &&\n@@ -62,7 +57,7 @@ test_expect_success 'setup' '\n '\n \n # Configure a custom difftool.<tool>.cmd and use it\n-test_expect_success 'custom commands' '\n+test_expect_success PERL 'custom commands' '\n \trestore_test_defaults &&\n \tgit config difftool.test-tool.cmd \"cat \\$REMOTE\" &&\n \n@@ -75,13 +70,13 @@ test_expect_success 'custom commands' '\n '\n \n # Ensures that git-difftool ignores bogus --tool values\n-test_expect_success 'difftool ignores bad --tool values' '\n+test_expect_success PERL 'difftool ignores bad --tool values' '\n \tdiff=$(git difftool --no-prompt --tool=bad-tool branch)\n \ttest \"$?\" = 1 &&\n \ttest \"$diff\" = \"\"\n '\n \n-test_expect_success 'difftool honors --gui' '\n+test_expect_success PERL 'difftool honors --gui' '\n \tgit config merge.tool bogus-tool &&\n \tgit config diff.tool bogus-tool &&\n \tgit config diff.guitool test-tool &&\n@@ -92,7 +87,7 @@ test_expect_success 'difftool honors --gui' '\n \trestore_test_defaults\n '\n \n-test_expect_success 'difftool --gui works without configured diff.guitool' '\n+test_expect_success PERL 'difftool --gui works without configured diff.guitool' '\n \tgit config diff.tool test-tool &&\n \n \tdiff=$(git difftool --no-prompt --gui branch) &&\n@@ -102,7 +97,7 @@ test_expect_success 'difftool --gui works without configured diff.guitool' '\n '\n \n # Specify the diff tool using $GIT_DIFF_TOOL\n-test_expect_success 'GIT_DIFF_TOOL variable' '\n+test_expect_success PERL 'GIT_DIFF_TOOL variable' '\n \tgit config --unset diff.tool\n \tGIT_DIFF_TOOL=test-tool &&\n \texport GIT_DIFF_TOOL &&\n@@ -115,7 +110,7 @@ test_expect_success 'GIT_DIFF_TOOL variable' '\n \n # Test the $GIT_*_TOOL variables and ensure\n # that $GIT_DIFF_TOOL always wins unless --tool is specified\n-test_expect_success 'GIT_DIFF_TOOL overrides' '\n+test_expect_success PERL 'GIT_DIFF_TOOL overrides' '\n \tgit config diff.tool bogus-tool &&\n \tgit config merge.tool bogus-tool &&\n \n@@ -136,7 +131,7 @@ test_expect_success 'GIT_DIFF_TOOL overrides' '\n \n # Test that we don't have to pass --no-prompt to difftool\n # when $GIT_DIFFTOOL_NO_PROMPT is true\n-test_expect_success 'GIT_DIFFTOOL_NO_PROMPT variable' '\n+test_expect_success PERL 'GIT_DIFFTOOL_NO_PROMPT variable' '\n \tGIT_DIFFTOOL_NO_PROMPT=true &&\n \texport GIT_DIFFTOOL_NO_PROMPT &&\n \n@@ -148,7 +143,7 @@ test_expect_success 'GIT_DIFFTOOL_NO_PROMPT variable' '\n \n # git-difftool supports the difftool.prompt variable.\n # Test that GIT_DIFFTOOL_PROMPT can override difftool.prompt = false\n-test_expect_success 'GIT_DIFFTOOL_PROMPT variable' '\n+test_expect_success PERL 'GIT_DIFFTOOL_PROMPT variable' '\n \tgit config difftool.prompt false &&\n \tGIT_DIFFTOOL_PROMPT=true &&\n \texport GIT_DIFFTOOL_PROMPT &&\n@@ -160,7 +155,7 @@ test_expect_success 'GIT_DIFFTOOL_PROMPT variable' '\n '\n \n # Test that we don't have to pass --no-prompt when difftool.prompt is false\n-test_expect_success 'difftool.prompt config variable is false' '\n+test_expect_success PERL 'difftool.prompt config variable is false' '\n \tgit config difftool.prompt false &&\n \n \tdiff=$(git difftool branch) &&\n@@ -170,7 +165,7 @@ test_expect_success 'difftool.prompt config variable is false' '\n '\n \n # Test that we don't have to pass --no-prompt when mergetool.prompt is false\n-test_expect_success 'difftool merge.prompt = false' '\n+test_expect_success PERL 'difftool merge.prompt = false' '\n \tgit config --unset difftool.prompt\n \tgit config mergetool.prompt false &&\n \n@@ -181,7 +176,7 @@ test_expect_success 'difftool merge.prompt = false' '\n '\n \n # Test that the -y flag can override difftool.prompt = true\n-test_expect_success 'difftool.prompt can overridden with -y' '\n+test_expect_success PERL 'difftool.prompt can overridden with -y' '\n \tgit config difftool.prompt true &&\n \n \tdiff=$(git difftool -y branch) &&\n@@ -191,7 +186,7 @@ test_expect_success 'difftool.prompt can overridden with -y' '\n '\n \n # Test that the --prompt flag can override difftool.prompt = false\n-test_expect_success 'difftool.prompt can overridden with --prompt' '\n+test_expect_success PERL 'difftool.prompt can overridden with --prompt' '\n \tgit config difftool.prompt false &&\n \n \tprompt=$(echo | git difftool --prompt branch | tail -1) &&\n@@ -201,7 +196,7 @@ test_expect_success 'difftool.prompt can overridden with --prompt' '\n '\n \n # Test that the last flag passed on the command-line wins\n-test_expect_success 'difftool last flag wins' '\n+test_expect_success PERL 'difftool last flag wins' '\n \tdiff=$(git difftool --prompt --no-prompt branch) &&\n \ttest \"$diff\" = \"branch\" &&\n \n@@ -215,7 +210,7 @@ test_expect_success 'difftool last flag wins' '\n \n # git-difftool falls back to git-mergetool config variables\n # so test that behavior here\n-test_expect_success 'difftool + mergetool config variables' '\n+test_expect_success PERL 'difftool + mergetool config variables' '\n \tremove_config_vars\n \tgit config merge.tool test-tool &&\n \tgit config mergetool.test-tool.cmd \"cat \\$LOCAL\" &&\n@@ -233,7 +228,7 @@ test_expect_success 'difftool + mergetool config variables' '\n \trestore_test_defaults\n '\n \n-test_expect_success 'difftool.<tool>.path' '\n+test_expect_success PERL 'difftool.<tool>.path' '\n \tgit config difftool.tkdiff.path echo &&\n \tdiff=$(git difftool --tool=tkdiff --no-prompt branch) &&\n \tgit config --unset difftool.tkdiff.path &&\n@@ -243,32 +238,32 @@ test_expect_success 'difftool.<tool>.path' '\n \trestore_test_defaults\n '\n \n-test_expect_success 'difftool --extcmd=cat' '\n+test_expect_success PERL 'difftool --extcmd=cat' '\n \tdiff=$(git difftool --no-prompt --extcmd=cat branch) &&\n \ttest \"$diff\" = branch\"$LF\"master\n '\n \n-test_expect_success 'difftool --extcmd cat' '\n+test_expect_success PERL 'difftool --extcmd cat' '\n \tdiff=$(git difftool --no-prompt --extcmd cat branch) &&\n \ttest \"$diff\" = branch\"$LF\"master\n '\n \n-test_expect_success 'difftool -x cat' '\n+test_expect_success PERL 'difftool -x cat' '\n \tdiff=$(git difftool --no-prompt -x cat branch) &&\n \ttest \"$diff\" = branch\"$LF\"master\n '\n \n-test_expect_success 'difftool --extcmd echo arg1' '\n+test_expect_success PERL 'difftool --extcmd echo arg1' '\n \tdiff=$(git difftool --no-prompt --extcmd sh\\ -c\\ \\\"echo\\ \\$1\\\" branch)\n \ttest \"$diff\" = file\n '\n \n-test_expect_success 'difftool --extcmd cat arg1' '\n+test_expect_success PERL 'difftool --extcmd cat arg1' '\n \tdiff=$(git difftool --no-prompt --extcmd sh\\ -c\\ \\\"cat\\ \\$1\\\" branch)\n \ttest \"$diff\" = master\n '\n \n-test_expect_success 'difftool --extcmd cat arg2' '\n+test_expect_success PERL 'difftool --extcmd cat arg2' '\n \tdiff=$(git difftool --no-prompt --extcmd sh\\ -c\\ \\\"cat\\ \\$2\\\" branch)\n \ttest \"$diff\" = branch\n '\n-- \n1.7.0.4\n"},{"id":"146609","messageId":"1280313299-721-6-git-send-email-avarab@gmail.com","threadId":"24553","inReplyTo":"1280313299-721-1-git-send-email-avarab@gmail.com","subject":"[PATCH v2 5/5] t/README: Update \"Skipping tests\" to align with best practices","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-28T10:34:59Z","receivedAt":"2010-07-28T10:34:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The example I initially added to \"Skipping tests\" wasn't very\ngood. We'd rather skip tests using the three-arg prereq form to the\ntest_* functions, not bail out with a skip message.\n\nChange the documentation to reflect that, but retain the bailout\nexample under a disclaimer which explains that it's probably not a\ngood idea to use it.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/README |   21 ++++++++++++++++++---\n 1 files changed, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 0d1183c..12b714b 100644\n--- a/t/README\n+++ b/t/README\n@@ -307,9 +307,21 @@ Keep in mind:\n Skipping tests\n --------------\n \n-If you need to skip all the remaining tests you should set skip_all\n-and immediately call test_done. The string you give to skip_all will\n-be used as an explanation for why the test was skipped. for instance:\n+If you need to skip tests you should do so be using the three-arg form\n+of the test_* functions (see the \"Test harness library\" section\n+below), e.g.:\n+\n+    test_expect_success PERL 'I need Perl' \"\n+        '$PERL_PATH' -e 'hlagh() if unf_unf()'\n+    \"\n+\n+The advantage of skipping tests like this is that platforms that don't\n+have the PERL and other optional dependencies get an indication of how\n+many tests they're missing.\n+\n+If the test code is too hairy for that (i.e. does a lot of setup work\n+outside test assertions) you can also skip all remaining tests by\n+setting skip_all and immediately call test_done:\n \n \tif ! test_have_prereq PERL\n \tthen\n@@ -317,6 +329,9 @@ be used as an explanation for why the test was skipped. for instance:\n \t    test_done\n \tfi\n \n+The string you give to skip_all will be used as an explanation for why\n+the test was skipped.\n+\n End with test_done\n ------------------\n \n-- \n1.7.0.4\n"},{"id":"146672","messageId":"20100729010005.GE29156@dert.cs.uchicago.edu","threadId":"24553","inReplyTo":"1280313299-721-2-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2 1/5] tests: implicitly skip SYMLINKS tests using <prereq>","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-29T01:00:05Z","receivedAt":"2010-07-29T01:00:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n>  t/t2007-checkout-symlink.sh       |   14 ++++----------\n>  t/t4011-diff-symlink.sh           |   18 ++++++------------\n>  t/t4023-diff-rename-typechange.sh |   14 ++++----------\n>  t/t4114-apply-typechange.sh       |   28 +++++++++++-----------------\n>  t/t4115-apply-symlink.sh          |   12 +++---------\n>  t/t4122-apply-symlink-inside.sh   |   12 +++---------\n>  t/t5522-pull-symlink.sh           |   14 ++++----------\n>  t/t6035-merge-dir-to-symlink.sh   |   20 +++++++-------------\n>  8 files changed, 42 insertions(+), 90 deletions(-)\n\nI like it. :)\n"},{"id":"146674","messageId":"20100729010903.GF29156@dert.cs.uchicago.edu","threadId":"24553","inReplyTo":"1280313299-721-3-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2 2/5] t/t4004-diff-rename-symlink.sh: use three-arg <prereq>","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-29T01:09:03Z","receivedAt":"2010-07-29T01:09:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> Change the tests that skipped due to unavailable SYMLINKS support to\n> use the three-arg prereq form of test_expect_success.\n> \n> This is like the \"tests: implicitly skip SYMLINKS tests using\n> <prereq>\" change, but I needed to create an additional test for some\n> setup code. It's in a separate change as suggested by Jonathan Nieder\n> for ease of reviewing.\n\nHmm, I still don’t understand this.  Do you mean that there is\nsome setup that needs to be run before these commands, and so\nthe patch fails if that change is not included?\n\nOr is it a matter of \"while at it, fix this other problem\nI noticed\" (which would be fine, but it is clearer to\npresent it as such if so)?\n\n> diff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh\n> index 1a09e8d..92a65f4 100755\n> --- a/t/t4004-diff-rename-symlink.sh\n> +++ b/t/t4004-diff-rename-symlink.sh\n> @@ -40,8 +34,9 @@ test_expect_success \\\n>  # rezrov and nitfol are rename/copy of frotz and bozbar should be\n>  # a new creation.\n>  \n> -GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current\n> -cat >expected <<\\EOF\n> +test_expect_success SYMLINKS 'setup diff output' \"\n> +    GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current &&\n> +    cat >expected <<\\EOF\n>  diff --git a/bozbar b/bozbar\n>  new file mode 120000\n>  --- /dev/null\n> @@ -65,8 +60,9 @@ deleted file mode 100644\n>  -xyzzy\n>  \\ No newline at end of file\n>  EOF\n> +\"\n\nTip for the future: if you use 'straight quotes', then readers\ncan avoid carefully scanning through for $ and similar oddities\n(and the test script presented with the \"expecting success:\"\nprompt will use the friendlier $tree instead of 76c98ds89).\n\nThe patch looks good; my only remaining concern is the log\nmessage as mentioned above.\n\nThanks,\nJonathan\n"},{"id":"146675","messageId":"20100729011602.GG29156@dert.cs.uchicago.edu","threadId":"24553","inReplyTo":"1280313299-721-4-git-send-email-avarab@gmail.com","subject":"Re: [PATCH v2 3/5] t/t5800-remote-helpers.sh: Skip with prereq on python <2.4","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-07-29T01:16:02Z","receivedAt":"2010-07-29T01:16:02Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n\n> Change the t/t5800-remote-helpers.sh test to skip with the the\n> three-arg prereq form of test_expect_success instead of bailing out.\n\nRight:\n\n> +++ b/t/t5800-remote-helpers.sh\n> @@ -13,13 +13,11 @@ if sys.hexversion < 0x02040000:\n>      sys.exit(1)\n>  '\n>  then\n> -\t:\n> -else\n> -\tskip_all='skipping git remote-testgit tests: requires Python 2.4 or newer'\n> -\ttest_done\n> +    # Requires Python 2.4 or newer\n> +\ttest_set_prereq PYTHON_24\n>  fi\n\nAnd as a bonus, this makes the test clearer.  Nice.\n\nI would suggest dropping the \"Requires Python 2.4\" comment, or\nmaybe moving it somewhere else with a change to explain that\nit is git-remote-testgit that requires Python ≥ 2.4\n(rather than the remote helper facility in general).\n\nThe rest of the series looks good to me.\n\nThanks again,\nJonathan\n"},{"id":"146680","messageId":"AANLkTinFMjH6LfDXjCp9XGVEuQF9ZKrL0i8t-p8hmzJ-@mail.gmail.com","threadId":"24553","inReplyTo":"20100729010903.GF29156@dert.cs.uchicago.edu","subject":"Re: [PATCH v2 2/5] t/t4004-diff-rename-symlink.sh: use three-arg <prereq>","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-29T01:56:50Z","receivedAt":"2010-07-29T01:56:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Jul 29, 2010 at 01:09, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Ævar Arnfjörð Bjarmason wrote:\n>\n>> Change the tests that skipped due to unavailable SYMLINKS support to\n>> use the three-arg prereq form of test_expect_success.\n>>\n>> This is like the \"tests: implicitly skip SYMLINKS tests using\n>> <prereq>\" change, but I needed to create an additional test for some\n>> setup code. It's in a separate change as suggested by Jonathan Nieder\n>> for ease of reviewing.\n>\n> Hmm, I still don’t understand this.  Do you mean that there is\n> some setup that needs to be run before these commands, and so\n> the patch fails if that change is not included?\n>\n> Or is it a matter of \"while at it, fix this other problem\n> I noticed\" (which would be fine, but it is clearer to\n> present it as such if so)?\n\nThe setup code needs to be inside a test so that it'll only run if we\nhave SYMLINKS support.\n\nI could also have done:\n\n    if test_have_prereq PERL\n    then\n        ..setup code..\n    fi\n\nBut setup code should be inside tests, so that we'll get failure\nreporting.\n\n>> diff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh\n>> index 1a09e8d..92a65f4 100755\n>> --- a/t/t4004-diff-rename-symlink.sh\n>> +++ b/t/t4004-diff-rename-symlink.sh\n>> @@ -40,8 +34,9 @@ test_expect_success \\\n>>  # rezrov and nitfol are rename/copy of frotz and bozbar should be\n>>  # a new creation.\n>>\n>> -GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current\n>> -cat >expected <<\\EOF\n>> +test_expect_success SYMLINKS 'setup diff output' \"\n>> +    GIT_DIFF_OPTS=--unified=0 git diff-index -M -p $tree >current &&\n>> +    cat >expected <<\\EOF\n>>  diff --git a/bozbar b/bozbar\n>>  new file mode 120000\n>>  --- /dev/null\n>> @@ -65,8 +60,9 @@ deleted file mode 100644\n>>  -xyzzy\n>>  \\ No newline at end of file\n>>  EOF\n>> +\"\n>\n> Tip for the future: if you use 'straight quotes', then readers\n> can avoid carefully scanning through for $ and similar oddities\n> (and the test script presented with the \"expecting success:\"\n> prompt will use the friendlier $tree instead of 76c98ds89).\n\nAh, thanks.\n\n> The patch looks good; my only remaining concern is the log\n> message as mentioned above.\n\nHopefully that's cleared up now.\n"},{"id":"146682","messageId":"AANLkTimZ5GFMn3p3UxgWrY1sMpguA4PZ0F=oGtT02JJQ@mail.gmail.com","threadId":"24553","inReplyTo":"20100729011602.GG29156@dert.cs.uchicago.edu","subject":"Re: [PATCH v2 3/5] t/t5800-remote-helpers.sh: Skip with prereq on python <2.4","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-29T02:03:32Z","receivedAt":"2010-07-29T02:03:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Jul 29, 2010 at 01:16, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Ævar Arnfjörð Bjarmason wrote:\n>\n>> Change the t/t5800-remote-helpers.sh test to skip with the the\n>> three-arg prereq form of test_expect_success instead of bailing out.\n>\n> Right:\n>\n>> +++ b/t/t5800-remote-helpers.sh\n>> @@ -13,13 +13,11 @@ if sys.hexversion < 0x02040000:\n>>      sys.exit(1)\n>>  '\n>>  then\n>> -     :\n>> -else\n>> -     skip_all='skipping git remote-testgit tests: requires Python 2.4 or newer'\n>> -     test_done\n>> +    # Requires Python 2.4 or newer\n>> +     test_set_prereq PYTHON_24\n>>  fi\n>\n> And as a bonus, this makes the test clearer.  Nice.\n\nYup, <prereq> is really nice in all regards.\n\n> I would suggest dropping the \"Requires Python 2.4\" comment, or\n> maybe moving it somewhere else with a change to explain that\n> it is git-remote-testgit that requires Python ≥ 2.4\n> (rather than the remote helper facility in general).\n\nI put it there to make the connection between 24, 0x02040000, and 2.4.\n\nI think trying to avoid confusion like that isn't a problem in a test\nfile, it's pretty obvious from the context that the prereq only has to\ndo with the test. Someone looking up remote helpers is going to do\nthat in the documentation, not in some t/ test file.\n\nThanks for your continuing & detailed reviews b.t.w.\n"}]}