Bug 220421 - LayoutTestFinder should be in charge of finding expectations for each test
Summary: LayoutTestFinder should be in charge of finding expectations for each test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Sneddon [:gsnedders]
URL:
Keywords: InRadar
Depends on: 221577 222662 222770 224329 224442 232018 232576 235370 268379
Blocks:
  Show dependency treegraph
 
Reported: 2021-01-07 10:22 PST by Sam Sneddon [:gsnedders]
Modified: 2024-03-14 17:26 PDT (History)
5 users (show)

See Also:


Attachments
WIP patch (29.34 KB, patch)
2022-01-19 11:43 PST, Sam Sneddon [:gsnedders]
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Sneddon [:gsnedders] 2021-01-07 10:22:36 PST
Following on from bug 220025, as mentioned in comment 5:

> To summarise what [myself and jbedard] discussed in person, my longer term plan was to migrate all the expectation-finding code into the LayoutTestFinder as well. Currently we build up TestInputs in the manager, add the reference_files to them in layout_test_runner, and then construct a DriverOutput with all the relevant expectations in single_test_runner. This doesn't really make sense to have split across three places.
>
> At the moment, after [bug 220025], finding expectations (references among them) remain with the Port, but that should be changed in a later patch.
Comment 1 Radar WebKit Bug Importer 2021-01-07 10:22:44 PST
<rdar://problem/72894403>
Comment 2 Sam Sneddon [:gsnedders] 2021-02-05 14:34:15 PST
So, there's at least a few complications here:

We're not just wanting to create TestInput in the LayoutTestFinder itself, as these also contain information related to the specific run (e.g., the used timeout, should_dump_jsconsolelog_in_stderr).

Anything related to the specific run should probably just be passed through to the point where we create the port.driver.DriverInput, and most of what's in port.driver.DriverOuput should (perhaps ironically) be in the new TestInput.

Finally, there's a few places where we currently behave like the device_type can depend on the current worker (e.g. `self._port.target_host(self._worker_number).device_type`) and others where we don't (e.g., `self._driver.host.device_type`). I guess we can treat it as the latter everywhere?

I think stage one at this point is to simplify what we store in TestInput and move some of it into DriverInput, so that TestInput moves towards having less complexity in it, to make it easier for us to move where it gets created?
Comment 3 Sam Sneddon [:gsnedders] 2022-01-19 11:43:36 PST
Created attachment 449501 [details]
WIP patch
Comment 4 Sam Sneddon [:gsnedders] 2024-01-30 16:45:42 PST
Pull request: https://github.com/WebKit/WebKit/pull/23546
Comment 5 EWS 2024-03-14 17:26:56 PDT
Committed 276129@main (bee9faa82faa): <https://commits.webkit.org/276129@main>

Reviewed commits have been landed. Closing PR #23546 and removing active labels.