GNOME Bugzilla – Bug 788467
Fatal errors and warnings should be reported as TAP
Last modified: 2017-10-03 16:24:16 UTC
Created attachment 360827 [details] [review] testutils: Report fatal errors and warnings as TAP Lines starting with "Bail out!" are special TAP syntax: they mark the entire test execution (one binary or script) as failed, and stop processing. Automake's parallel test harness knows this, and will print the diagnostic in the test results, leading to clearer output. Without this change, having changed glib/tests/bytes.c to emit a spurious g_warning(): ERROR: bytes - too few tests run (expected 15, got 0) ERROR: bytes - exited with status 133 (terminated by signal 5?) With this change, it's clearer what has happened: ERROR: bytes - Bail out! FATAL-WARNING: I broke this as a demonstration --- I'm partly proposing this to make developers' lives easier when debugging test failures that happened locally, and partly because autobuilder maintainers reporting a failed build have a tendency to quote the first error seen, and "ERROR: foo - too few tests run" is not very useful. This should work for any g_error(), and for any g_critical() or g_warning() that has been made fatal.
Review of attachment 360827 [details] [review]: Looks good to me. Please push to master with or without the nitpick fix. ::: glib/gtestutils.c @@ +875,3 @@ + if (test_tap_log) + g_print ("Bail out! %s\n", string1); + else if (g_test_verbose()) Nitpick: Missing space before `()` (I realise this is a pre-existing problem from the code above which has been copied, but let’s fix both instances in this patch).
Comment on attachment 360827 [details] [review] testutils: Report fatal errors and warnings as TAP Committed as 23ba8aec9, thanks. (In reply to Philip Withnall from comment #1) > Nitpick: Missing space before `()` (I realise this is a pre-existing problem > from the code above which has been copied, but let’s fix both instances in > this patch). It's sufficiently consistently wrong that I didn't feel comfortable changing it, because having fixed one instance, it seemed strange not to fix the next, and it turns out there are quite a lot in that function. I've prepared another commit instead.
Created attachment 360841 [details] [review] g_test_log: Consistently use GLib whitespace style
Review of attachment 360841 [details] [review]: You are an excellent person, thank you.
Comment on attachment 360841 [details] [review] g_test_log: Consistently use GLib whitespace style Pushed as 733c7bd9c
Fixed in git for 2.55.0