Bug 176755

Summary: [WPE][GTK] Run tests with G_DEBUG=fatal-criticals
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, buildbot, cgarcia, commit-queue, glenn, mcatanzaro
Priority: P2    
Version: Other   
Hardware: PC   
OS: Linux   
See Also: https://bugs.webkit.org/show_bug.cgi?id=183330
Attachments:
Description Flags
Patch
none
Patch none

Description Michael Catanzaro 2017-09-11 16:56:40 PDT 
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.
Comment 1 Michael Catanzaro 2017-09-11 17:04:05 PDT 
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.
Comment 2 Michael Catanzaro 2017-09-11 17:07:02 PDT 
Created attachment 320507 [details]
Patch
Comment 3 Carlos Garcia Campos 2017-09-11 23:07:48 PDT 
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().
Comment 4 Michael Catanzaro 2017-09-12 07:32:01 PDT 
Created attachment 320546 [details]
Patch
Comment 5 Carlos Garcia Campos 2017-09-12 08:25:41 PDT 
Comment on attachment 320546 [details]
Patch

Ok, let's try
Comment 6 WebKit Commit Bot 2017-09-12 09:59:27 PDT 
Comment on attachment 320546 [details]
Patch

Clearing flags on attachment: 320546

Committed r221925: <http://trac.webkit.org/changeset/221925>
Comment 7 WebKit Commit Bot 2017-09-12 09:59:29 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 Michael Catanzaro 2017-09-12 13:44:39 PDT 
Committed r221938: <http://trac.webkit.org/changeset/221938>