[webkit-reviews] review denied: [Bug 76749] run-webkit-tests --lint-test-files should lint all the ports by default : [Attachment 123397] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 76749: run-webkit-tests --lint-test-files should lint all the ports by
default
https://bugs.webkit.org/show_bug.cgi?id=76749

Attachment 123397: Patch
https://bugs.webkit.org/attachment.cgi?id=123397&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=123397&action=review


Can you add a simple test that verifies that we actually iterate through all
the test configurations? Something like
http://trac.webkit.org/changeset/105452.

> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:53
> +	   ports_to_lint = [host.port_factory.get(name) for name in
host.port_factory.all_port_names()]

This is going to be slow. We'll be checking the each Chromium port when we only
need to check one of them, no?

I'm not sure what the right design is here, but it would be good to do some
quick performance testing to see what the impact is once bug 76745 is resolved.


> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:70
> +    # Currently all real ports use the same list of tests and then skip
> +    # tests as desired per platform. So, in order to speed this up, we
> +    # only fetch the list of tests once, instead of once per port as would
> +    # be strictly correct.
> +    # FIXME(dpranke): Should we cache this list in port/base.py instead?
> +    #
> +    # The exception is the Chromium GPU tests, but
> +    # (a) they're going away, and (b) they're a subset of the Chromium
tests,
> +    # so it should be safe to reuse the larger list with them.
> +    printer.print_update("Collecting tests ...")
> +    try:
> +	   tests = port.tests([])
> +    except IOError, e:
> +	   if e.errno == errno.ENOENT:
> +	       return -1
> +	   raise

I don't think we need to pass the list of tests at all.

> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:73
> +    for port_to_lint in ports_to_lint:

Port<-->test_configuration is a 1:N mapping. You need to call
all_test_configurations() for each port.