Summary: | Test262 bot does not go red with failures | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
Component: | Tools / Tests | Assignee: | Joseph Pecoraro <joepeck> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | aakash_jain, ap, buildbot, joepeck, lforschler, saam | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Joseph Pecoraro
2017-04-19 23:36:12 PDT
Also note that from where run-jsc-stress-tests is run, there is a results directory which produces a "failures" file with all of the tests that failed. Presumably that could be used to check for failures instead of parsing the entire content (loading into memory is probably a lot of memory because it contains a string for each of the thousands of tests). Created attachment 307566 [details]
[PATCH] Proposed Fix
Again, I don't have the means to test this on bots, but I'm hopeful!
Attachment 307566 [details] did not pass style-queue:
ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:155: [RunTest262TestsTest.test_no_regressions_output] Undefined variable 'SUCCESS' [pylint/E0602] [5]
ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:165: [RunTest262TestsTest.test_failure_output] Undefined variable 'FAILURE' [pylint/E0602] [5]
ERROR: Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:177: [RunTest262TestsTest.test_failures_output] Undefined variable 'FAILURE' [pylint/E0602] [5]
Total errors found: 3 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 307566 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=307566&action=review looks good to me. minor comments inline. > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:155 > + self.assertResults(SUCCESS, ["test262-test"], 0, """Using the following jsc path: /Volumes/Data/slave/elcapitan-release-tests-test262/build/WebKitBuild/Release/jsc do we need to have the complete path having 'elcapitan-release-tests' for unit-tests? previous path seems fine to me. > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:165 > + self.assertResults(FAILURE, ["1 Test262 test failed"], 0, """Using the following jsc path: /Volumes/Data/slave/elcapitan-release-tests-test262/build/WebKitBuild/Release/jsc Ditto. > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:171 > +Running test262.yaml/test262/test/annexB/built-ins/Date/prototype/getYear/length.js.default-strict Do we need to have 4 lines for passing test-cases? probably just 1 would be enough, since we don't use them anyways. > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:177 > + self.assertResults(FAILURE, ["2 Test262 tests failed"], 0, """Using the following jsc path: /Volumes/Data/slave/elcapitan-release-tests-test262/build/WebKitBuild/Release/jsc Ditto. > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:182 > +Running test262.yaml/test262/test/annexB/built-ins/Date/prototype/getYear/length.js.default Ditto. Do we need to have 3 lines for passing test-cases? probably just 1 line would be enough. (In reply to Aakash Jain from comment #4) > Comment on attachment 307566 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=307566&action=review > > looks good to me. minor comments inline. > > > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:155 > > + self.assertResults(SUCCESS, ["test262-test"], 0, """Using the following jsc path: /Volumes/Data/slave/elcapitan-release-tests-test262/build/WebKitBuild/Release/jsc > > do we need to have the complete path having 'elcapitan-release-tests' for > unit-tests? previous path seems fine to me. > > > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:165 > > + self.assertResults(FAILURE, ["1 Test262 test failed"], 0, """Using the following jsc path: /Volumes/Data/slave/elcapitan-release-tests-test262/build/WebKitBuild/Release/jsc > > Ditto. > > > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:171 > > +Running test262.yaml/test262/test/annexB/built-ins/Date/prototype/getYear/length.js.default-strict > > Do we need to have 4 lines for passing test-cases? probably just 1 would be > enough, since we don't use them anyways. > > > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:177 > > + self.assertResults(FAILURE, ["2 Test262 tests failed"], 0, """Using the following jsc path: /Volumes/Data/slave/elcapitan-release-tests-test262/build/WebKitBuild/Release/jsc > > Ditto. > > > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:182 > > +Running test262.yaml/test262/test/annexB/built-ins/Date/prototype/getYear/length.js.default > > Ditto. Do we need to have 3 lines for passing test-cases? probably just 1 > line would be enough. The intent was to make the output as realistic as possible, and here I just pulled real output from a bot and modified it. But yes, I can simplify it. |