Bug 60086

Summary: rebaseline_chromium_webkit_tests is not generating output diffs
Product: WebKit Reporter: Steve Lacey <sjl>
Component: Tools / TestsAssignee: Steve Lacey <sjl>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, dpranke, mihaip, ojan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch none

Steve Lacey
Reported 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.
Attachments
Patch (4.39 KB, patch)
2011-05-04 09:42 PDT, Steve Lacey 		no flags
DetailsFormatted DiffDiff
Patch (12.30 KB, patch)
2011-05-04 23:04 PDT, Steve Lacey 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Steve Lacey
Comment 1 2011-05-04 09:42:54 PDT
Created attachment 92260 [details] Patch
Steve Lacey
Comment 2 2011-05-04 09:43:40 PDT
Fixed. Adam - could you take a look? Thx.
Dirk Pranke
Comment 3 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.
Steve Lacey
Comment 4 2011-05-04 23:04:14 PDT
Created attachment 92378 [details] Patch
Steve Lacey
Comment 5 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...)
Dirk Pranke
Comment 6 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.
WebKit Commit Bot
Comment 7 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.
WebKit Commit Bot
Comment 8 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>
WebKit Commit Bot
Comment 9 2011-05-05 17:28:36 PDT
All reviewed patches have been landed. Closing bug.
WebKit Commit Bot
Comment 10 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.
Eric Seidel (no email)
Comment 11 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).
Note You need to log in before you can comment on or make changes to this bug.