rra / c-tap-harness

C harness for running TAP-compliant tests
https://www.eyrie.org/~eagle/software/c-tap-harness/
Other
36 stars 15 forks source link

diag() printing to stdout vs stderr #11

Open adeason opened 6 months ago

adeason commented 6 months ago

The diag() in c-tap-harness currently prints to stdout, but the comments mention stderr:

/*
 * Report a diagnostic to stderr.  Always returns 1 to allow embedding in
 * compound statements.
 */

(and libtap.sh is similar)

In contrast, the diag() from perl's Test::More et al does actually print to stderr and I assume has for a long time. This difference can be pretty confusing, since runtests from c-tap-harness discards stderr unless we run with -o.

So if you're running tests through runtests -v, you get diagnostic info if the test was written in C using c-tap-harness, but you don't if it's written in perl's Test::More. If a test fails, you don't know why:

$ cat foo-t
#!/usr/bin/env perl

use strict;
use warnings;

use Test::More;

plan tests => 1;

is(1, 0, "example");

$ runtests -v foo
foo

1..1
not ok 1 - example
FAILED 1 (exit status 1)
[...]

The behavior of c-tap-harness makes more sense to me than perl's, but I'm not familiar with the historical context here. Searching for some info finds some discussion implying this can be a contentious issue (such as https://www.nntp.perl.org/group/perl.qa/2007/03/msg8289.html ), suggesting that there may be no good fix for this.

If we just don't ignore stderr for runtests -v (or have a new option to let stderr through), that would let the diagnostics be visible. They'd probably be interleaved weirdly, but at least the data is there.

Or if things are left alone, it could be helpful to mention this as a slight incompatibility with other TAP producers like perl's. The user can change any perl tests to show diagnostic info on stdout in the TAP stream; having a note would help explain why you're not seeing any diagnostics and what you need to do.

jaltman commented 6 months ago

The commit message of commit a2471c10beef9f8eff3ec2f09f10a90bd9effe75 consistently references stderr instead of stdout but writes to stdout. @rra, could this have been an oversight in the original commit?

Author: Russ Allbery rra@stanford.edu Date: Tue Apr 13 17:00:10 2010 -0700

  Add diag and sysdiag functions to the C TAP library

  Add diag and sysdiag functions to the basic C TAP library, which output
  diagnostics to standard error preceded by the protocol-required #
  character.

` +/*

rra commented 6 months ago

There are two different problems here, one of which is a long-standing missing feature and one of which was (I think) intentional but an unfortunate mismatch with Perl's terminology that should probably be fixed.

Perl's test framework distinguishes between diag (which goes to stderr) and note (which goes to stdout). I think at the time I implemented this note may not have existed (or perhaps I just missed it), so c-tap-harness only implements diag. I think I originally intended it to use stderr but then immediately discovered that running such tests under prove spammed all the diagnostics into the summary output, which was not my intent. So I switched to stdout and effectively made it equivalent to note. I now use diag (and diag_file_add) all over the place with the intended semantics of note.

Separately, Perl's harness passes through stderr, which frequently results in very messy and ugly output but which does mean that the errors are more visible. My intent was to capture stderr and display it more intelligently than Perl does, after the failed test, but I've never gotten around to implementing that. (It's the sort of thing that's kind of annoying to do in C and requires a bit of thought about the output format.)

I think the correct solution to the first problem is unfortunately not backward-compatible: introduce note (and note_file_add and note_file_remove), and then change diag and friends to use stderr. This still won't restore the same output as the Perl test harness; that will require implementing the more difficult feature of buffering and outputting stderr in the test summary somehow.

rra commented 6 months ago

Oh, and re-reading the issue, there's a third problem as well: runtests -v probably shouldn't discard stderr.