…by removing stdout buffering and always cat tests.log after a travis run. Fixes #645.
I noticed that according to the doc tests.log should contain stdout as well as stderr. But it doesn’t because stdout isn’t flushed. I removed buffering completely to avoid having to call fflush twice.
Travis is instructed to always show the seed which seems helpful with after_script by cating ./tests.log. In case the tests fail it looks like https://travis-ci.org/jonasnick/secp256k1/jobs/606446234.
5207@@ -5208,6 +5208,7 @@ int main(int argc, char **argv) {
5208 }
5209 secp256k1_rand_seed(seed16);
5210
5211+ setbuf(stdout, NULL);
setbuf should be called before any operation is performed on the stream. This means that it is currently in the right position but I think it would be less fragile to put at the beginning of main(). (For example, someone could have the idea of changing the fprintf(stderr, ...) in line 5195 to stdout.) Also, I think this call deserves a comment with a rationale.
Thanks for picking up this issue, looks good!
ACK except nit
make check cat the log files to stderr in case of failure.
Good point, but I’m tempted to blame this on the user. With automake you could run make check with VERBOSE=1 which will print the stdout and stderr of a failed test to stdout. So not capturing stdout seems to be not a good idea when you need diagnostics anyway. Setting VERBOSE=1 should be possible in a build system and is probably useful for other automake based packages too.
If I understand correctly what you’re asking for is to define a custom test runner for which my automake skills are insufficient. And it could be argued that such a non-standard customization isn’t particularly helpful in the long run.
Very nice change. Might it makes sense to also unbuffer stderr?
It is probably unbuffered on most systems (https://stackoverflow.com/a/3746795) but yep, it won’t hurt and it’s good for consistency.