Bug 105619 – Remove old-run-webkit-tests support from webkitpy

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Eric Seidel (no email)]

Remove old-run-webkit-tests support from webkitpy

It's just confusing things.  Only Apple Win still uses old-run-webkit-tests, and the Apple-Win EWS does not run tests.

I believe partially-broken NRWT support is at the root of bug 103839, in part because NRWT is currently a second-class "experimental" citizen in the EWS code.  Since NRWT has now long been the default, we should make it a real boy and kill ORWT (since I'm very likely to break it by re-working this code anyway).

At least the following tools will break:
1. sheriff-bot, webkit-patch will no longer understand ORWT results files (from Apple-Win buildbots, etc.)
2. webkit-patch build-and-test (something no one uses) will not work on AppleWin
3. The AppleWin EWS cannot graduate to a testing-EWS w/o moving to NRWT (that's probably for the best, given how impossibly slow it is already).

There is no urgency to this, but again, I think it's likely I may break ORWT support in webkitpy by accident as part of fixing bug 103839.

[Eric Seidel (no email)]

I should note that this will *not* affect the buildbots, or those calling run-webkit-tests directly.  Only webkit-patch (and associated commands) which currently know how to run old-run-webkit-tests or interpret its results (as I've listed above).

[Dirk Pranke]

We could also investigate modifying ORWT to output NRWT-compatible stuff. I don't know if that's more or less work than finishing getting NRWT running on win.

[Adam Barth]

Created attachment 181044 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 181044 [details]
Patch

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

I suspect there is more than this.  :)  But this is a start.

> Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py:88
>          return LayoutTestResults.results_from_string(results_file)

This could call results_from_json directly now, I think.

> Tools/Scripts/webkitpy/common/net/layouttestresults.py:63
>      # FIXME: run-webkit-tests should store the --exit-after-N-failures value

This seems like this would be trivial for one of us to fix.  I'm kinda surprised this is still here. :(

> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:-182
> -            # This differs from ORWT because it does not include WebProcess crashes.

I would leave this in?  I'm not sure if WebKit2 folks care about this or not.

[Adam Barth]

(In reply to comment #4)
> (From update of attachment 181044 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=181044&action=review
> 
> I suspect there is more than this.  :)  But this is a start.
> 
> > Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py:88
> >          return LayoutTestResults.results_from_string(results_file)
> 
> This could call results_from_json directly now, I think.

Searching 1977 files for "results_from_json"

0 matches across 0 files

If you mean ResultsJSONParser.parse_results_json, we could but we'd still need to construct a LayoutTestResults interface to wrap the results.

> > Tools/Scripts/webkitpy/common/net/layouttestresults.py:63
> >      # FIXME: run-webkit-tests should store the --exit-after-N-failures value
> 
> This seems like this would be trivial for one of us to fix.  I'm kinda surprised this is still here. :(

Yeah.  I think it might actually be in the JSON already.

> > Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_runner.py:-182
> > -            # This differs from ORWT because it does not include WebProcess crashes.
> 
> I would leave this in?  I'm not sure if WebKit2 folks care about this or not.

Fixed.

[Adam Barth]

Created attachment 181045 [details]
Patch for landing

[Eric Seidel (no email)]

Thank you.  Related bug 103839 is on my short-list for next week.  (Although you're welcome to tackle it before then if you are in the mood.)

[WebKit Review Bot]

Comment on attachment 181045 [details]
Patch for landing

Clearing flags on attachment: 181045

Committed r138634: <http://trac.webkit.org/changeset/138634>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.