Bug 136125

Summary: Fix countFailures of RunLLINTCLoopTests and Run32bitJSCTests
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ggaren, lforschler, msaboff, ossy, rnagy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 136856    
Bug Blocks: 126604, 131054    
Attachments:
Description Flags
Proposed patch
none
Proposed patch none

Csaba Osztrogonác
Reported 2014-08-21 01:33:41 PDT
countFailures of RunLLINTCLoopTests and Run32bitJSCTests in master.cfg don't work at all, they don't report the number of failing tests: http://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/3703 It's hard to determine which patch caused how many regressions if we can't see the exact number on the waterfall. The buggy countFailures introduced in: https://trac.webkit.org/changeset/166608 https://trac.webkit.org/changeset/161461 Why can't we reuse the RunJavaScriptCoreTests class which works fine and its commandComplete covered by unittests too?
Attachments
Proposed patch (5.26 KB, patch)
2014-08-21 04:32 PDT, Nagy Renátó 		no flags
DetailsFormatted DiffDiff
Proposed patch (deleted)
2014-08-21 04:50 PDT, Nagy Renátó 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1 2014-08-21 01:40:06 PDT
I think only the 32 bit countFailures is buggy, because this bot runs the jsc-stress tests, not the old Mozilla tests, and their output are different. But unifying the output parsing of run-javascriptcore-tests would be great.
Csaba Osztrogonác
Comment 2 2014-08-21 03:37:30 PDT
Run32bitJSCTests buildstep runs the JSC stress tests, so its regexp should accept "failure" and "failures", not "regressions". RunLLINTCLoopTests buildstep runs Mozilla tests, so its regexp should accept "regression" and "regressions" too. The command of RunJavaScriptCoreTests, RunLLINTCLoopTests and Run32bitJSCTests are a little bit different, so unifying them isn't trivial. So let's fix the regexps quickly and try to unify them later.
Nagy Renátó
Comment 3 2014-08-21 04:32:17 PDT
Created attachment 236923 [details] Proposed patch Quick fix in regexps at RunLLINTCLoopTests and Run32bitJSCTests class.
WebKit Commit Bot
Comment 4 2014-08-21 04:33:39 PDT
Attachment 236923 [details] did not pass style-queue: ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:148: [RunLLINTCLoopTestsTest.assertResults] Undefined variable 'RunLLINTCLoopTests' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:157: [RunLLINTCLoopTestsTest.test_failures] Undefined variable 'FAILURE' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:160: [RunLLINTCLoopTestsTest.test_failure] Undefined variable 'FAILURE' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:163: [RunLLINTCLoopTestsTest.test_no_failure] Undefined variable 'SUCCESS' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:169: [Run32bitJSCTestsTest.assertResults] Undefined variable 'Run32bitJSCTests' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:178: [Run32bitJSCTestsTest.test_failures] Undefined variable 'FAILURE' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:181: [Run32bitJSCTestsTest.test_failure] Undefined variable 'FAILURE' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:184: [Run32bitJSCTestsTest.test_no_failure] Undefined variable 'SUCCESS' [pylint/E0602] [5] Total errors found: 8 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Csaba Osztrogonác
Comment 5 2014-08-21 04:40:36 PDT
Comment on attachment 236923 [details] Proposed patch LGTM, just a nit: Can we use only single or double quotes?
Nagy Renátó
Comment 6 2014-08-21 04:50:39 PDT
Created attachment 236924 [details] Proposed patch Change all double quote to singe quote.
WebKit Commit Bot
Comment 7 2014-08-21 04:52:29 PDT
Attachment 236924 [details] did not pass style-queue: ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:148: [RunLLINTCLoopTestsTest.assertResults] Undefined variable 'RunLLINTCLoopTests' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:157: [RunLLINTCLoopTestsTest.test_failures] Undefined variable 'FAILURE' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:160: [RunLLINTCLoopTestsTest.test_failure] Undefined variable 'FAILURE' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:163: [RunLLINTCLoopTestsTest.test_no_failure] Undefined variable 'SUCCESS' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:169: [Run32bitJSCTestsTest.assertResults] Undefined variable 'Run32bitJSCTests' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:178: [Run32bitJSCTestsTest.test_failures] Undefined variable 'FAILURE' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:181: [Run32bitJSCTestsTest.test_failure] Undefined variable 'FAILURE' [pylint/E0602] [5] ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:184: [Run32bitJSCTestsTest.test_no_failure] Undefined variable 'SUCCESS' [pylint/E0602] [5] Total errors found: 8 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Commit Bot
Comment 8 2014-08-21 05:29:49 PDT
Comment on attachment 236924 [details] Proposed patch Clearing flags on attachment: 236924 Committed r172829: <http://trac.webkit.org/changeset/172829>
WebKit Commit Bot
Comment 9 2014-08-21 05:29:53 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.