Bug 48744 – REGRESSION: rebaseline-chromium-webkit-tests uses non-zero tolerance for image dup detection

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

[Kent Tamura]

For example, editing/selection/transformed-selection-rects.html is failing as IMAGE on Mac, and layout-test-results.zip of the canary bot contains .png and .checksum for it.
Then, add "REBASELINE" to the line for it in test_expectations.txt and run the following:
 % rebaseline-chromium-webkit-tests -w

WIN and LINUX results are added to the local working copy, but MAC results are not added.

rebaseline-chromium-webkit-tests uses a port object for Mac (not Chromium-Mac), and port.diff_image() for Mac uses the default tolerance value 0.1.
]

[Kent Tamura]

Created attachment 72493 [details]
Patch

[Mihai Parparita]

Comment on attachment 72493 [details]
Patch

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

Sorry about the regression.

> WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:881
> +        options.tolerance = 0

Any reason why you wouldn't always set this? Before r70279, this tool was always setting tolerance to 0, which seems like the right thing to do.

[Kent Tamura]

(In reply to comment #2)
> > WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:881
> > +        options.tolerance = 0
> 
> Any reason why you wouldn't always set this? Before r70279, this tool was always setting tolerance to 0, which seems like the right thing to do.

Because It seems rebaseline-chromium-webkit-tests has code to support non-Chromium port targets.  I don't know if it works.

[Mihai Parparita]

(In reply to comment #3)
> Because It seems rebaseline-chromium-webkit-tests has code to support non-Chromium port targets.  I don't know if it works.

Ports that don't care about the tolerance setting will just ignore the field in the options object. new-run-webkit-tests always sets tolerance, it's up to each port to do something with it (only the Mac port respects it), and that hasn't been an issue.

[Kent Tamura]

(In reply to comment #4)
> (In reply to comment #3)
> > Because It seems rebaseline-chromium-webkit-tests has code to support non-Chromium port targets.  I don't know if it works.
> 
> Ports that don't care about the tolerance setting will just ignore the field in the options object. new-run-webkit-tests always sets tolerance, it's up to each port to do something with it (only the Mac port respects it), and that hasn't been an issue.

Ah, right.
So, the ideal fix is that ImageDiff of the host port runs with the tolerance value of the target port?

[Dirk Pranke]

Good catch. I agree with Mihai that we should use a tolerance of 0 across the board.

[Dirk Pranke]

Meaning, I wouldn't even make it a command line option, just set it to zero in the call.

[Kent Tamura]

Created attachment 72585 [details]
Patch 2 (always tolerance=0)

[Kent Tamura]

Would some review this please?

[Kent Tamura]

(In reply to comment #9)
> Would some review this please?
                someone

[Adam Barth]

Our goal here is to match the HTML5 spec.  How does a WebKit nightly compare to Firefox 4 beta?

[Adam Barth]

wrong bug.  :)

[Dimitri Glazkov (Google)]

Comment on attachment 72585 [details]
Patch 2 (always tolerance=0)

ok

[Kent Tamura]

Comment on attachment 72585 [details]
Patch 2 (always tolerance=0)

Clearing flags on attachment: 72585

Committed r71228: <http://trac.webkit.org/changeset/71228>

[Kent Tamura]

All reviewed patches have been landed.  Closing bug.