Re: [RFC/PATCH v2] t3300-*.sh: Fix a TAP parse error

2012-07-25 Thread Ramsay Jones
Jonathan Nieder wrote:
 This version of the patch only moves code to determine the test
 prerequisite to the outer level, while leaving the 'setup' aspects
 of the first test in place.
 
 I guess I don't see the point.

You don't see the point of fixing the TAP Parse error? :-D

The current convention of don't do
 anything complicated outside test assertions is easy to explain.
 What new convention are you suggesting to replace it?

Hmm, well I guess I'm not going to suggest anything!

However, what is anything complicated?

At the end of test-lib.sh we find:

# test whether the filesystem supports symbolic links
ln -s x y 2/dev/null  test -h y 2/dev/null  test_set_prereq SYMLINKS
rm -f y

# When the tests are run as root, permission tests will report that
# things are writable when they shouldn't be.
test -w / || test_set_prereq SANITY

Is this too complicated? If not, why not? If yes, should it be added to
a test assertion?

Would it be acceptable for me to add some code, here at the end of
test-lib.sh, to set the TABS_IN_FILENAME test prerequisite and use it
in tests t3300-funny-names.sh, t3902-quoted.sh, t3600-rm.sh,
t4016-diff-quote.sh and t4135-apply-weird-filenames.sh?

How about some of the test library files:

diff-lib.sh  lib-git-p4.shlib-read-tree.sh
gitweb-lib.shlib-git-svn.sh   lib-rebase.sh
lib-bash.sh  lib-gpg.sh*  lib-t6000.sh
lib-credential.sh*   lib-httpd.sh lib-terminal.sh
lib-cvs.sh   lib-pager.sh test-lib-functions.sh
lib-diff-alternative.sh  lib-patch-mode.shtest-lib.sh
lib-gettext.sh   lib-prereq-FILEMODE.sh
lib-git-daemon.shlib-read-tree-m-3way.sh

Several of these files contain executable code (rather than just a
library of functions). For example, look at lib-cvs.sh, lib-httpd.sh,
and lib-prereq-FILEMODE.sh. Is this code too complicated?

Would it be acceptable for me to create an lib-prereq-TABSINFILE.sh
file so that I could source it only in the test files that require
the TABS_IN_FILENAME prerequisite?

ATB,
Ramsay Jones

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH v2] t3300-*.sh: Fix a TAP parse error

2012-07-24 Thread Jonathan Nieder
Hi,

Ramsay Jones wrote:

 Hi Jonathan,

 This version of the patch only moves code to determine the test
 prerequisite to the outer level, while leaving the 'setup' aspects
 of the first test in place.

I guess I don't see the point.  The current convention of don't do
anything complicated outside test assertions is easy to explain.
What new convention are you suggesting to replace it?

Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC/PATCH v2] t3300-*.sh: Fix a TAP parse error

2012-07-24 Thread Ramsay Jones

At present, running the t3300-*.sh test on cygwin looks like:

$ cd t
$ ./t3300-funny-names.sh
ok 1 - setup
# passed all 1 test(s)
1..1 # SKIP Your filesystem does not allow tabs in filenames
$

Unfortunately, this is not valid TAP output, which prove notes
as follows:

$ prove --exec sh t3300-funny-names.sh
t3300-funny-names.sh .. All 1 subtests passed

Test Summary Report
---
t3300-funny-names.sh (Wstat: 0 Tests: 1 Failed: 0)
  Parse errors: No plan found in TAP output
Files=1, Tests=1,  2 wallclock secs ( 0.05 usr  0.00 sys +  \
0.90 cusr  0.49 csys =  1.43 CPU)
Result: FAIL
$

This is due to the 'trailing_plan' having a 'skip_directive'
attached to it. This is not allowed by the TAP grammar, which
only allows a 'leading_plan' to be followed by an optional
'skip_directive'. (see perldoc TAP::Parser::Grammar).

A trailing_plan is one that appears in the TAP output after one or
more test status lines (that start 'not '? 'ok ' ...), whereas a
leading_plan must appear before all test status lines (if any).

In practice, this means that the test script cannot contain a use
of the 'skip all' facility:

skip_all='Some reason to skip *all* tests in this file'
test_done

after having already executed one or more tests with (for example)
'test_expect_success'. Unfortunately, this is exactly what this
test script is doing. The first 'setup' test is actually used to
determine if the test prerequisite is satisfied by the filesystem
(ie does it allow tabs in filenames?).

In order to fix the parse errors, place the code to determine the
test prerequisite at the top level of the script, prior to the
first test, rather than as a parameter to test_expect_success.
This allows us to correctly use 'skip_all', thus:

$ ./t3300-funny-names.sh
# passed all 0 test(s)
1..0 # SKIP Your filesystem does not allow tabs in filenames
$

$ prove --exec sh t3300-funny-names.sh
t3300-funny-names.sh .. skipped: Your filesystem does not \
allow tabs in filenames
Files=1, Tests=0,  2 wallclock secs ( 0.02 usr  0.03 sys +  \
0.84 cusr  0.41 csys =  1.29 CPU)
Result: NOTESTS
$

Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---

Hi Jonathan,

This version of the patch only moves code to determine the test
prerequisite to the outer level, while leaving the 'setup' aspects
of the first test in place.

I did think about merging the following test (setup: populate index
and tree) into the first test, but I wanted to keep it simple for now.

What do you think?

ATB,
Ramsay Jones

 t/t3300-funny-names.sh | 24 +++-
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh
index 1f35e55..7480d6e 100755
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -11,6 +11,16 @@ tree, index, and tree objects.
 
 . ./test-lib.sh
 
+HT='   '
+
+echo 2/dev/null  Name with an${HT}HT
+if ! test -f Name with an${HT}HT
+then
+   # since FAT/NTFS does not allow tabs in filenames, skip this test
+   skip_all='Your filesystem does not allow tabs in filenames'
+   test_done
+fi
+
 p0='no-funny'
 p1='tabs   , (dq) and spaces'
 p2='just space'
@@ -23,21 +33,9 @@ test_expect_success 'setup' '
EOF
 
{ cat $p0 $p1 || :; } 
-   { echo Foo Bar Baz $p2 || :; } 
-
-   if test -f $p1  cmp $p0 $p1
-   then
-   test_set_prereq TABS_IN_FILENAMES
-   fi
+   { echo Foo Bar Baz $p2 || :; }
 '
 
-if ! test_have_prereq TABS_IN_FILENAMES
-then
-   # since FAT/NTFS does not allow tabs in filenames, skip this test
-   skip_all='Your filesystem does not allow tabs in filenames'
-   test_done
-fi
-
 test_expect_success 'setup: populate index and tree' '
git update-index --add $p0 $p2 
t0=$(git write-tree)
-- 
1.7.11.2

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html