[webkit-reviews] review granted: [Bug 171044] Test262 bot does not go red with failures : [Attachment 307566] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Aakash Jain <aakash_jain at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 171044: Test262 bot does not go red with failures
https://bugs.webkit.org/show_bug.cgi?id=171044

Attachment 307566: [PATCH] Proposed Fix

https://bugs.webkit.org/attachment.cgi?id=307566&action=review




--- Comment #4 from Aakash Jain <aakash_jain at apple.com> ---
Comment on attachment 307566
  --> https://bugs.webkit.org/attachment.cgi?id=307566
[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/j
sc

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/j
sc

Ditto.

> Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:171
> +Running
test262.yaml/test262/test/annexB/built-ins/Date/prototype/getYear/length.js.def
ault-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/j
sc

Ditto.

> Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:182
> +Running
test262.yaml/test262/test/annexB/built-ins/Date/prototype/getYear/length.js.def
ault

Ditto. Do we need to have 3 lines for passing test-cases? probably just 1 line
would be enough.