Created attachment 263990 [details] [review]
gtestutils: skipping a test should count as success, not failure

Review of attachment 263990 [details] [review]:

Agreed with this patch, but there is another bug here.

The overall return value from g_test_run() is the number of failed tests, even if this number is 77.  We should probably fix that to never return 77, and then return 77 in the case that all tests were skipped.

Created attachment 264483 [details] [review]
gtestutils: rename test_skip_count to test_startup_skip_count

Created attachment 264484 [details] [review]
Make g_test_run() return 77 if all tests are skipped

Change g_test_run() to return 1 on failure (rather than the number of
failed tests), and 77 if all tests are skipped (since automake and
some other test harnesses recognize that status code).

Previously g_test_run() returned the number of failed tests, but this
behavior was not documented, and at any rate, prior to 2.39,
g_test_run() would normally not return at all if an error occurred.

Review of attachment 264484 [details] [review]:

::: glib/gtestutils.c
@@ +1531,3 @@
+    return 1;
+
+  if (test_run_count > 0 && test_run_count == test_skipped_count)

Why specifically check this?

It's easy to imagine some situation like so:

main ()
{
  test_init()

  if (cond) { add tests; }
  if (other_cond) { add tests; }

  return test_run();
}

where no tests actually get added.  In that case, I'd actually expect to see the 77.

On the other hand, this could be considered an incompatible change because it would cause a process that used to return 0 to start returning 77... some users of gtester who are not "77-aware" might be hit by that...

Review of attachment 264483 [details] [review]:

Makes sense if we decide to commit the other one, but please wait until then.

Review of attachment 264484 [details] [review]:

11:25 < danw> desrt: because really you should do the perf check inside the test and call g_test_skip() if you're 
              skipping, so that it shows up as skipped in the output and the user is made aware that there are tests 
              that are not being run
11:26 < danw> desrt: you could make an argument for any of the three return values when there are no test cases...
11:27 < desrt> so i guess we can allow the argument of 'preserve current behaviour' to prevail

Attachment 263990 [details] pushed as 8c188fc - gtestutils: skipping a test should count as success, not failure
Attachment 264483 [details] pushed as 10d82f9 - gtestutils: rename test_skip_count to test_startup_skip_count
Attachment 264484 [details] pushed as fab0805 - Make g_test_run() return 77 if all tests are skipped

*** Bug 709894 has been marked as a duplicate of this bug. ***

Any chance of this being backported into the 2.38 branch?