-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding a LaunchApp command to Chrome specific webdriver client. #168
Changes from all commits
d050592
bee8b2b
36a9d76
3744cad
ce6a475
befdb2c
1337b45
7ff729d
9f25326
abc9890
91c18cc
493f166
189c2d8
91da5d9
eea5568
d841066
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,15 +19,20 @@ | |
package org.openqa.selenium.chrome; | ||
|
||
import com.google.common.base.Throwables; | ||
import com.google.common.collect.ImmutableMap; | ||
|
||
import org.openqa.selenium.WebDriverException; | ||
import org.openqa.selenium.chrome.ChromeDriverCommand; | ||
import org.openqa.selenium.remote.Command; | ||
import org.openqa.selenium.remote.DriverCommand; | ||
import org.openqa.selenium.remote.CommandInfo; | ||
import org.openqa.selenium.remote.HttpCommandExecutor; | ||
import org.openqa.selenium.remote.Response; | ||
|
||
import java.io.IOException; | ||
import java.net.ConnectException; | ||
import java.net.URL; | ||
import java.util.Map; | ||
import java.util.HashMap; | ||
|
||
/** | ||
* A specialized {@link HttpCommandExecutor} that will use a {@link ChromeDriverService} that lives | ||
|
@@ -36,19 +41,34 @@ | |
*/ | ||
class ChromeCommandExecutor extends HttpCommandExecutor { | ||
|
||
private static final Map<String, CommandInfo> CHROME_COMMANDS_NAME_TO_URL = ImmutableMap.of( | ||
ChromeDriverCommand.LAUNCH_APP, | ||
post("/session/:sessionId/chromium/launch_app")); | ||
|
||
private final ChromeDriverService service; | ||
|
||
/** | ||
* Creates a new ChromeCommandExecutor which will communicate with the chromedriver as configured | ||
* by the given {@code service}. | ||
* | ||
* | ||
* @param service The ChromeDriverService to send commands to. | ||
*/ | ||
public ChromeCommandExecutor(ChromeDriverService service) { | ||
super(service.getUrl()); | ||
super(CHROME_COMMANDS_NAME_TO_URL, service.getUrl()); | ||
this.service = service; | ||
} | ||
|
||
/** | ||
* Creates a new ChromeCommandExecutor which will communicate with the chromedriver as configured | ||
* by the given {@code service}. | ||
* | ||
* @param serviceUrl The URL of the service to communicate with. | ||
*/ | ||
public ChromeCommandExecutor(URL serviceUrl) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you add this constructor? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this constructor to support the case where the chromedriver server is running independently of the client, and we don't want to manage it here in client code. |
||
super(CHROME_COMMANDS_NAME_TO_URL, serviceUrl); | ||
this.service = null; | ||
} | ||
|
||
/** | ||
* Sends the {@code command} to the chromedriver server for execution. The server will be started | ||
* if requesting a new session. Likewise, if terminating a session, the server will be shutdown | ||
|
@@ -60,7 +80,8 @@ public ChromeCommandExecutor(ChromeDriverService service) { | |
*/ | ||
@Override | ||
public Response execute(Command command) throws IOException { | ||
if (DriverCommand.NEW_SESSION.equals(command.getName())) { | ||
if (ChromeDriverCommand.NEW_SESSION.equals(command.getName()) && | ||
this.service != null) { | ||
service.start(); | ||
} | ||
|
||
|
@@ -76,7 +97,8 @@ public Response execute(Command command) throws IOException { | |
Throwables.propagateIfPossible(t); | ||
throw new WebDriverException(t); | ||
} finally { | ||
if (DriverCommand.QUIT.equals(command.getName())) { | ||
if (ChromeDriverCommand.QUIT.equals(command.getName()) && | ||
this.service != null) { | ||
service.stop(); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
/* | ||
Copyright 2014 Software Freedom Conservancy | ||
|
||
Licensed under the Apache License, Version 2.0 (the "License"); | ||
you may not use this file except in compliance with the License. | ||
You may obtain a copy of the License at | ||
|
||
http://www.apache.org/licenses/LICENSE-2.0 | ||
|
||
Unless required by applicable law or agreed to in writing, software | ||
distributed under the License is distributed on an "AS IS" BASIS, | ||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
See the License for the specific language governing permissions and | ||
limitations under the License. | ||
*/ | ||
|
||
package org.openqa.selenium.chrome; | ||
|
||
import org.openqa.selenium.remote.DriverCommand; | ||
|
||
final class ChromeDriverCommand implements DriverCommand { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really see how this class/enum helps anything, especially while it is is declared with package-private access. I suggest reverting this class and simply using string literal "chromium.launchApp" in one place where it's used. |
||
private ChromeDriverCommand(){} | ||
|
||
static final String LAUNCH_APP = "chromium.launchApp"; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -496,15 +496,15 @@ private Response createResponse(HttpResponse httpResponse, HttpContext context, | |
return response; | ||
} | ||
|
||
private static CommandInfo get(String url) { | ||
protected static CommandInfo get(String url) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest we revert all changes in this file. They are not necessary. Other classes that already create instances of CommandInfo outside of this class simply use CommandInfo constructor(s), we could keep doing the same here. |
||
return new CommandInfo(url, HttpVerb.GET); | ||
} | ||
|
||
private static CommandInfo post(String url) { | ||
protected static CommandInfo post(String url) { | ||
return new CommandInfo(url, HttpVerb.POST); | ||
} | ||
|
||
private static CommandInfo delete(String url) { | ||
protected static CommandInfo delete(String url) { | ||
return new CommandInfo(url, HttpVerb.DELETE); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
# Copyright 2014 Software Freedom Conservancy | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
from selenium.webdriver.remote.command import Command | ||
|
||
class ChromeCommand(Command): | ||
""" | ||
Defines Chrome specific commands while including standard WebDriver ones. | ||
""" | ||
LAUNCH_APP = "launchApp" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enums isn't really a thing in Python. Please just embed the "launchApp" string in chrome/webdriver.py. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
# Copyright 2014 Software Freedom Conservancy | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
from selenium.webdriver.remote.remote_connection import RemoteConnection | ||
from selenium.webdriver.chrome.command import ChromeCommand | ||
|
||
class ChromeRemoteConnection(RemoteConnection): | ||
|
||
def __init__(self, remote_server_addr, keep_alive=False): | ||
RemoteConnection.__init__(self, remote_server_addr, keep_alive) | ||
self._commands[ChromeCommand.LAUNCH_APP] =\ | ||
('POST', '/session/$sessionId/chromium/launch_app') |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -15,6 +15,8 @@ | |||
# limitations under the License. | ||||
|
||||
import base64 | ||||
from command import ChromeCommand | ||||
from remote_connection import ChromeRemoteConnection | ||||
from selenium.webdriver.remote.command import Command | ||||
from selenium.webdriver.remote.webdriver import WebDriver as RemoteWebDriver | ||||
from selenium.common.exceptions import WebDriverException | ||||
|
@@ -61,13 +63,21 @@ def __init__(self, executable_path="chromedriver", port=0, | |||
try: | ||||
RemoteWebDriver.__init__(self, | ||||
command_executor=self.service.service_url, | ||||
remote_connection_client=ChromeRemoteConnection, | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remote_connection_client parameter is unnecessary, it should not be passed here and reverted from remote/webdriver.py. What you need to do is to pass an instance of ChromeRemoteConnection as a value of command_executor parameter. See the current latest revision for an updated documentation that should make this less confusing:
|
||||
desired_capabilities=desired_capabilities, | ||||
keep_alive=True) | ||||
except: | ||||
self.quit() | ||||
raise | ||||
self._is_remote = False | ||||
|
||||
def launch_app(self, app_id): | ||||
""" | ||||
Launches an app with the specified id | ||||
""" | ||||
return self.execute(ChromeCommand.LAUNCH_APP, | ||||
{'id': app_id}) | ||||
|
||||
def quit(self): | ||||
""" | ||||
Closes the browser and shuts down the ChromeDriver executable | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,7 +45,8 @@ class WebDriver(object): | |
""" | ||
|
||
def __init__(self, command_executor='http://127.0.0.1:4444/wd/hub', | ||
desired_capabilities=None, browser_profile=None, proxy=None, keep_alive=False): | ||
desired_capabilities=None, browser_profile=None, proxy=None, keep_alive=False, | ||
remote_connection_client=RemoteConnection): | ||
""" | ||
Create a new driver that will issue commands using the wire protocol. | ||
|
||
|
@@ -62,7 +63,7 @@ def __init__(self, command_executor='http://127.0.0.1:4444/wd/hub', | |
proxy.add_to_capabilities(desired_capabilities) | ||
self.command_executor = command_executor | ||
if type(self.command_executor) is bytes or type(self.command_executor) is str: | ||
self.command_executor = RemoteConnection(command_executor, keep_alive=keep_alive) | ||
self.command_executor = remote_connection_client(command_executor, keep_alive=keep_alive) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't see that the symbol There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One of the commits in the PR added remote_connection_client parameter to this constructor. However that change is unnecessary and everything in this file needs to be reverted. (I'll add comments in chrome/webdriver.py explaining how) |
||
self._is_remote = True | ||
self.session_id = None | ||
self.capabilities = {} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please simply use constructor, new CommandInfo(), instead of method post()