Bug 90555 – [NRWT] Compare results between different platforms

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Kristóf Kosztyó]

It would be useful for gardening if we could compare our results to different ports.
Now we use the orwt for this because the port detection is very simple: e.g. if we pass --platform mac this will set the baselines for the mac port and run the tests with the already built drt.

[Dirk Pranke]

I'm not sure I understand what you're asking for. Do you want to be able to run the Qt port's binary and compare against the apple mac baselines? Or something else?

[Csaba Osztrogonác]

(In reply to comment #1)
> I'm not sure I understand what you're asking for. Do you want to be able to run the Qt port's binary and compare against the apple mac baselines? Or something else?

Exactly. Comparing our results for to Mac/GTK/Chromium can be useful. For
example for new tests, tests were skipped long time, when results changed
at night and other ports updated their results before we get up :), ...
We usually do it with ORWT long time ago and it is really useful. ;-)

Kristóf, as far as I know, you started working on this bug.
Could you submit a preliminary patch?

[Kristóf Kosztyó]

Created attachment 152048 [details]
work in progress

Yes, as Ossy said we would like to test with our binary and use the other ports baselines for check the results. I think the baselineoptimizer give the most general approach because it knows almost every ports baseline paths.

[Dirk Pranke]

Comment on attachment 152048 [details]
work in progress

Okay, if this feature is useful to you I have no argument against it (although I wonder if it would be better to have a separate tool that compared the results, or integrated this into results.html or something).

A few comments on the approach, though.

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

> Tools/Scripts/webkitpy/layout_tests/port/base.py:366
> +        baseline_search_path = self.get_option('additional_platform_directory', []) + compare_baseline(self) + self.baseline_search_path()

1) It's a bug that we are creating a "baseline_search_path" different from what baseline_search_path() actually returns. There's two different uses of the baseline path here ... the one the port wants, and the one the user wants, which is the port's path + --additional-platform-directory + (now) --compare-port's path. 

We should rename baseline_search_path (the port's version) to default_baseline_search_path(), and move this line into baseline_search_path(). That way callers get a single consistent view.

2) I don't think going through baseline optimizer buys you anything. You should be able to call host.port_factory.get(self.get_option('compare_port')).default_baseline_search_path() to get the same result, without the dependency, except ...

3) Port isn't allowed to depend on Host, just SystemHost, so you can't assume that host.port_factory exists. You need to create a PortFactory() and pass it the host. This should work in most (if not all) cases, but could cause weirdness if a caller is using a custom/mocked port factory object.

> Tools/Scripts/webkitpy/layout_tests/port/compare.py:1
> +# Copyright (C) 2010 Google Inc. All rights reserved.

This copyright claim is probably wrong :).

> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:327
> +            help="Compare the results with the specified port."),

I'm not sure about the command line flag name here, but I haven't thought of anything that is grammatically much better. I might at least change the help text to "Use the specified port's baselines first".

[Kristóf Kosztyó]

Created attachment 155539 [details]
proposed fix

[Dirk Pranke]

Comment on attachment 155539 [details]
proposed fix

sorry, this must've slipped through the cracks ... can you just add compare_baseline() into the Port class? I can't see an advantage to having another file for one 6-line function :) Looks fine otherwise.

[Kristóf Kosztyó]

Created attachment 157721 [details]
proposed fix

[Dirk Pranke]

Comment on attachment 157721 [details]
proposed fix

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

looks great! thanks!

> Tools/Scripts/webkitpy/layout_tests/port/base.py:216
> +    def compare_baseline(self):

minor nit: I might name this _compare_baseline() since I don't want this to be public ...

[Kristóf Kosztyó]

Committed r125524: <http://trac.webkit.org/changeset/125524>