Bug 176104 – [GTK] Improve the way unit test are run and the results reported

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Carlos Garcia Campos]

There are several issues with the way unit tests are run by run-gtk-tests and also with the way results are reported:

 - The results summary only mentions the test binaries, not the actual test cases, so you always have to scroll up to find the actual test cases failing.
 - The number of reported failures is the number of test binaries that failed, so if a new test case fails for the same binary in a new revision, we won't notice it just looking at the number of failures.
 - We show detailed information about skipped test ins the results summary, which is just noise.
 - In the case of glib tests, when a test case times out, we finish the test suite, instead of continue with the rest of the test case like we do for normal failures or crashes. If a new test case fails after a test case that times out we will not notice until we fix or skip the test cases timing out.
 - In the case of glib tests, the timeout is aplied to the whole suite, instead of per test case, we have a hack to make it longer only for that. It has worked so far, but it doesn't scale, and it's an ugly hack.
 - It's not currently possible to detect flaky tests, because again, we know the binaries/suites that failed but not the actual test cases.

[Carlos Garcia Campos]

Created attachment 319362 [details]
Patch

Current test results is something like this:

Tests failed (4): /WebKit2Gtk/TestWebKitWebContext, /WebKit2Gtk/TestWebExtensions, /WebKit2Gtk/TestWebsiteData, /WebKit2Gtk/TestBackForwardList
Tests that timed out (3): /WebKit2Gtk/TestWebViewEditor, /WebKit2Gtk/TestWebKitWebView, /WebKit2/TestWebKit2

With the patch the output is like this:

Unexpected failures (9)
    /WebKit2Gtk/TestSSL
        /webkit2/WebKitWebView/tls-subresource
        /webkit2/WebKitWebView/load-failed-with-tls-errors
        /webkit2/WebKitWebView/ssl
    /WebKit2Gtk/TestAutomationSession
        /webkit2/WebKitAutomationSession/request-session
    /WTF/TestWTF
        WTF_DateMath.calculateLocalTimeOffset
    /WebKit2Gtk/TestWebKitAccessibility
        /webkit2/WebKitAccessibility/atspi-basic-hierarchy
    /WebKit2Gtk/TestLoaderClient
        /webkit2/WebKitWebView/title
    /WebKit2Gtk/TestWebExtensions
        /webkit2/WebKitWebExtension/form-controls-associated-signal
    /WebKit2Gtk/TestUIClient
        /webkit2/WebKitWebView/mouse-target

Unexpected crashes (4)
    /WebKit2/TestWebKit2
        WebKit2.InputMethodFilterContextFocusOutDuringOngoingComposition
        WebKit2.InputMethodFilterContextMouseClickDuringOngoingComposition
        WebKit2.MouseMoveAfterCrash
        WebKit2.InputMethodFilterComposeKey

Unexpected timeouts (2)
    /WebKit2Gtk/TestLoaderClient
        /webkit2/WebKitWebView/reload
        /webkit2/WebKitURIRequest/http-headers

[Carlos Garcia Campos]

Created attachment 319364 [details]
Patch

Just fixed some typos in the ChangeLog.

[Carlos Alberto Lopez Perez]

Comment on attachment 319364 [details]
Patch

The information about the test run is much cleaner now. Thanks!
BTW.. I think the buildmaster is not automatically restarted, so you may have to send a mail to admin <at> webkit.org requesting it to get restarted

[Carlos Garcia Campos]

(In reply to Carlos Alberto Lopez Perez from comment #3)
> Comment on attachment 319364 [details]
> Patch
> 
> The information about the test run is much cleaner now. Thanks!
> BTW.. I think the buildmaster is not automatically restarted, so you may
> have to send a mail to admin <at> webkit.org requesting it to get restarted

Thanks! 

Lucas, do you need to manually restart the master when the config file changes? Maybe it's easier if you just cq+ this when you can, and restart the master right after the patch lands?

[Carlos Alberto Lopez Perez]

(In reply to Carlos Garcia Campos from comment #4)
> (In reply to Carlos Alberto Lopez Perez from comment #3)
> > Comment on attachment 319364 [details]
> > Patch
> > 
> > The information about the test run is much cleaner now. Thanks!
> > BTW.. I think the buildmaster is not automatically restarted, so you may
> > have to send a mail to admin <at> webkit.org requesting it to get restarted
> 
> Thanks! 
> 
> Lucas, do you need to manually restart the master when the config file
> changes? Maybe it's easier if you just cq+ this when you can, and restart
> the master right after the patch lands?

I think the sync/copy from the webkit repo to the server where the config runs is done manually. So what I do when doing changes to the master is this:

 - Land the patch
 - Send an email to admin at webkit.org telling that I landing the patch at rxxxx and asking if they can schedule a restart of the master to apply the new config.

My 2 cents

[Carlos Garcia Campos]

Committed r221474: <http://trac.webkit.org/changeset/221474>

[Radar WebKit Bug Importer]

<rdar://problem/34693855>