git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 6/7] test-lib: make --verbose output valid TAP

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Mar 10, 2021, 02:35 UTC
Message-ID
<875z1zivlv.fsf@evledraar.gmail.com>
In-Reply-To
<20210309213117.GB1016223@szeder.dev>
On Tue, Mar 09 2021, SZEDER Gábor wrote:
Show 29 quoted lines
> On Tue, Mar 09, 2021 at 09:57:03PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> >> diff --git a/t/README b/t/README
>> >> index 2cc8cbc7185..f09d94e754e 100644
>> >> --- a/t/README
>> >> +++ b/t/README
>> >> @@ -157,10 +157,13 @@ appropriately before running "make". Short options can be bundled, i.e.
>> >>  
>> >>  -V::
>> >>  --verbose-log::
>> >> -	Write verbose output to the same logfile as `--tee`, but do
>> >> -	_not_ write it to stdout. Unlike `--tee --verbose`, this option
>> >> +	An alias for `--verbose --tee`. This option
>> >>  	is safe to use when stdout is being consumed by a TAP parser
>> >> -	like `prove`. Implies `--tee` and `--verbose`.
>> >> +	like `prove`.
>> >> +	Historically this option was different from `--verbose --tee`
>> >> +	and would not write any verbose output to stdout to ensure the
>> >> +	TAP-correctness of the output. The TAP-correctness of the
>> >> +	output is now sanity checked by the test library,
>> >
>> > Not everyone is using a TAP harness to run the tests, and, therefore,
>> > '--verbose-log' should not spew out verbose output to the terminal.
>> 
>> IOW even though --verbose-log was meant as a hack to make prove happy,
>> you've since come to like the "verbose in log, but not stdout" mode and
>> want that kept?
>
> Yes, '--verbose-log' proved to be really convenient, even if it was
> meant to solve a different issue.
Sure, let's keep it then.
Show 15 quoted lines
>> Yes, this patch takes that mode away.
>> 
>> Yes, I can change it.
>> 
>> Would your use-case for this be satisfied by having prove(1) just emit
>> different output for you in this scenario, so you'd need to invoke this
>> as something like:
>> 
>>     prove <test> :: --verbose --tee # or --verbose-log
>
> I use prove to run the test suite, but I don't and won't use prove to
> run a single test.
>
> The behavior of './t1234-foo.sh -V' with or without '-x' should not
> change without _very_ convincing reasons.

I think I can make it the same, except for emulating the sheared write issue 452320f1f5 mentions. I.e. moving to this method inherently requires us to squash together stdout/stderr into one, which isn't what we're doing now.

> "We now output valid TAP even with --verbose, so we don't need it for
> the TAP harness" is definitely not convincing.

It's more like "we produce one machine-readable output format, so if you need junit, or an abbreviated format (--verbose-log) or a nyancat progress bar or whatever, it makes more sense to pipe it to another process that'll munge that for you... :)

Show 9 quoted lines
>> Becuse the advantage of this series is that that sort of thing becomes
>> really trivial without everything needing to be hardcoded in
>> test-lib.sh, observe (this is with my series):
>> 
>>     
>>     0 $ PERL5LIB=. prove -v --formatter=SZEDERVerboseLog ./t0201-gettext-fallbacks.sh :: --verbose-log
>
> Well, this doesn't look trivial at all, does it?  In fact, I consider
> this unusably convoluted.

... I meant to say something closer to "does that output look ok?". Obviously we'd then make the --verbose-log run something like that under the hood.

But in any case, I found it easier to just add this feature to my "tee" program than doing it with Perl's TAP libraries, i.e. something like this on top (will integrate it in an eventual re-roll):

    diff --git a/t/helper/test-tee.c b/t/helper/test-tee.c
    index 4ed34e9db9..063b9277e0 100644
    --- a/t/helper/test-tee.c
    +++ b/t/helper/test-tee.c
    @@ -29,2 +29,3 @@ int cmd__tee(int argc, const char **argv)
     {
    +       int only_tap_on_stdout = 0;
            int tap = 0;
    @@ -38,2 +39,4 @@ int cmd__tee(int argc, const char **argv)
            struct option options[] = {
    +               OPT_BOOL(0, "only-tap-on-stdout", &only_tap_on_stdout,
    +                        "only emit TAP on stdout, not 'unknown' etc."),
                    OPT_BOOL(0, "escape-stdout", &escape_stdout,
    @@ -75,3 +78,7 @@ int cmd__tee(int argc, const char **argv)
     
    -               fprintf(stdout, "%s\n", line.buf + offs);
    +               if (!only_tap_on_stdout)
    +                       fprintf(stdout, "%s\n", line.buf + offs);
    +               else if (offs)
    +                       fprintf(stdout, "%s\n", line.buf + offs);
    +
                    if (logfp)
    diff --git a/t/test-lib.sh b/t/test-lib.sh
    index 479ad4fb38..731ecc36bb 100644
    --- a/t/test-lib.sh
    +++ b/t/test-lib.sh
    @@ -153,2 +153,3 @@ parse_option () {
                    verbose=t
    +               verbose_log=t
                    tee=t
    @@ -371,2 +372,3 @@ then
                    --escape-stdout ${HARNESS_ACTIVE+--escape-file} \
    +               ${verbose_log+--only-tap-on-stdout} \
                    "$GIT_TEST_TEE_OUTPUT_FILE"
Show 50 quoted lines
>>     # lib-gettext: No is_IS UTF-8 locale available
>>     # lib-gettext: No is_IS ISO-8859-1 locale available
>>     ok 1 - sanity: $GIT_INTERNAL_GETTEXT_SH_SCHEME is set (to fallthrough)
>>     ok 2 - sanity: $GIT_INTERNAL_GETTEXT_TEST_FALLBACKS is set
>>     ok 3 - sanity: $GIT_INTERNAL_GETTEXT_SH_SCHEME" is fallthrough
>>     ok 4 - gettext: our gettext() fallback has pass-through semantics
>>     ok 5 - eval_gettext: our eval_gettext() fallback has pass-through semantics
>>     ok 6 - eval_gettext: our eval_gettext() fallback can interpolate variables
>>     ok 7 - eval_gettext: our eval_gettext() fallback can interpolate variables with spaces
>>     ok 8 - eval_gettext: our eval_gettext() fallback can interpolate variables with spaces and quotes
>>     # passed all 8 test(s)
>>     1..8
>>     ok
>>     All tests successful.
>>     Files=1, Tests=8,  1 wallclock secs ( 0.01 usr  0.01 sys +  0.07 cusr  0.01 csys =  0.10 CPU)
>>     Result: PASS
>>     $ wc -l test-results/t0201-gettext-fallbacks.out 
>>     75 test-results/t0201-gettext-fallbacks.out
>> 
>> All without any patching on top to the test-lib.sh, just:
>>     
>>     $ cat SZEDERVerboseLog.pm 
>>     package SZEDERVerboseLog::Session;
>>     use base 'TAP::Formatter::Console::Session';
>>     
>>     sub result {
>>         my ($self, $result) = @_;
>>         return if $result->is_unknown;
>>         return $self->SUPER::result($result);
>>     }
>>     
>>     package SZEDERVerboseLog;
>>     use strict;
>>     use warnings;
>>     use base 'TAP::Formatter::Console';
>>     
>>     sub open_test {
>>         my ($self, $test, $parser) = @_;
>>         my $session = SZEDERVerboseLog::Session->new( {
>>             name            => $test,
>>             formatter       => $self,
>>             parser          => $parser,
>>         } );
>>         return $session;
>>     }
>>     
>>     1;
>> 
>> The "is_unknown" is everything that's not TAP syntax.
>>
Previous: SZEDER GáborNext: Ævar Arnfjörð Bjarmason
Message 33 of 47 in “Prove "Tests out of sequence" Error”
  1. Lars SchneiderOct 21, 2016
  2. Stefan BellerOct 21, 2016
  3. Jeff KingOct 21, 2016
  4. Jeff KingOct 21, 2016
  5. 0/3 fix travis TAP/--verbose conflictJeff King, Oct 21, 2016
  6. 1/3 test-lib: handle TEST_OUTPUT_DIRECTORY with spacesJeff King, Oct 21, 2016
  7. 2/3 test-lib: add --verbose-log optionJeff King, Oct 21, 2016
  8. Junio C HamanoOct 21, 2016
  9. Jeff KingOct 21, 2016
  10. test-lib: make --verbose work under proveÆvar Arnfjörð Bjarmason, Feb 28, 2021
  11. Jeff KingMar 1, 2021
  12. Ævar Arnfjörð BjarmasonMar 1, 2021
  13. 0/6 test-lib: make --verbose output valid TAPÆvar Arnfjörð Bjarmason, Mar 9, 2021
  14. SZEDER GáborMar 9, 2021
  15. Ævar Arnfjörð BjarmasonMar 9, 2021
  16. SZEDER GáborMar 9, 2021
  17. 3/7 test-lib: color "ok" TAP directives green under --verbose (or -x)Ævar Arnfjörð Bjarmason, Mar 9, 2021
  18. 4/7 test-lib: add tee with TAP support to test-toolÆvar Arnfjörð Bjarmason, Mar 9, 2021
  19. 2/7 test-lib: add say_color_tap helper to emit TAP formatÆvar Arnfjörð Bjarmason, Mar 9, 2021
  20. Junio C HamanoMar 10, 2021
  21. 1/7 test-lib: remove test_externalÆvar Arnfjörð Bjarmason, Mar 9, 2021
  22. Junio C HamanoMar 10, 2021
  23. Ævar Arnfjörð BjarmasonMar 10, 2021
  24. 5/7 test-lib: indent and format GIT_TEST_TEE_OUTPUT_FILE codeÆvar Arnfjörð Bjarmason, Mar 9, 2021
  25. 7/7 test-lib: generate JUnit output via TAPÆvar Arnfjörð Bjarmason, Mar 9, 2021
  26. Johannes SchindelinMar 19, 2021
  27. Ævar Arnfjörð BjarmasonMar 21, 2021
  28. Johannes SchindelinMar 22, 2021
  29. 6/7 test-lib: make --verbose output valid TAPÆvar Arnfjörð Bjarmason, Mar 9, 2021
  30. SZEDER GáborMar 9, 2021
  31. Ævar Arnfjörð BjarmasonMar 9, 2021
  32. SZEDER GáborMar 9, 2021
  33. Ævar Arnfjörð BjarmasonMar 10, 2021
  34. Ævar Arnfjörð BjarmasonMar 16, 2021
  35. SZEDER GáborMar 9, 2021
  36. Junio C HamanoMar 10, 2021
  37. Chris TorekMar 10, 2021
  38. 3/3 travis: use --verbose-log test optionJeff King, Oct 21, 2016
  39. Stefan BellerOct 21, 2016
  40. Lars SchneiderOct 24, 2016
  41. Jacob KellerOct 21, 2016
  42. Jeff KingOct 21, 2016
  43. Jacob KellerOct 21, 2016
  44. Jeff KingOct 21, 2016
  45. Jacob KellerOct 21, 2016
  46. 4/3 test-lib: bail out when "-v" used under "prove"Jeff King, Oct 22, 2016
  47. Jacob KellerOct 22, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.