GNOME Bugzilla – Bug 709615
Cannot use g_test_expect_message with g_error
Last modified: 2013-10-21 18:33:55 UTC
Dear GLib Developers, In #514920 a "for(;;) ;" was added to the g_error() macro because it was assumed that it should never return. Yet is one uses g_test_expect_message() to try catch the precise error that occured, the g_error() macro will return has the underlying g_logv function does. This makes the test case enter in an infinite loop. This problem was discovered on a Debian Jessie/SID running GLib 2.36.4, but having a quick look a the git repository show that the harmful loop is still present ([1], line 203 in HEAD). Regards, Nicolas CANIART. [1] https://git.gnome.org/browse/glib/tree/glib/gmessages.h#n195
from the g_test_expect_message() docs: "Note that you cannot use this to test g_error() messages, since g_error() intentionally never returns even if the program doesn't abort; use g_test_trap_subprocess() in this case." Alternatively though, now that we have G_VERSION_MIN_REQUIRED / G_VERSION_MAX_ALLOWED, we could take one of the various other options in bug 514920 without causing ABI issues, and then make it possible to use g_test_expect_messages() with g_error() as well.
Hi, I missed that in the documentation. Yet there is something wrong anyways since g_error() indeed returns, in that case. Either it should not or g_test_expect_message() should reject any G_LOG_LEVEL_ERROR as an input, as it is exceptionnally invalid with respect to the values of the GLogLevelFlags enumeration. I personnaly like testing error conditions, as well normal conditions. The goal being, providing the user with as much as possible useful, and *predictible* feedback on what went wrong. Thus the test(s) I wrote that made me stumble on this. And all in all, I think removing "special cases" is generally a good idea in general. But I am not sure I grasp the whole picture with respect to the 'noreturn' attribute and the "for(;;);" GCC trick. So if there are any technical reasons that would make this a bad idea, I could live with it (feel free to provide quick explanation or pointers. I tryed to look into this but what I found so far is fairly vague). Thanks, Nicolas.
Note that you can still use g_test_trap_subprocess() and g_test_trap_assert_stderr() to test g_error()s. It's less convenient, but it's doable.
Hi, This works only with glib >= 2.38.0, which has not landed in Debian unstable yet. I'll try that as soon as it does. Looking at the doc, it seems simple enough. I would still have g_test_expect_message() fail if given G_LOG_LEVEL_ERROR as input. You know, for guys who only take a glimpse at a function prototype in the doc, and think they can figure out the rest, without reading the most of the text below. (Yeah, who does that? Right ? ;-) Then I guess the bug can be closed.
Created attachment 257678 [details] [review] Don't g_test_assert_expected_messages for g_error Don't allow the user to assert for expected g_error(). They need to use subprocess for this.
Review of attachment 257678 [details] [review]: Makes sense to me.
Pushed. Thanks.