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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html