[webkit-reviews] review denied: [Bug 220025] LayoutTestFinder should be in charge of finding layout tests : [Attachment 416550] Patch

[bugzilla-daemon at webkit.org]

Jonathan Bedard <jbedard at apple.com> has denied Sam Sneddon [:gsnedders]
<gsnedders at apple.com>'s request for review:
Bug 220025: LayoutTestFinder should be in charge of finding layout tests
https://bugs.webkit.org/show_bug.cgi?id=220025

Attachment 416550: Patch

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




--- Comment #2 from Jonathan Bedard <jbedard at apple.com> ---
Comment on attachment 416550
  --> https://bugs.webkit.org/attachment.cgi?id=416550
Patch

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

This patch needs a changelog.

I also think that while it's going the right direction, we probably want to
take this one piece at a time.nFor example, it seems like the removal of
mock_drt is needed for the change, but not strictly part of the change. I also
think we've missed some fundemental building blocks of layout test discovery
that are still owned by base.py after this patch, such as Base.test_key

> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_finder.py:51
> +    if ext not in Port._supported_reference_extensions:

Code like this makes me think that this patch doesn't finish what it intends to
do. It seems bad to separate supported_test_extensions from their reference
files (and it seems bad that both of them are in base.py)