Bug 36417

Summary: fix dangling references to chromium directories and baseline paths
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: Tools / TestsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, ojan, victorw
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch none

Dirk Pranke
Reported 2010-03-20 12:06:37 PDT
There are a few spots in the layout test infrastructure that still refer to downstream directories that are now obsolete. Remove those. Note that the overrides path is *not* one of them.
Attachments
Patch (2.53 KB, patch)
2010-03-20 12:07 PDT, Dirk Pranke
no flags
Patch (4.53 KB, patch)
2010-03-22 14:36 PDT, Dirk Pranke
no flags
Dirk Pranke
Comment 1 2010-03-20 12:07:20 PDT
Dimitri Glazkov (Google)
Comment 2 2010-03-20 12:12:11 PDT
Comment on attachment 51227 [details] Patch excellent! Should we also get rid of _chromium_baseline_path?
Dirk Pranke
Comment 3 2010-03-20 14:02:37 PDT
(In reply to comment #2) > (From update of attachment 51227 [details]) > excellent! > > Should we also get rid of _chromium_baseline_path? Eventually, but I don't want to get rid of it just yet. I also don't want to commit this until we get a chance to discuss http://trac.webkit.org/changeset/56307 It looks like you made a similar change to fix the rebaselining tool, but I don't understand why it would've fixed your crash.
Dirk Pranke
Comment 4 2010-03-22 14:36:39 PDT
Dimitri Glazkov (Google)
Comment 5 2010-03-22 14:37:51 PDT
Comment on attachment 51355 [details] Patch yay!
Dirk Pranke
Comment 6 2010-03-22 14:40:22 PDT
(In reply to comment #3) > I also don't want to commit this until we get a chance to discuss > > http://trac.webkit.org/changeset/56307 > > It looks like you made a similar change to fix the rebaselining tool, but I > don't understand why it would've fixed your crash. Okay, it turns out that when we tried to determine the chrome base directory, if we didn't find it, the code would return a full file path to chromium.py instead of erroring out. The revised patch will instead raise an error in this case. I've also added some better path handling to rebaseline-chromium-webkit-tests so it would work if simplejson wasn't in the PYTHONPATH by default. Ojan, Victor, can you take a look at this and make sure it looks okay?
Victor Wang
Comment 7 2010-03-22 14:55:20 PDT
LGTM (In reply to comment #6) > (In reply to comment #3) > > I also don't want to commit this until we get a chance to discuss > > > > http://trac.webkit.org/changeset/56307 > > > > It looks like you made a similar change to fix the rebaselining tool, but I > > don't understand why it would've fixed your crash. > > Okay, it turns out that when we tried to determine the chrome base directory, > if we didn't find it, the code would return a full file path to chromium.py > instead of erroring out. The revised patch will instead raise > an error in this case. > > I've also added some better path handling to rebaseline-chromium-webkit-tests > so it would work if simplejson wasn't in the PYTHONPATH by default. > > Ojan, Victor, can you take a look at this and make sure it looks okay?
Dirk Pranke
Comment 8 2010-03-22 16:04:09 PDT
Comment on attachment 51355 [details] Patch Clearing flags on attachment: 51355 Committed r56366: <http://trac.webkit.org/changeset/56366>
Dirk Pranke
Comment 9 2010-03-22 16:04:13 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.