Bug 176755 – [WPE][GTK] Run tests with G_DEBUG=fatal-criticals

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

Our bots are too green. Make more tests fail by running with G_DEBUG=fatal-warnings. This is expected to break two GTK+ API tests and <50 layout tests (but only due to a couple different bugs that affect many tests). I'm investigating the API tests now. It's too hard to update layout test expectations before the patch lands, so I'll handle that after landing.

This will be particularly useful for layout tests as right now we have some criticals that are hard to file bug reports for without accompanying backtraces.

I really want to use fatal-warnings, but if this gets rejected, we should *definitely* use fatal-criticals.

[Michael Catanzaro]

Actually I think we can't be this strict for API tests, looks like they're testing stuff that actually needs to emit warnings. E.g.:

    // There can't be more than one context with automation enabled
    GRefPtr<WebKitWebContext> otherContext = adoptGRef(webkit_web_context_new());
    test->removeLogFatalFlag(G_LOG_LEVEL_WARNING);
    webkit_web_context_set_automation_allowed(otherContext.get(), TRUE);
    test->addLogFatalFlag(G_LOG_LEVEL_WARNING);
    g_assert(!webkit_web_context_is_automation_allowed(otherContext.get()));

But API tests should still always crash on criticals. No exceptions to that. Let's use fatal-criticals for layout tests to be consistent. I'm not aware of any warnings being printed in layout tests, and the important part of this change is to get backtraces to criticals.

[Michael Catanzaro]

Created attachment 320507 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 320507 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=320507&action=review

> Tools/Scripts/run-gtk-tests:220
> +        self._test_env["G_DEBUG"] = "fatal-criticals"

This is not needed, glib tests are already run with both warnings and criticals fatal, that's why we need to change the flags in some test when we want to actually check the warnings. This is done by g_test_init using g_log_set_always_fatal().

[Michael Catanzaro]

Created attachment 320546 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 320546 [details]
Patch

Ok, let's try

[WebKit Commit Bot]

Comment on attachment 320546 [details]
Patch

Clearing flags on attachment: 320546

Committed r221925: <http://trac.webkit.org/changeset/221925>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

Committed r221938: <http://trac.webkit.org/changeset/221938>