After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769135 - External control for g_test_add/g_test_run
External control for g_test_add/g_test_run
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
2.48.x
Other Mac OS
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2016-07-24 20:48 UTC by Daniel Macks
Modified: 2017-08-03 11:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add missing documentation for "-s" (2.24 KB, patch)
2016-07-24 21:32 UTC, Daniel Macks
committed Details | Review
Use G_TEST_SKIP_PATHS env var as list of tests to force status=SKIP (3.19 KB, patch)
2017-05-22 02:48 UTC, Daniel Macks
none Details | Review
Handle -s as explicit SKIP instead of inhibiting altogether (3.58 KB, patch)
2017-05-25 14:27 UTC, Daniel Macks
none Details | Review
Handle -s as explicit SKIP instead of inhibiting altogether (3.58 KB, patch)
2017-06-02 21:02 UTC, Daniel Macks
committed Details | Review

Description Daniel Macks 2016-07-24 20:48:02 UTC
I would like a way to have the user cause specific tests to be skipped or xfailed without having to hack a test-suite's source files. Rationale: while porting to OS X, I know I have some tests that will fail for odd platform reasons that I might care about fixing yet. And some other tests seem to be failing due to the sandbox I'm using. And some other ones are really slow but pass, so I don't want to wait for them every time.

Currently, I can hack */tests/*.c to comment-out some of the g_test_add_func() calls, but that means I have to rehack/recompile any time I change my mind. I can call 'make check' with the full list of whole test suites I _do_ want (test_programs=... or similar) or maybe that I expect to fail (XFAIL_TESTS I think?), is not very fine-grained. Instead, I want the "set a variable" convenience for the g_test_add_func() level of granularity and alteration of default over-rides. 

Consider instead if g_test_run() checked an environment variable for a list of testpath strings, and caused these individual tests to be g_test_skip()ed.
Comment 1 Daniel Macks 2016-07-24 21:20:29 UTC
I just found the "-s" flag in gtestutils.c:parse_args, which is not listed in the g_test_init() documentation. That seems to be what I want. Does anyone know what Makefile variable I use to propagate command-line parameters to the test programs?
Comment 2 Daniel Macks 2016-07-24 21:32:17 UTC
Created attachment 332054 [details] [review]
Add missing documentation for "-s"

This doesn't fix my problem fully (still need mechanism to pass that flag), but at least now more people will know it exists.
Comment 3 Matthias Clasen 2016-07-25 16:36:32 UTC
Review of attachment 332054 [details] [review]:

sure
Comment 4 Daniel Macks 2016-07-25 18:50:07 UTC
Automake does not appear to have a way to propagate commandline arguments to test programs. Presumably other build toolchains might (but I can't find any quickly), and no reason to expect even those that do have it standardized. Is it reasonable for g_test_init() to look for a shell env that is processed as if it were additional argv? That would be a toolchain-agnostic way (==nicely portable for end users) for anyone using gtestutils. If it sounds reasonable, I can hack on it.
Comment 5 Matthias Clasen 2016-07-26 13:06:31 UTC
I don't like environment variables
Comment 6 Matthias Clasen 2016-07-26 13:09:16 UTC
And they don't strike me as the most appropriate way of handling expected failures: those xfail marking will probably have to be maintained over time; so you'll have to stuff those environment variables into a shell script to set them - why not use a file in the first place.
Comment 7 Daniel Macks 2016-07-26 14:15:45 UTC
I'm confused how I would use a file (and even more confused by automake the more I experiment with how the tests are run). What should I put where to, as a specific example, skip (or xfail) the /fileutils/stdio-wrappers test that is part of the glib/tests/fileutils suite?
Comment 8 Daniel Macks 2017-05-22 02:48:34 UTC
Created attachment 352326 [details] [review]
Use G_TEST_SKIP_PATHS env var as list of tests to force  status=SKIP

Here's what I'm using in fink. For example, per bug 780384, comment 7 I know that /contenttype/icon and /contenttype/symbolic-icon are two specific tests that fail on my platform, so I just want to skip them. So I:

G_TEST_SKIP_PATHS=/contenttype/icon:/contenttype/symbolic-icon

and now they are SKIP'ed as if their own internal logic decided to skip them on this platform.
Comment 9 Philip Withnall 2017-05-22 07:45:09 UTC
Review of attachment 352326 [details] [review]:

Like Matthias, I am not a fan of environment variables. The state space is too global.

Your workflow sounds like a very specific one: I think you’re better off having a local helper script which sets up the `make check` environment and runs the test program with -s set appropriately.
Comment 10 Daniel Macks 2017-05-22 08:04:06 UTC
I agree that the env var is not a clean solution.

However, as I mentioned, -s seems disallowed by when tests use certain test configurations. In my original hacking, I simply patched the internal code that handles -s flags to overload it with my values (so I didn't have to figure out the makefile or wrapper incantations). I got a pile of:

options that skip some tests are incompatible with --tap

from g_test_init's "/* sanity check */" code.

And also -s completely disables the test, so it does not appear in the 'make check' transcript. My goal was to make it *visibly* skipped, so that results are easier to compare on different platforms. Some test outputs are numbered sequentially, so *omitting* (for example) test 7 gets the meanings of "test 8" out of sync--again, harder to compare results on different platforms--whereas having test 7 still exist doesn't alter the meaning or output of the test 8.

Should I fork this latter issue, the actual handling of -s or different meanings of "skipping" a test, into a separate bug?
Comment 11 Philip Withnall 2017-05-22 08:30:39 UTC
(In reply to Daniel Macks from comment #10)
> I agree that the env var is not a clean solution.
> 
> However, as I mentioned, -s seems disallowed by when tests use certain test
> configurations. In my original hacking, I simply patched the internal code
> that handles -s flags to overload it with my values (so I didn't have to
> figure out the makefile or wrapper incantations). I got a pile of:
> 
> options that skip some tests are incompatible with --tap
> 
> from g_test_init's "/* sanity check */" code.
> 
> And also -s completely disables the test, so it does not appear in the 'make
> check' transcript. My goal was to make it *visibly* skipped, so that results
> are easier to compare on different platforms. Some test outputs are numbered
> sequentially, so *omitting* (for example) test 7 gets the meanings of "test
> 8" out of sync--again, harder to compare results on different
> platforms--whereas having test 7 still exist doesn't alter the meaning or
> output of the test 8.

Ah, I missed that bit, sorry. I wonder why -s omits the test rather than SKIPping it? It would seem to make more sense to make -s actually emit TAP skip output for the test. I’d be happy with a patch which does this (and which removes the corresponding check from the ‘sanity check’ code).

> Should I fork this latter issue, the actual handling of -s or different
> meanings of "skipping" a test, into a separate bug?

It would solve this bug for you, wouldn’t it? I suggest keeping it in this bug, and morphing the subject slightly to be about making -s work with TAP.
Comment 12 Dan Winship 2017-05-22 14:00:39 UTC
(In reply to Philip Withnall from comment #11)
> Ah, I missed that bit, sorry. I wonder why -s omits the test rather than
> SKIPping it?

-s predates g_test_skip(). I don't think there was any particular reason for NOT rewriting "-s" to behave the new way though, other than "no one really cared".
Comment 13 Daniel Macks 2017-05-25 14:27:26 UTC
Created attachment 352580 [details] [review]
Handle -s as explicit SKIP instead of inhibiting altogether

Also documents why some flags are not allowed for TAP (from commit  6e382208f72cfd449cf076ac1f1fa340fe6eea0f). Those other ones are harder to fix.
Comment 14 Daniel Macks 2017-05-25 14:30:48 UTC
Does anyone know how to actually get that flag passed to the programs for purposes of "make check"?
Comment 15 Daniel Macks 2017-05-25 16:23:03 UTC
(In reply to Daniel Macks from comment #14)
> Does anyone know how to actually get that flag passed to the programs for
> purposes of "make check"?

Ahah! I can stuff them into the ./tap-test script
Comment 16 Philip Withnall 2017-05-29 10:35:50 UTC
Review of attachment 352580 [details] [review]:

Looks good apart from this nitpick.

::: glib/gtestutils.c
@@ +2153,3 @@
       g_test_log_set_fatal_handler (NULL, NULL);
+      if (test_paths_skipped && g_slist_find_custom (test_paths_skipped, test_run_name, (GCompareFunc)g_strcmp0))
+        g_test_skip("by request (-s option)");

Nitpick: Missing a space before the `(` in the function call.
Comment 17 Daniel Macks 2017-06-02 21:02:05 UTC
Created attachment 353099 [details] [review]
Handle -s as explicit SKIP instead of inhibiting altogether

Fix nit noted in Comment #16.
Comment 18 Philip Withnall 2017-06-05 15:06:09 UTC
Review of attachment 353099 [details] [review]:

++
Comment 19 Philip Withnall 2017-08-03 11:46:38 UTC
Pushed. Sorry for the delay.