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

Reply via email to