Bug 171044 - Test262 bot does not go red with failures
Summary: Test262 bot does not go red with failures
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-19 23:36 PDT by Joseph Pecoraro
Modified: 2017-04-20 18:50 PDT (History)
6 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (5.52 KB, patch)
2017-04-20 00:00 PDT, Joseph Pecoraro
aakash_jain: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 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!
Comment 1 Joseph Pecoraro 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).
Comment 2 Joseph Pecoraro 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!
Comment 3 Build Bot 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.
Comment 4 Aakash Jain 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.
Comment 5 Joseph Pecoraro 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.
Comment 6 Joseph Pecoraro 2017-04-20 18:50:18 PDT
<https://trac.webkit.org/changeset/215599/webkit>