Bug 36417 - fix dangling references to chromium directories and baseline paths
Summary: fix dangling references to chromium directories and baseline paths
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: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-20 12:06 PDT by Dirk Pranke
Modified: 2010-03-22 16:04 PDT (History)
3 users (show)

See Also:


Attachments
Patch (2.53 KB, patch)
2010-03-20 12:07 PDT, Dirk Pranke
no flags Details | Formatted Diff | Diff
Patch (4.53 KB, patch)
2010-03-22 14:36 PDT, Dirk Pranke
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 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.
Comment 1 Dirk Pranke 2010-03-20 12:07:20 PDT
Created attachment 51227 [details]
Patch
Comment 2 Dimitri Glazkov (Google) 2010-03-20 12:12:11 PDT
Comment on attachment 51227 [details]
Patch

excellent!

Should we also get rid of _chromium_baseline_path?
Comment 3 Dirk Pranke 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.
Comment 4 Dirk Pranke 2010-03-22 14:36:39 PDT
Created attachment 51355 [details]
Patch
Comment 5 Dimitri Glazkov (Google) 2010-03-22 14:37:51 PDT
Comment on attachment 51355 [details]
Patch

yay!
Comment 6 Dirk Pranke 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?
Comment 7 Victor Wang 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?
Comment 8 Dirk Pranke 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>
Comment 9 Dirk Pranke 2010-03-22 16:04:13 PDT
All reviewed patches have been landed.  Closing bug.