Summary: | rebaseline_chromium_webkit_tests is not generating output diffs | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Steve Lacey <sjl> | ||||||
Component: | Tools / Tests | Assignee: | 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
Steve Lacey
2011-05-03 16:58:54 PDT
Created attachment 92260 [details]
Patch
Fixed. Adam - could you take a look? Thx. 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. Created attachment 92378 [details]
Patch
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...) Patch LGTM. Ojan, Tony, Mihai, could one of you review this? It'd be nice to get it landed. 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 on attachment 92378 [details] Patch Clearing flags on attachment: 92378 Committed r85905: <http://trac.webkit.org/changeset/85905> All reviewed patches have been landed. Closing bug. 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 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). |