GNOME Bugzilla – Bug 754284
gtestutils: print the TAP test plan first, not last
Last modified: 2015-08-31 18:18:52 UTC
Some random gtestutils reorganization to let us print the TAP "1..N" line at the beginning rather than at the end, so that automake won't claim "missing test plan" when a test program crashes... (Counting the tests ahead of time is slightly tricky, and this is a pretty minor improvement, so I'm cool if this gets WONTFIXed.)
Created attachment 310252 [details] [review] gtestutils: reorganize g_test_name manipulation
Created attachment 310253 [details] [review] gtestutils: make g_test_suite_run{,internal} less confusing Rewrite g_test_suite_run() and g_test_suite_run_internal() to make it clearer what they do (while still preserving exact backward compatibility, meaning we need to handle the "-p" case differently from the non-"-p" case).
Created attachment 310254 [details] [review] gtestutils: move "/subprocess" path special-casing
Created attachment 310255 [details] [review] gtestutils: print the TAP test plan first, not last TAP allows you to print the "test plan" (ie, the expected number of tests" either at the start or the end of the test program, but if you put it at the end, and the program crashes, automake will complain "missing test plan", which is confusing to users (particularly since it prints that *before* it prints that the test program crashed, suggesting that somehow the lack of test plan was responsible for the crash or something, rather than vice versa). Anyway, change it to count the tests ahead of time, and print the test plan first. Keeping this simple requires disallowing the '-p', '-s', and '--GTestSkipCount' options when using '--tap' (although we were already printing the wrong number in the --GTestSkipCount case anyway).
Review of attachment 310252 [details] [review]: Okay.
Review of attachment 310253 [details] [review]: ::: glib/gtestutils.c @@ +2185,3 @@ + GTestCase *tc = iter->data; + + test_run_name = g_build_path ("/", old_name, tc->name, NULL); Mmh, g_build_path() on Windows will generate a test path like '/foo\bar\baz', which is not what we want… @@ +2198,3 @@ + GTestSuite *ts = iter->data; + + test_run_name = g_build_path ("/", old_name, ts->name, NULL); Same as above.
Review of attachment 310254 [details] [review]: Looks good.
Review of attachment 310255 [details] [review]: Looks good.
(In reply to Emmanuele Bassi (:ebassi) from comment #6) > + test_run_name = g_build_path ("/", old_name, tc->name, NULL); > > Mmh, g_build_path() on Windows will generate a test path like > '/foo\bar\baz', which is not what we want… No, that's g_build_filename(). g_build_path() takes the separator to use as its first argument. (It only took me a few years to be able to remember this distinction reliably...)
Review of attachment 310253 [details] [review]: Dan explained this, update patch status
Review of attachment 310254 [details] [review]: Looks good to me too
Review of attachment 310255 [details] [review]: Great, I've always wanted to print the test plan first, but couldn't figure out how to make it work.
Attachment 310252 [details] pushed as 510331b - gtestutils: reorganize g_test_name manipulation Attachment 310253 [details] pushed as 91ff2ba - gtestutils: make g_test_suite_run{,internal} less confusing Attachment 310254 [details] pushed as 51c91ed - gtestutils: move "/subprocess" path special-casing Attachment 310255 [details] pushed as 6e38220 - gtestutils: print the TAP test plan first, not last