RESOLVED FIXED 171044
Test262 bot does not go red with failures 
https://bugs.webkit.org/show_bug.cgi?id=171044
Summary Test262 bot does not go red with failures
Joseph Pecoraro
Reported 2017-04-19 23:36:12 PDT
Summary: Test262 bot does not go red with failures Case: https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20Test262%20%28Tests%29/builds/265 Output: ... Running test262.yaml/test262/test/annexB/language/eval-code/indirect/global-block-decl-eval-global-exsting-fn-update.js.default Running test262.yaml/test262/test/annexB/language/eval-code/indirect/global-block-decl-eval-global-exsting-global-init.js.default test262.yaml/test262/test/annexB/language/eval-code/direct/var-env-lower-lex-catch-non-strict.js.default: Exception: SyntaxError: Can't create duplicate variable in eval: 'err' test262.yaml/test262/test/annexB/language/eval-code/direct/var-env-lower-lex-catch-non-strict.js.default: at var-env-lower-lex-catch-non-strict.js:28 test262.yaml/test262/test/annexB/language/eval-code/direct/var-env-lower-lex-catch-non-strict.js.default: eval@[native code] test262.yaml/test262/test/annexB/language/eval-code/direct/var-env-lower-lex-catch-non-strict.js.default: global code@var-env-lower-lex-catch-non-strict.js:28:7 Running test262.yaml/test262/test/annexB/language/eval-code/indirect/global-block-decl-eval-global-exsting-global-update.js.default Running test262.yaml/test262/test/annexB/language/eval-code/indirect/global-block-decl-eval-global-exsting-var-no-init.js.default Running test262.yaml/test262/test/annexB/language/eval-code/indirect/global-block-decl-eval-global-exsting-var-update.js.default test262.yaml/test262/test/annexB/language/eval-code/direct/var-env-lower-lex-catch-non-strict.js.default: ERROR: Unexpected exit code: 3 Running test262.yaml/test262/test/annexB/language/eval-code/indirect/global-block-decl-eval-global-init.js.default FAIL: test262.yaml/test262/test/annexB/language/eval-code/direct/var-env-lower-lex-catch-non-strict.js.default Running test262.yaml/test262/test/annexB/language/eval-code/indirect/global-block-decl-eval-global-no-skip-try.js.default ... Notes: • It appears that when run-jsc-stress-tests output is piped it produces different output: - Running: <test> - Running: <test> - FAIL: <test> So we should change the Test bot to check for this output instead of the output when it is not piped!
Attachments
[PATCH] Proposed Fix (5.52 KB, patch)
2017-04-20 00:00 PDT, Joseph Pecoraro 		aakash_jain: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Joseph Pecoraro
Comment 1 2017-04-19 23:43:22 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).
Joseph Pecoraro
Comment 2 2017-04-20 00:00:14 PDT
Created attachment 307566 [details] [PATCH] Proposed Fix Again, I don't have the means to test this on bots, but I'm hopeful!
Build Bot
Comment 3 2017-04-20 00:01:16 PDT
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.
Aakash Jain
Comment 4 2017-04-20 14:59:37 PDT
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.
Joseph Pecoraro
Comment 5 2017-04-20 15:01:09 PDT
(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.
Joseph Pecoraro
Comment 6 2017-04-20 18:50:18 PDT
<https://trac.webkit.org/changeset/215599/webkit>
Note You need to log in before you can comment on or make changes to this bug.