Bug 76905 – [Qt] run-qtwebkit-tests should report crashes

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Csaba Osztrogonác]

Unfortunately run-qtwebkit-tests has a serious bug. Now it runs each API test binary 
and summarizes its sub-summarys. But if a binary crashes then the sub-summary is missing 
and the result will be absolutely false positive.

run-qtwebkit-tests must report error if an API test crashes.


http://build.webkit.sed.hu/builders/x86-32%20Linux%20Qt%20Release%20WebKit2/builds/19059/steps/API%20tests/logs/stdio
An example simplyfied output of a good test:
---------------------------------------------

INFO:Exec:Finished WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/publicapi/tst_publicapi
INFO:Exec:Finished WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/qquickwebview/tst_qquickwebview
INFO:Exec:Finished WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/qmltests/tst_qmltests
...

********* Start testing of tst_publicapi *********
...
Totals: 3 passed, 0 failed, 0 skipped
********* Finished testing of tst_publicapi *********

********* Start testing of qmltests *********
...
Totals: 86 passed, 0 failed, 0 skipped
********* Finished testing of qmltests *********

********* Start testing of tst_QQuickWebView *********
...
Totals: 20 passed, 0 failed, 0 skipped
********* Finished testing of tst_QQuickWebView *********


**********************************************************************
**             TOTALS: 109 passed, 0 failed, 0 skipped              **
**********************************************************************


http://build.webkit.sed.hu/builders/x86-32%20Linux%20Qt%20Release%20WebKit2/builds/19020/steps/API%20tests/logs/stdio
An example simplyfied output of a bad test:
---------------------------------------------
INFO:Exec:Finished WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/publicapi/tst_publicapi
tst_qmltests: tpp.c:63: __pthread_tpp_change_priority: Assertion `new_prio == -1 || (new_prio >= __sched_fifo_min_prio && new_prio <= __sched_fifo_max_prio)' failed.
INFO:Exec:Finished WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/qmltests/tst_qmltests
INFO:Exec:Finished WebKitBuild/Release/Source/WebKit2/UIProcess/API/qt/tests/qquickwebview/tst_qquickwebview
...
********* Start testing of tst_publicapi *********
...
Totals: 3 passed, 0 failed, 0 skipped
********* Finished testing of tst_publicapi *********

********* Start testing of qmltests *********
Config: Using QTest library 5.0.0, Qt 5.0.0
PASS   : qmltests::DesktopWebViewLinkHovered::initTestCase()
PASS   : qmltests::DesktopWebViewLinkHovered::test_linkHovered()
PASS   : qmltests::DesktopWebViewLinkHovered::test_linkHoveredDoesntEmitRepeated()
PASS   : qmltests::DesktopWebViewLinkHovered::cleanupTestCase()
PASS   : qmltests::DesktopWebViewLoadHtml::initTestCase()
PASS   : qmltests::DesktopWebViewLoadHtml::test_baseUrlAfterLoadHtml()
PASS   : qmltests::DesktopWebViewLoadHtml::cleanupTestCase()
PASS   : qmltests::DesktopWebViewMessaging::initTestCase()

********* Start testing of tst_QQuickWebView *********
...
Totals: 20 passed, 0 failed, 0 skipped
********* Finished testing of tst_QQuickWebView *********


**********************************************************************
**              TOTALS: 23 passed, 0 failed, 0 skipped              **
**********************************************************************

[Ádám Kallai]

Created attachment 127970 [details]
draft of solution

The patch is not complete yet. It's a draft, any kind of help or ideas are appreciated. :)
Added checking of crashing tests. TOTALS now includes the total number of crashing tests too. Missing sub-summary is considered as a crash. Test object with "None" output is now handled. I commented off convert_to_html function, because these modifications are to be made here too. 
I induced the crash with exit(1) in an API test (tst_qquickwebview.cpp) so I could test this.

[Ádám Kallai]

Created attachment 128671 [details]
proposed patch

I added my modification to convert_to_html function too.

[Ádám Kallai]

Created attachment 129222 [details]
proposed patch

I have finished the patch. Made unit test for this modification.

[Csaba Osztrogonác]

Comment on attachment 129222 [details]
proposed patch

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

> Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:87
> +
> +    def assertSummary(self, expected_text, stdio):
> +        rc = 0
> +        cmd = StubRemoteCommand(rc, stdio)
> +        #step = RunQtWebKitTests()
> +

Is this accidentally in the patch?

[Ádám Kallai]

Created attachment 129228 [details]
proposed patch

Thanks for comment. It was there accidentally. It has been fixed.

[Csaba Osztrogonác]

Comment on attachment 129228 [details]
proposed patch

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

r=me with removing accidentally uploaded code.

> Tools/ChangeLog:16
> +        (RunQtAPITestsTest.assertSummary):

Remove it too.

> Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:87
> +
> +    def assertSummary(self, expected_text, stdio):
> +        rc = 0
> +        cmd = StubRemoteCommand(rc, stdio)
> +        #step = RunQtWebKitTests()
> +

We still don't need this code. :)

[Csaba Osztrogonác]

Landed in http://trac.webkit.org/changeset/112383