Bug 178480 - Add support for webkitpy tests EWS
Summary: Add support for webkitpy tests EWS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-10-18 12:45 PDT by Aakash Jain
Modified: 2020-06-17 09:18 PDT (History)
7 users (show)

See Also:


Attachments
Proposed patch (20.88 KB, patch)
2017-10-19 13:09 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff
Updated patch (28.32 KB, patch)
2017-11-02 15:58 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2017-10-18 12:45:42 PDT
We should have a EWS which runs webkitpy tests. It would help catch webkitpy failures early-on.

See <rdar://problem/18231414>.
Comment 1 Aakash Jain 2017-10-19 13:09:15 PDT
Created attachment 324268 [details]
Proposed patch
Comment 2 Aakash Jain 2017-10-19 13:19:59 PDT
Tested on an staging instance.
Comment 3 Aakash Jain 2017-10-25 14:48:50 PDT
This patch is similar to bindings-test EWS patches, combination of https://bugs.webkit.org/show_bug.cgi?id=168979, https://bugs.webkit.org/show_bug.cgi?id=169063, https://bugs.webkit.org/show_bug.cgi?id=169308
Comment 4 Daniel Bates 2017-10-26 22:48:56 PDT
Comment on attachment 324268 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=324268&action=review

> Tools/Scripts/webkitpy/common/config/ews.json:62
> +    "Webkitpy EWS": {

Webkitpy => webkitpy OR WebKitPy

> Tools/Scripts/webkitpy/common/net/webkitpytestresults.py:30
> +class BindingsTestResults(AbstractTestResults):

Please name the file after this class or vice versa.

> Tools/Scripts/webkitpy/common/net/webkitpytestresults_unittest.py:28
> +class BindingsTestResultsTest(unittest.TestCase):

Ditto.

> Tools/Scripts/webkitpy/tool/bot/webkitpytestresultsreader.py:31
> +class BindingsTestResultsReader(AbstractTestResultsReader):

Ditto.

> Tools/Scripts/webkitpy/tool/bot/webkitpytestresultsreader.py:31
> +class WebkitpyTestResultsReader(AbstractTestResultsReader):

Ditto.

> Tools/Scripts/webkitpy/tool/commands/earlywarningsystem.py:67
> +            self._test_results_reader = WebkitpyTestResultsReader(self._tool, self._port.jsc_results_directory())

We should rename jsc_results_directory() to something more generic since we use this directory for other test results.

> Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:204
> +            'webkitpy-ews',

Please add this to the list in sorted order according to the UNIX sort command.

> Tools/Scripts/webkitpy/tool/steps/runtests.py:84
> +        # FIXME: Remove this code since we now have a separate webkitpy EWS.

We talked about this in person. It does not seem harmful to support running these tests when using webkit-patch interactively.

> Tools/Scripts/webkitpy/tool/steps/runtests.py:187
> +        args.append("--json-output=%s" % results_file_path)

I believe we prefer String.format() over % operator.
Comment 5 Aakash Jain 2017-11-02 15:58:23 PDT
Created attachment 325789 [details]
Updated patch

Review comments incorporated. Removed some code duplication.

Also enabled checking for patch relevance so that webkitpy-ews bubble only shows for relevant patches.

Tested on staging instance.
Comment 6 WebKit Commit Bot 2017-11-07 14:31:00 PST
Comment on attachment 325789 [details]
Updated patch

Clearing flags on attachment: 325789

Committed r224547: <https://trac.webkit.org/changeset/224547>
Comment 7 WebKit Commit Bot 2017-11-07 14:31:02 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Aakash Jain 2017-11-07 17:28:44 PST
Seems to be working fine: https://webkit-queues.webkit.org/queue-status/webkitpy-ews