Bug 142938 – Add --allowed-host support to run-webkit-tests

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

Add --allowed-host support to run-webkit-tests

[Jer Noble]

Created attachment 249173 [details]
Patch

[WebKit Commit Bot]

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.

[Jer Noble]

Created attachment 249420 [details]
Patch

[WebKit Commit Bot]

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.

[Brent Fulgham]

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?

[Jer Noble]

(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.

[Jer Noble]

Comment on attachment 249420 [details]
Patch

Clearing flags on attachment: 249420

Committed r182018: <http://trac.webkit.org/changeset/182018>

[Jer Noble]

All reviewed patches have been landed.  Closing bug.