Bug 76905

Summary:

[Qt] run-qtwebkit-tests should report crashes

Product:

WebKit

Reporter:

Csaba Osztrogonác <ossy>

Component:

Tools / Tests

Assignee:

Ádám Kallai <kadam>

Status:

RESOLVED FIXED

Severity:

Critical

CC:

jturcotte, ossy

Priority:

Keywords:

Qt, QtTriaged

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
draft of solution	none
proposed patch	none
proposed patch	none
proposed patch	none

Csaba Osztrogonác

Reported 2012-01-24 04:48:06 PST

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 ** **********************************************************************

Attachments
draft of solution (2.64 KB, patch) 2012-02-21 07:35 PST, Ádám Kallai	no flags	Details Formatted Diff Diff
proposed patch (4.24 KB, patch) 2012-02-24 00:17 PST, Ádám Kallai	no flags	Details Formatted Diff Diff
proposed patch (10.58 KB, patch) 2012-02-28 03:23 PST, Ádám Kallai	no flags	Details Formatted Diff Diff
proposed patch (10.59 KB, patch) 2012-02-28 04:13 PST, Ádám Kallai	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Ádám Kallai

Comment 1 2012-02-21 07:35:24 PST

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

Comment 2 2012-02-24 00:17:04 PST

Created attachment 128671 [details] proposed patch I added my modification to convert_to_html function too.

Ádám Kallai

Comment 3 2012-02-28 03:23:34 PST

Created attachment 129222 [details] proposed patch I have finished the patch. Made unit test for this modification.

Csaba Osztrogonác

Comment 4 2012-02-28 03:28:51 PST

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

Comment 5 2012-02-28 04:13:53 PST

Created attachment 129228 [details] proposed patch Thanks for comment. It was there accidentally. It has been fixed.

Csaba Osztrogonác

Comment 6 2012-03-28 04:47:50 PDT

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

Comment 7 2012-03-28 05:18:34 PDT

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

Note You need to log in before you can comment on or make changes to this bug.