Bug 171044 – Test262 bot does not go red with failures

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Joseph Pecoraro]

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 [e-mail]: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!

[Joseph Pecoraro]

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]

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]

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

(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]

<https://trac.webkit.org/changeset/215599/webkit>