Bug 49360 - new-run-webkit-tests: respect set-webkit-configuration again
Summary: new-run-webkit-tests: respect set-webkit-configuration again
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: 49361
  Show dependency treegraph
 
Reported: 2010-11-10 19:30 PST by Dirk Pranke
Modified: 2010-11-12 18:01 PST (History)
4 users (show)

See Also:


Attachments
Patch (3.23 KB, patch)
2010-11-10 19:32 PST, Dirk Pranke
no flags Details | Formatted Diff | Diff
Patch (8.29 KB, patch)
2010-11-11 14:32 PST, Dirk Pranke
no flags Details | Formatted Diff | Diff
Patch (10.52 KB, patch)
2010-11-12 17:51 PST, 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-11-10 19:30:46 PST
the refactoring out of the Config object had a typo that caused NRWT to stop respecting set-webkit-configuration again :(
Comment 1 Dirk Pranke 2010-11-10 19:32:03 PST
Created attachment 73571 [details]
Patch
Comment 2 Dirk Pranke 2010-11-11 14:32:41 PST
Created attachment 73665 [details]
Patch
Comment 3 Adam Roben (:aroben) 2010-11-11 14:38:13 PST
Comment on attachment 73665 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=73665&action=review

> WebKitTools/Scripts/webkitpy/layout_tests/port/config_standalone.py:56
> +    else:
> +        e = executive.Executive()
> +        fs = filesystem.FileSystem()

Is this really needed? It's never used.
Comment 4 Dirk Pranke 2010-11-11 14:39:09 PST
(In reply to comment #3)
> (From update of attachment 73665 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=73665&action=review
> 
> > WebKitTools/Scripts/webkitpy/layout_tests/port/config_standalone.py:56
> > +    else:
> > +        e = executive.Executive()
> > +        fs = filesystem.FileSystem()
> 
> Is this really needed? It's never used.

Yeah, I've used it by hand to verify that the actual codepath works, and I'll probably use it in the future when we have "integration tests".
Comment 5 Eric Seidel (no email) 2010-11-11 15:21:12 PST
Comment on attachment 73665 [details]
Patch

Sigh.  We really need to kill that static.  Thanks for fixing.
Comment 6 Dirk Pranke 2010-11-11 15:51:11 PST
Comment on attachment 73665 [details]
Patch

Clearing flags on attachment: 73665

Committed r71858: <http://trac.webkit.org/changeset/71858>
Comment 7 Dirk Pranke 2010-11-11 15:51:15 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Mihai Parparita 2010-11-12 08:31:16 PST
This broke the NaCl tests, which use new-run-webkit-httpd, which also tries to get the configuration (unclear if it needs it):

http://build.chromium.org/p/chromium/builders/NACL%20Tests/builds/1636/steps/nacl_ui_tests/logs/stdio

The breakage is preventing WebKit rolls, so I'm going to roll this out for now.
Comment 9 Mihai Parparita 2010-11-12 08:38:25 PST
(In reply to comment #8)
> The breakage is preventing WebKit rolls, so I'm going to roll this out for now.

Rolled out with http://trac.webkit.org/changeset/71916.
Comment 10 Dirk Pranke 2010-11-12 17:51:30 PST
Created attachment 73801 [details]
Patch
Comment 11 Dirk Pranke 2010-11-12 18:01:03 PST
Comment on attachment 73801 [details]
Patch

Clearing flags on attachment: 73801

Committed r71960: <http://trac.webkit.org/changeset/71960>
Comment 12 Dirk Pranke 2010-11-12 18:01:07 PST
All reviewed patches have been landed.  Closing bug.