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

2012-08-19 Thread Ramsay Jones
Junio C Hamano wrote:
> Jonathan Nieder  writes:
>> Ramsay Jones wrote:
>>> Junio C Hamano wrote:
>>
 Observing that all well-written test scripts we have begin with this
 boilerplate line:

test_expect_success setup '

 I wouldn't mind introducing a new helper function test_setup that
 behaves like test_expect_success but is meant to be used in the
 first "set-up" phase of the tests in a test script.
>>
>> Neat.  This could be used for later set-up tests, too, perhaps with a
>> long-term goal of making non set-up tests independent of each other
>> (reorderable and skippable).
>>
>> [...]
>>> [1] For example, what should/will happen if someone uses test_must_fail,
>>> test_might_fail, etc., within the test_fixture script? Should they simply
>>> be banned within a text_fixture?
>>
>> Why wouldn't they act just like they do in test_expect_success blocks?
>>
>> FWIW I find Junio's test_setup name more self-explanatory.  What
>> mnemonic should I be using to remember the _fixture name?
> 
> I see that I was distracted by the "where does the fixture come
> from" and did not follow through.
> 
> I think what it does makes sense (I haven't checked all the
> redirections, though).  Do we want to resurrect the topic?  It needs
> a paragraph in the proposed commit log and t/README to explain the
> motivation and the usage.

Yes, this is on my TODO list.

I will name the function 'test_setup' rather than 'test_fixture'.

Also, the test_fixture function had a single script parameter, since
I didn't see the point of having a "title" like test_expect_success.
However, I'm now in two minds about this; if it were to take a title
it may be useful to include the title in the error message, if the
test contained multiple calls to test_setup. I'm still inclined to
*not* include a title parameter ...

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] t3300-*.sh: Fix a TAP parse error

2012-08-16 Thread Junio C Hamano
Jonathan Nieder  writes:

> Ramsay Jones wrote:
>> Junio C Hamano wrote:
>
>>> Observing that all well-written test scripts we have begin with this
>>> boilerplate line:
>>>
>>> test_expect_success setup '
>>>
>>> I wouldn't mind introducing a new helper function test_setup that
>>> behaves like test_expect_success but is meant to be used in the
>>> first "set-up" phase of the tests in a test script.
>
> Neat.  This could be used for later set-up tests, too, perhaps with a
> long-term goal of making non set-up tests independent of each other
> (reorderable and skippable).
>
> [...]
>> [1] For example, what should/will happen if someone uses test_must_fail,
>> test_might_fail, etc., within the test_fixture script? Should they simply
>> be banned within a text_fixture?
>
> Why wouldn't they act just like they do in test_expect_success blocks?
>
> FWIW I find Junio's test_setup name more self-explanatory.  What
> mnemonic should I be using to remember the _fixture name?

I see that I was distracted by the "where does the fixture come
from" and did not follow through.

I think what it does makes sense (I haven't checked all the
redirections, though).  Do we want to resurrect the topic?  It needs
a paragraph in the proposed commit log and t/README to explain the
motivation and the usage.

Thanks.
--
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] t3300-*.sh: Fix a TAP parse error

2012-07-28 Thread Ramsay Jones
Junio C Hamano wrote:
> Jonathan Nieder  writes:
> 
>> FWIW I find Junio's test_setup name more self-explanatory.  What
>> mnemonic should I be using to remember the _fixture name?
> 
> Previous exposure to things like Rails?

I did once have a brief look at ruby, but my "new language to learn"
list was over-subscribed. (I think I went with D) So, I'm not at all
familiar with Rails.

A test-fixture may be used in various xUnit unit-test libraries.
(eg JUnit, cppUnit, etc)

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] t3300-*.sh: Fix a TAP parse error

2012-07-28 Thread Ramsay Jones
Jonathan Nieder wrote:
> [...]
>> [1] For example, what should/will happen if someone uses test_must_fail,
>> test_might_fail, etc., within the test_fixture script? Should they simply
>> be banned within a text_fixture?
> 
> Why wouldn't they act just like they do in test_expect_success blocks?

Heh, well they do indeed act just like they do in text_expect_success blocks!

I spent only about 20 minutes writing test_fixture, playing with it, and then
deciding to shelve it for now. Again, I wanted a *quick* fix for the TAP
parse error, so that it would make it into v1.7.12. :(

Having now spent a further 30 minutes, I can see that I did a better job than
I thought! :-P

Actually, scratch that; rather I should say that Junio and the other authors
of the test infrastructure did such a good job (particularly with separation
of concerns), that I lucked into a good implementation.

I still haven't done any serious testing, so if I subsequently find any
problems, then the lousy implementation is my fault! ;-)

> FWIW I find Junio's test_setup name more self-explanatory.  What
> mnemonic should I be using to remember the _fixture name?

I don't have a problem with 'test_setup' either; test-fixture comes from the
various xUnit unit-test libraries. (I think Kent Beck et.al. wrote JUnit first
and then it was ported to various other languages. eg cppUnit for C++).

Briefly, a test-fixture provides a context or common environment, via code for
test setup and teardown, in which to run one or more tests.

HTH

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] t3300-*.sh: Fix a TAP parse error

2012-07-25 Thread Junio C Hamano
Jonathan Nieder  writes:

> FWIW I find Junio's test_setup name more self-explanatory.  What
> mnemonic should I be using to remember the _fixture name?

Previous exposure to things like Rails?
--
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] t3300-*.sh: Fix a TAP parse error

2012-07-25 Thread Jonathan Nieder
Hi,

Ramsay Jones wrote:
> Junio C Hamano wrote:

>> Observing that all well-written test scripts we have begin with this
>> boilerplate line:
>>
>>  test_expect_success setup '
>>
>> I wouldn't mind introducing a new helper function test_setup that
>> behaves like test_expect_success but is meant to be used in the
>> first "set-up" phase of the tests in a test script.

Neat.  This could be used for later set-up tests, too, perhaps with a
long-term goal of making non set-up tests independent of each other
(reorderable and skippable).

[...]
> [1] For example, what should/will happen if someone uses test_must_fail,
> test_might_fail, etc., within the test_fixture script? Should they simply
> be banned within a text_fixture?

Why wouldn't they act just like they do in test_expect_success blocks?

FWIW I find Junio's test_setup name more self-explanatory.  What
mnemonic should I be using to remember the _fixture name?

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


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

2012-07-25 Thread Ramsay Jones
Junio C Hamano wrote:
[...]
> As I am more worried about longer-term health of the codebase, what
> the part you moved outside test_expect_* with this patch happens to
> do _right now_ is of secondary importance, at least from my point of
> view.  The next patch that updates this scirpt may want to run more
> involved commands that can fail in different ways.
> 
> Being able to rely on the protection test_expect_* gives us in the
> set-up phase of the test has been very valuable (in addition to
> making the result more readable) to us.  Can we keep that property
> in some way while also keeping the ability to skip the remainder of
> the test script?
> 
> Observing that all well-written test scripts we have begin with this
> boilerplate line:
> 
>   test_expect_success setup '
> 
> I wouldn't mind introducing a new helper function test_setup that
> behaves like test_expect_success but is meant to be used in the
> first "set-up" phase of the tests in a test script. Perhaps we can
> give its failure a meaning different than failures in other normal
> tests (e.g. "even set-up fails, and the remainder of the test is N/A
> here, hence abort the whole thing"), and do not count "test_setup"
> as part of the test (i.e. do not increment $test_count and friends).

Heh, I did exactly this (except mine was called test_fixture) as part
of my first (abandoned) attempt to address this problem. :-D

I've attached the patch below, just for discussion.

I didn't test it very much, but it seems to work with a superficial
workout:

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

$ ./t3300-funny-names.sh -v
Initialized empty Git repository in /home/ramsay/git/t/trash 
directory.t3300-fun
ny-names/.git/
test fixture:
cat >"$p0" <<-\EOF &&
1. A quick brown fox jumps over the lazy cat, oops dog.
2. A quick brown fox jumps over the lazy cat, oops dog.
3. A quick brown fox jumps over the lazy cat, oops dog.
EOF

{ cat "$p0" >"$p1" || :; } &&
{ echo "Foo Bar Baz" >"$p2" || :; } &&

if test -f "$p1" && cmp "$p0" "$p1"
then
test_set_prereq TABS_IN_FILENAMES
fi

./test-lib.sh: line 274: tabs   ," (dq) and spaces: No such file or 
directory

# 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 
filename
s
Files=1, Tests=0,  1 wallclock secs ( 0.03 usr  0.05 sys +  0.87 cusr  0.41 
csys
 =  1.36 CPU)
Result: NOTESTS
$

So why did I abandon this patch? Well, I didn't really; I just decided
that I needed *much* more time to polish[1] 'test_fixture'. I wanted to
fix the "TAP parse error" problem before v1.7.12-rc0! :(

HTH

ATB,
Ramsay Jones

[1] For example, what should/will happen if someone uses test_must_fail,
test_might_fail, etc., within the test_fixture script? Should they simply
be banned within a text_fixture?

--- >8 ---
Subject: [PATCH] test-lib-functions.sh: Add a test_fixture function


Signed-off-by: Ramsay Jones 
---
 t/t3300-funny-names.sh  |  2 +-
 t/test-lib-functions.sh | 11 +++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh
index 1f35e55..59331a0 100755
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -15,7 +15,7 @@ p0='no-funny'
 p1='tabs   ," (dq) and spaces'
 p2='just space'
 
-test_expect_success 'setup' '
+test_fixture '
cat >"$p0" <<-\EOF &&
1. A quick brown fox jumps over the lazy cat, oops dog.
2. A quick brown fox jumps over the lazy cat, oops dog.
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 80daaca..b70c963 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -302,6 +302,17 @@ test_expect_success () {
echo >&3 ""
 }
 
+test_fixture () {
+   test "$#" = 1 ||
+   error "bug in test script: must be single argument to test_fixture"
+   say >&3 "test fixture: $1"
+   if ! test_run_ "$1"
+   then
+   error "failure in test_fixture code"
+   fi
+   echo >&3 ""
+}
+
 # test_external runs external test scripts that provide continuous
 # test output about their progress, and succeeds/fails on
 # zero/non-zero exit code.  It outputs the test output on stdout even
-- 
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


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

2012-07-25 Thread Ramsay Jones
Jonathan Nieder wrote:
> [...]
>> No, I don't think this would be a good direction to go in. This may
>> not be a good idea either, but if you wanted to add a check here, then
>> maybe something like this (totally untested):
>>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index acda33d..53a2422 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -354,6 +354,9 @@ test_done () {
>>  case "$test_failure" in
>>  0)
>>  # Maybe print SKIP message
>> +if test -n "$skip_all" && test $test_count -gt 0; then
>> +error "Can't use skip_all after running some tests"
>> +fi
>>  [ -z "$skip_all" ] || skip_all=" # SKIP $skip_all"
>>  
>>  if test $test_external_has_tap -eq 0; then
> 
> I think preventing invalid TAP output like this would be a very good
> thing!  As a start, this patchlet looks good to me, and then I guess
> we'll have to think more about what happens when a person wants to
> skip_all_remaining after a test has already been run.
> 
> Care to format it as a "git am"-able patch, or should I?

Yes, I will happily create a proper (tested) patch and send it to the list.

However, given that we are now in the RC period, I probably won't get to it
immediately; I need to set aside *at least* one full evening to running the
testsuite on cygwin! ;-)

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] t3300-*.sh: Fix a TAP parse error

2012-07-24 Thread Junio C Hamano
Ramsay Jones  writes:

> The only problematic platforms I test on are "NTFS/bash" on cygwin and MinGW.
> Since commit 2b843732 ("Suppress some bash redirection error messages",
> 26-08-2008), I have not noticed any complaints regarding this problem.
> What have I missed?
>
> Assuming we are not talking about errors like ENOSPC, EROFS etc., then the
> only command which would issue a complaint to stderr would be the line
> following the above snippet, thus:
>
> +cat 2>/dev/null >"$p1" "$p0"
>
> (note the stderr redirection).

As I am more worried about longer-term health of the codebase, what
the part you moved outside test_expect_* with this patch happens to
do _right now_ is of secondary importance, at least from my point of
view.  The next patch that updates this scirpt may want to run more
involved commands that can fail in different ways.

Being able to rely on the protection test_expect_* gives us in the
set-up phase of the test has been very valuable (in addition to
making the result more readable) to us.  Can we keep that property
in some way while also keeping the ability to skip the remainder of
the test script?

Observing that all well-written test scripts we have begin with this
boilerplate line:

test_expect_success setup '

I wouldn't mind introducing a new helper function test_setup that
behaves like test_expect_success but is meant to be used in the
first "set-up" phase of the tests in a test script. Perhaps we can
give its failure a meaning different than failures in other normal
tests (e.g. "even set-up fails, and the remainder of the test is N/A
here, hence abort the whole thing"), and do not count "test_setup"
as part of the test (i.e. do not increment $test_count and friends).

--
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] t3300-*.sh: Fix a TAP parse error

2012-07-24 Thread Jonathan Nieder
Hi,

Ramsay Jones wrote:

> The only problematic platforms I test on are "NTFS/bash" on cygwin and MinGW.
> Since commit 2b843732 ("Suppress some bash redirection error messages",
> 26-08-2008), I have not noticed any complaints regarding this problem.

Yeah, that probably took care of squashing the messages.  Maybe my
memory is too long. ;-)

[...]
> No, I don't think this would be a good direction to go in. This may
> not be a good idea either, but if you wanted to add a check here, then
> maybe something like this (totally untested):
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index acda33d..53a2422 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -354,6 +354,9 @@ test_done () {
>   case "$test_failure" in
>   0)
>   # Maybe print SKIP message
> + if test -n "$skip_all" && test $test_count -gt 0; then
> + error "Can't use skip_all after running some tests"
> + fi
>   [ -z "$skip_all" ] || skip_all=" # SKIP $skip_all"
>  
>   if test $test_external_has_tap -eq 0; then

I think preventing invalid TAP output like this would be a very good
thing!  As a start, this patchlet looks good to me, and then I guess
we'll have to think more about what happens when a person wants to
skip_all_remaining after a test has already been run.

Care to format it as a "git am"-able patch, or should I?

Thanks again,
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


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

2012-07-24 Thread Ramsay Jones
Jonathan Nieder wrote:
>> Needless to say, I much prefer the patch below. :-D
> 
> Thanks for a nice explanation.  In general I definitely like getting
> rid of these setup tests when possible.  Let's see:
> 
> [...]
>> --- a/t/t3300-funny-names.sh
>> +++ b/t/t3300-funny-names.sh
>> @@ -15,28 +15,20 @@ p0='no-funny'
>>  p1='tabs," (dq) and spaces'
>>  p2='just space'
>>  
>> -test_expect_success 'setup' '
>> -cat >"$p0" <<-\EOF &&
>> -1. A quick brown fox jumps over the lazy cat, oops dog.
>> -2. A quick brown fox jumps over the lazy cat, oops dog.
>> -3. A quick brown fox jumps over the lazy cat, oops dog.
>> -EOF
>> +cat >"$p0" <<\EOF
>> +1. A quick brown fox jumps over the lazy cat, oops dog.
>> +2. A quick brown fox jumps over the lazy cat, oops dog.
>> +3. A quick brown fox jumps over the lazy cat, oops dog.
>> +EOF
> 
> The problem is that on platforms not supporting funny filenames, it
> will write a complaint to stderr and because the code is not guarded
> by test_expect_success, that output goes to the terminal.  So I think
> this is a wrong approach.

Huh? Which platforms are we talking about?

The only problematic platforms I test on are "NTFS/bash" on cygwin and MinGW.
Since commit 2b843732 ("Suppress some bash redirection error messages",
26-08-2008), I have not noticed any complaints regarding this problem.
What have I missed?

Assuming we are not talking about errors like ENOSPC, EROFS etc., then the
only command which would issue a complaint to stderr would be the line
following the above snippet, thus:

+cat 2>/dev/null >"$p1" "$p0"

(note the stderr redirection). This does not output an error to the terminal
when using bash (I think I also tested with dash). However, this does rely
on the shell performing the redirections in the order, left to right, on the
command line. [I had intended to check with POSIX to see if this order was
mandated or not, but didn't get around to it ...]

Have you found a shell were this does not work?

> Would it make sense to avoid the "# SKIP" comment when a test has
> been run, like this?
> 
> diff --git i/t/test-lib.sh w/t/test-lib.sh
> index acda33d1..038f6e9f 100644
> --- i/t/test-lib.sh
> +++ w/t/test-lib.sh
> @@ -354,6 +354,11 @@ test_done () {
>   case "$test_failure" in
>   0)
>   # Maybe print SKIP message
> + if test -n "$skip_all" && test "$test_count" != 0
> + then
> + say "# SKIP $skill_all"
> + skip_all=
> + fi
>   [ -z "$skip_all" ] || skip_all=" # SKIP $skip_all"
>  
>   if test $test_external_has_tap -eq 0; then

No, I don't think this would be a good direction to go in. This may
not be a good idea either, but if you wanted to add a check here, then
maybe something like this (totally untested):

diff --git a/t/test-lib.sh b/t/test-lib.sh
index acda33d..53a2422 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -354,6 +354,9 @@ test_done () {
case "$test_failure" in
0)
# Maybe print SKIP message
+   if test -n "$skip_all" && test $test_count -gt 0; then
+   error "Can't use skip_all after running some tests"
+   fi
[ -z "$skip_all" ] || skip_all=" # SKIP $skip_all"
 
if test $test_external_has_tap -eq 0; then

Dunno! :-D

I will be sending a v2 patch soon.

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] t3300-*.sh: Fix a TAP parse error

2012-07-21 Thread Jonathan Nieder
(cc-ing Ævar, TAP wizard)
Hi,

Ramsay Jones wrote:

> $ ./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:
[...]
>   Parse errors: No plan found in TAP output
[...]
> This is an RFC because I suspect some people may prefer the much
> simpler patch:
[...]
>   # since FAT/NTFS does not allow tabs in filenames, skip this test
> - skip_all='Your filesystem does not allow tabs in filenames'
> + say '# SKIP Your filesystem does not allow tabs in filenames'
>   test_done
[...]
> ... the output of which looks like:
[...]
> ok 1 - setup
> # SKIP Your filesystem does not allow tabs in filenames
> # passed all 1 test(s)
> 1..1
[..]
> Needless to say, I much prefer the patch below. :-D

Thanks for a nice explanation.  In general I definitely like getting
rid of these setup tests when possible.  Let's see:

[...]
> --- a/t/t3300-funny-names.sh
> +++ b/t/t3300-funny-names.sh
> @@ -15,28 +15,20 @@ p0='no-funny'
>  p1='tabs ," (dq) and spaces'
>  p2='just space'
>  
> -test_expect_success 'setup' '
> - cat >"$p0" <<-\EOF &&
> - 1. A quick brown fox jumps over the lazy cat, oops dog.
> - 2. A quick brown fox jumps over the lazy cat, oops dog.
> - 3. A quick brown fox jumps over the lazy cat, oops dog.
> - EOF
> +cat >"$p0" <<\EOF
> +1. A quick brown fox jumps over the lazy cat, oops dog.
> +2. A quick brown fox jumps over the lazy cat, oops dog.
> +3. A quick brown fox jumps over the lazy cat, oops dog.
> +EOF

The problem is that on platforms not supporting funny filenames, it
will write a complaint to stderr and because the code is not guarded
by test_expect_success, that output goes to the terminal.  So I think
this is a wrong approach.

Would it make sense to avoid the "# SKIP" comment when a test has
been run, like this?

diff --git i/t/test-lib.sh w/t/test-lib.sh
index acda33d1..038f6e9f 100644
--- i/t/test-lib.sh
+++ w/t/test-lib.sh
@@ -354,6 +354,11 @@ test_done () {
case "$test_failure" in
0)
# Maybe print SKIP message
+   if test -n "$skip_all" && test "$test_count" != 0
+   then
+   say "# SKIP $skill_all"
+   skip_all=
+   fi
[ -z "$skip_all" ] || skip_all=" # SKIP $skip_all"
 
if test $test_external_has_tap -eq 0; then
--
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] t3300-*.sh: Fix a TAP parse error

2012-07-21 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, 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 
---

Hi Jonathan,

This is an RFC because I suspect some people may prefer the much
simpler patch:

diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh
index 1f35e55..2a64385 100755
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -34,7 +34,7 @@ test_expect_success 'setup' '
 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'
+   say '# SKIP Your filesystem does not allow tabs in filenames'
test_done
 fi
 
... the output of which looks like:

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

$ prove --exec sh t3300-funny-names.sh
t3300-funny-names.sh .. ok
All tests successful.
Files=1, Tests=1,  1 wallclock secs ( 0.03 usr  0.01 sys +  \
0.85 cusr  0.34 csys =  1.24 CPU)
Result: PASS
$

Needless to say, I much prefer the patch below. :-D

ATB,
Ramsay Jones

 t/t3300-funny-names.sh | 26 +-
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/t/t3300-funny-names.sh b/t/t3300-funny-names.sh
index 1f35e55..c51674a 100755
--- a/t/t3300-funny-names.sh
+++ b/t/t3300-funny-names.sh
@@ -15,28 +15,20 @@ p0='no-funny'
 p1='tabs   ," (dq) and spaces'
 p2='just space'
 
-test_expect_success 'setup' '
-   cat >"$p0" <<-\EOF &&
-   1. A quick brown fox jumps over the lazy cat, oops dog.
-   2. A quick brown fox jumps over the lazy cat, oops dog.
-   3. A quick brown fox jumps over the lazy cat, oops dog.
-   EOF
-
-   { cat "$p0" >"$p1" || :; } &&
-   { echo "Foo Bar Baz" >"$p2" || :; } &&
+cat >"$p0" <<\EOF
+1. A quick brown fox jumps over the lazy cat, oops dog.
+2. A quick brown fox jumps over the lazy cat, oops dog.
+3. A quick brown fox jumps over the lazy cat, oops dog.
+EOF
 
-   if test -f "$p1" && cmp "$p0" "$p1"
-   then
-   test_set_prereq TABS_IN_FILENAMES
-   fi
-'
+cat 2>/dev/null >"$p1" "$p0"
+echo 'Foo Bar Baz' >"$p2"
 
-if ! test_have_prereq TABS_IN_FILENAMES
-then
+test -f "$p1" && cmp "$p0" "$p1" || {
# 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" &&
-- 
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