Bug 60086 - rebaseline_chromium_webkit_tests is not generating output diffs
Summary: rebaseline_chromium_webkit_tests is not generating output diffs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Steve Lacey
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-03 16:58 PDT by Steve Lacey
Modified: 2011-05-09 21:00 PDT (History)
6 users (show)

See Also:


Attachments
Patch (4.39 KB, patch)
2011-05-04 09:42 PDT, Steve Lacey
no flags Details | Formatted Diff | Diff
Patch (12.30 KB, patch)
2011-05-04 23:04 PDT, Steve Lacey
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Lacey 2011-05-03 16:58:54 PDT
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.
Comment 1 Steve Lacey 2011-05-04 09:42:54 PDT
Created attachment 92260 [details]
Patch
Comment 2 Steve Lacey 2011-05-04 09:43:40 PDT
Fixed. Adam - could you take a look? Thx.
Comment 3 Dirk Pranke 2011-05-04 16:38:07 PDT
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.
Comment 4 Steve Lacey 2011-05-04 23:04:14 PDT
Created attachment 92378 [details]
Patch
Comment 5 Steve Lacey 2011-05-04 23:05:33 PDT
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...)
Comment 6 Dirk Pranke 2011-05-05 15:37:26 PDT
Patch LGTM. Ojan, Tony, Mihai, could one of you review this? It'd be nice to get it landed.
Comment 7 WebKit Commit Bot 2011-05-05 17:27:05 PDT
The commit-queue encountered the following flaky tests while processing attachment 92378 [details]:

http/tests/xmlhttprequest/cross-origin-no-authorization.html bug 33357 (author: ap@webkit.org)
The commit-queue is continuing to process your patch.
Comment 8 WebKit Commit Bot 2011-05-05 17:28:31 PDT
Comment on attachment 92378 [details]
Patch

Clearing flags on attachment: 92378

Committed r85905: <http://trac.webkit.org/changeset/85905>
Comment 9 WebKit Commit Bot 2011-05-05 17:28:36 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 WebKit Commit Bot 2011-05-05 18:27:59 PDT
The commit-queue encountered the following flaky tests while processing attachment 92378 [details]:

inspector/debugger/debugger-scripts.html bug 59921 (authors: pfeldman@chromium.org and podivilov@chromium.org)
The commit-queue is continuing to process your patch.
Comment 11 Eric Seidel (no email) 2011-05-09 21:00:26 PDT
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).