Bug 176755 - [WPE][GTK] Run tests with G_DEBUG=fatal-criticals
Summary: [WPE][GTK] Run tests with G_DEBUG=fatal-criticals
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-11 16:56 PDT by Michael Catanzaro
Modified: 2018-03-05 06:49 PST (History)
6 users (show)

See Also:


Attachments
Patch (3.78 KB, patch)
2017-09-11 17:07 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (2.94 KB, patch)
2017-09-12 07:32 PDT, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>