Added context manager for chrome and content #2753
Conversation
| @@ -29,6 +29,8 @@ | |||
| import socket | |||
| import sys | |||
|
|
|||
| from contextlib import contextmanager | |||
There was a problem hiding this comment.
Nit: Move this to line 28 so it's with the other standard library imports.
|
This looks fine to me - the Travis CI failures are unrelated. I'd like @AutomatedTester to give feedback though before merging this. |
|
If we want to keep python more in line with java, switching context should be on the 'switch_to' object / class. |
We already have
I'm not too concerned with matching the marionette client. I'm happy with with selenium.context(selenium.CONTEXT_CHROME):
# interact with chrome |
|
This looks fine |
cuff-links
force-pushed
the
Using_Context
branch
from
005295e to
debe7f2
Compare
| capabilities = {'marionette': True} | ||
| self.driver = webdriver.Firefox(capabilities=capabilities) | ||
|
|
||
| def test_using_context_sets_correct_context_and_returns(self): |
There was a problem hiding this comment.
This file needs to move to /py/test/selenium/webdriver/marionette and should now use the driver fixture instead of the setup_method and teardown_method.
There was a problem hiding this comment.
This still needs to be moved to the marionette directory.
| @@ -96,6 +98,7 @@ def __init__(self, firefox_profile=None, firefox_binary=None, | |||
| :param firefox_options: Instance of ``options.Options``. | |||
| :param log_path: Where to log information from the driver. | |||
|
|
|||
| <<<<<<< 6bb01d6fb21694fbd74c24c6cb4529fd4b5536e5 | |||
| self.options = firefox_options or Options() | ||
| self.options.binary_location = self.binary if isinstance(self.binary, basestring) else self.binary._start_cmd | ||
| self.options.profile = self.profile | ||
| capabilities.update(self.options.to_capabilities()) |
There was a problem hiding this comment.
We don't want to bring any of this back in - please remove lines 136-146.
| self.CONTEXT_CONTENT = 'content' | ||
|
|
||
| # marionette | ||
|
|
There was a problem hiding this comment.
Remove this comment and blank line.
| def setup_method(self, method): | ||
|
|
||
| capabilities = {'marionette': True} | ||
| self.driver = webdriver.Firefox(capabilities=capabilities) |
There was a problem hiding this comment.
Remove the setup_method as these tests are no longer based on unittest.TestCase
| try: | ||
| self.driver.quit() | ||
| except: | ||
| pass |
There was a problem hiding this comment.
Remove the teardown_method and instead add driver to your test method signature.
There was a problem hiding this comment.
Just a couple of tweaks. I like the new name of simply context but would appreciate @AutomatedTester's approval before we squash and merge this.
|
|
||
| class TestUsingContext(object): | ||
|
|
||
| def test_using_context_sets_correct_context_and_returns(self, capabilities): |
There was a problem hiding this comment.
Nit: The test name needs updating to reflect the rename from using_context
There was a problem hiding this comment.
You can use the driver fixture instead of capabilities and then your instance will be cleaned up after.
| # specific language governing permissions and limitations | ||
| # under the License. | ||
|
|
||
| from selenium.webdriver import Firefox |
There was a problem hiding this comment.
You won't need this if you use the driver fixture.
|
Would other language bindings support this feature, like java and ruby? |
|
@zzhengjian I don't mind doing a port in Java. I am not that familiar with ruby but I can give it a whirl as well. |
|
@silne30 that'll be great for java. |
|
@davehunt Should I file a new issue for the additional bindings for this? Definitely gonna do Java and ruby is a possiblity. |
|
@silne30 I don't think it's necessary to file an issue, but it won't do any harm. You could just open the pull requests when your patches are ready. |
Xin the preceding checkbox, I verify that I have signed the Contributor License Agreement