Bug 142938

Summary: Add --allowed-host support to run-webkit-tests
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, glenn
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 142931    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Description Jer Noble 2015-03-21 09:00:12 PDT
Add --allowed-host support to run-webkit-tests
Comment 1 Jer Noble 2015-03-21 09:08:35 PDT
Created attachment 249173 [details]
Patch
Comment 2 WebKit Commit Bot 2015-03-21 09:11:03 PDT
Attachment 249173 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:324:  whitespace before ']'  [pep8/E202] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Jer Noble 2015-03-25 11:56:53 PDT
Created attachment 249420 [details]
Patch
Comment 4 WebKit Commit Bot 2015-03-25 11:58:52 PDT
Attachment 249420 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:324:  whitespace before ']'  [pep8/E202] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Brent Fulgham 2015-03-25 13:02:35 PDT
Comment on attachment 249420 [details]
Patch

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

Looks good. You might be able to simplify the "options.layout_tests_dir" change, but it's fine to leave as-is.

> Tools/Scripts/webkitpy/port/base.py:141
> +        self._layout_tests_dir = hasattr(options, 'layout_tests_dir') and options.layout_tests_dir and self._filesystem.abspath(options.layout_tests_dir)

Would this work with just "hasattr(options, 'layout_tests_dir') and self._filesystem.abspath(options.layout_tests_dir)"? I'm not sure what leaving the "options.layout_tests_dir" gets us. If it's "", we should get the same thing out of self._filesystem.abspath, right?
Comment 6 Jer Noble 2015-03-25 13:03:59 PDT
(In reply to comment #5)
> Comment on attachment 249420 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249420&action=review
> 
> Looks good. You might be able to simplify the "options.layout_tests_dir"
> change, but it's fine to leave as-is.
> 
> > Tools/Scripts/webkitpy/port/base.py:141
> > +        self._layout_tests_dir = hasattr(options, 'layout_tests_dir') and options.layout_tests_dir and self._filesystem.abspath(options.layout_tests_dir)
> 
> Would this work with just "hasattr(options, 'layout_tests_dir') and
> self._filesystem.abspath(options.layout_tests_dir)"? I'm not sure what
> leaving the "options.layout_tests_dir" gets us. If it's "", we should get
> the same thing out of self._filesystem.abspath, right?

I had that originally, but abspath() will throw if you pass in None, which is the default. So hasattr() will return true, the getter will return None, and abspath() will throw.

So that's why the extra boolean check is there.
Comment 7 Jer Noble 2015-03-26 11:16:47 PDT
Comment on attachment 249420 [details]
Patch

Clearing flags on attachment: 249420

Committed r182018: <http://trac.webkit.org/changeset/182018>
Comment 8 Jer Noble 2015-03-26 11:16:51 PDT
All reviewed patches have been landed.  Closing bug.