Bug 60086 – rebaseline_chromium_webkit_tests is not generating output diffs

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

[Steve Lacey]

The output html file is missing results and also missing image diffs.
This is due to a non-relative path being passed to scn.exists() in _create_html_baseline_tests().

While fixing this I noticed we're not generating image diffs also.

I have a fix in the works. Testing now.

[Steve Lacey]

Created attachment 92260 [details]
Patch

[Steve Lacey]

Fixed. Adam - could you take a look? Thx.

[Dirk Pranke]

Comment on attachment 92260 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:546
> +        baseline_relpath = self._filesystem.relpath(baseline_fullpath, self._filesystem.getcwd())

Argh. It looks like this is another case of us getting bitten by SCM not handling absolute paths. Assuming I'm right, this only works because of the filesystem.chdir() having already occurred on line 393.

Can you move that chdir() and the corresponding # FIXME to somewhere near the beginning of the main() method (around line 968) so that we can make this an explicit invariant that rebaseline-chromium-webkit-tests operates from the top of the webkit tree? 

Then, just delete the second argument to this call (so we can use the default, which is IMO slightly less confusing).

Note that we need to actually do the chdir() rather than just passing in the start dir, because Git is sufficiently broken on the Mac that it is confused by symlinks.

Patch looks fine otherwise.

[Steve Lacey]

Created attachment 92378 [details]
Patch

[Steve Lacey]

Comment on attachment 92260 [details]
Patch

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

>> Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:546
>> +        baseline_relpath = self._filesystem.relpath(baseline_fullpath, self._filesystem.getcwd())
> 
> Argh. It looks like this is another case of us getting bitten by SCM not handling absolute paths. Assuming I'm right, this only works because of the filesystem.chdir() having already occurred on line 393.
> 
> Can you move that chdir() and the corresponding # FIXME to somewhere near the beginning of the main() method (around line 968) so that we can make this an explicit invariant that rebaseline-chromium-webkit-tests operates from the top of the webkit tree? 
> 
> Then, just delete the second argument to this call (so we can use the default, which is IMO slightly less confusing).
> 
> Note that we need to actually do the chdir() rather than just passing in the start dir, because Git is sufficiently broken on the Mac that it is confused by symlinks.
> 
> Patch looks fine otherwise.

All done!

Also updated the unittest (oops...)

[Dirk Pranke]

Patch LGTM. Ojan, Tony, Mihai, could one of you review this? It'd be nice to get it landed.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 92378 [details]:

http/tests/xmlhttprequest/cross-origin-no-authorization.html bug 33357 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 92378 [details]
Patch

Clearing flags on attachment: 92378

Committed r85905: <http://trac.webkit.org/changeset/85905>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 92378 [details]:

inspector/debugger/debugger-scripts.html bug 59921 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[Eric Seidel (no email)]

Comment on attachment 92260 [details]
Patch

Cleared review? from obsolete attachment 92260 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).