RESOLVED FIXED 176104
[GTK] Improve the way unit test are run and the results reported
https://bugs.webkit.org/show_bug.cgi?id=176104
Summary [GTK] Improve the way unit test are run and the results reported
Carlos Garcia Campos
Reported 2017-08-30 08:29:38 PDT
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.
Attachments
Patch (17.78 KB, patch)
2017-08-30 08:40 PDT, Carlos Garcia Campos
no flags
Patch (17.78 KB, patch)
2017-08-30 08:50 PDT, Carlos Garcia Campos
clopez: review+
Carlos Garcia Campos
Comment 1 2017-08-30 08:40:58 PDT
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
Comment 2 2017-08-30 08:50:16 PDT
Created attachment 319364 [details] Patch Just fixed some typos in the ChangeLog.
Carlos Alberto Lopez Perez
Comment 3 2017-08-31 07:22:48 PDT
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
Comment 4 2017-08-31 10:01:55 PDT
(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
Comment 5 2017-08-31 10:07:17 PDT
(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
Comment 6 2017-08-31 23:36:42 PDT
Radar WebKit Bug Importer
Comment 7 2017-09-27 12:44:13 PDT
Note You need to log in before you can comment on or make changes to this bug.