Skip to content

Commit ba04acd

Browse files
committed
[py]: Base Service tidy up
1 parent ca217d2 commit ba04acd

File tree

2 files changed

+40
-39
lines changed

2 files changed

+40
-39
lines changed

py/selenium/webdriver/common/service.py

Lines changed: 39 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
# KIND, either express or implied. See the License for the
1515
# specific language governing permissions and limitations
1616
# under the License.
17+
import contextlib
1718
import errno
1819
import os
1920
import subprocess
21+
import typing
2022
from platform import system
2123
from subprocess import DEVNULL
2224
from subprocess import PIPE
@@ -29,22 +31,22 @@
2931

3032

3133
class Service:
32-
33-
def __init__(self, executable, port=0, log_file=DEVNULL, env=None, start_error_message=""):
34+
def __init__(
35+
self,
36+
executable: str,
37+
port: int = 0,
38+
log_file=DEVNULL,
39+
env: typing.Optional[typing.Dict[typing.Any, typing.Any]] = None,
40+
start_error_message: str = "",
41+
) -> None:
3442
self.path = executable
35-
36-
self.port = port
37-
if self.port == 0:
38-
self.port = utils.free_port()
39-
40-
if not _HAS_NATIVE_DEVNULL and log_file == DEVNULL:
41-
log_file = open(os.devnull, 'wb')
42-
43+
self.port = port or utils.free_port()
44+
self.log_file = open(os.devnull, "wb") if not _HAS_NATIVE_DEVNULL and log_file == DEVNULL else log_file
4345
self.start_error_message = start_error_message
44-
self.log_file = log_file
4546
# Default value for every python subprocess: subprocess.Popen(..., creationflags=0)
46-
self.creationflags = 0
47+
self.creation_flags = 0
4748
self.env = env or os.environ
49+
self.process = None
4850

4951
@property
5052
def service_url(self):
@@ -67,31 +69,37 @@ def start(self):
6769
try:
6870
cmd = [self.path]
6971
cmd.extend(self.command_line_args())
70-
self.process = subprocess.Popen(cmd, env=self.env,
71-
close_fds=system() != 'Windows',
72-
stdout=self.log_file,
73-
stderr=self.log_file,
74-
stdin=PIPE,
75-
creationflags=self.creationflags)
72+
self.process = subprocess.Popen(
73+
cmd,
74+
env=self.env,
75+
close_fds=system() != "Windows",
76+
stdout=self.log_file,
77+
stderr=self.log_file,
78+
stdin=PIPE,
79+
creationflags=self.creation_flags,
80+
)
7681
except TypeError:
7782
raise
7883
except OSError as err:
7984
if err.errno == errno.ENOENT:
8085
raise WebDriverException(
8186
"'{}' executable needs to be in PATH. {}".format(
82-
os.path.basename(self.path), self.start_error_message)
87+
os.path.basename(self.path), self.start_error_message
88+
)
8389
)
8490
elif err.errno == errno.EACCES:
8591
raise WebDriverException(
8692
"'{}' executable may have wrong permissions. {}".format(
87-
os.path.basename(self.path), self.start_error_message)
93+
os.path.basename(self.path), self.start_error_message
94+
)
8895
)
8996
else:
9097
raise
9198
except Exception as e:
9299
raise WebDriverException(
93-
"The executable %s needs to be available in the path. %s\n%s" %
94-
(os.path.basename(self.path), self.start_error_message, str(e)))
100+
"The executable %s needs to be available in the path. %s\n%s"
101+
% (os.path.basename(self.path), self.start_error_message, str(e))
102+
)
95103
count = 0
96104
while True:
97105
self.assert_process_still_running()
@@ -106,19 +114,17 @@ def start(self):
106114
def assert_process_still_running(self):
107115
return_code = self.process.poll()
108116
if return_code:
109-
raise WebDriverException(
110-
'Service %s unexpectedly exited. Status code was: %s'
111-
% (self.path, return_code)
112-
)
117+
raise WebDriverException(f"Service {self.path} unexpectedly exited. Status code was: {return_code}")
113118

114119
def is_connectable(self):
115120
return utils.is_connectable(self.port)
116121

117122
def send_remote_shutdown_command(self):
118123
from urllib import request
119124
from urllib.error import URLError
125+
120126
try:
121-
request.urlopen("%s/shutdown" % self.service_url)
127+
request.urlopen(f"{self.service_url}/shutdown")
122128
except URLError:
123129
return
124130

@@ -127,15 +133,14 @@ def send_remote_shutdown_command(self):
127133
break
128134
sleep(1)
129135

130-
def stop(self):
136+
def stop(self) -> None:
131137
"""
132138
Stops the service.
133139
"""
134140
if self.log_file != PIPE and not (self.log_file == DEVNULL and _HAS_NATIVE_DEVNULL):
135-
try:
141+
with contextlib.suppress(Exception):
142+
# Todo: Be explicit in what we are catching here.
136143
self.log_file.close()
137-
except Exception:
138-
pass
139144

140145
if not self.process:
141146
return
@@ -147,9 +152,7 @@ def stop(self):
147152

148153
try:
149154
if self.process:
150-
for stream in [self.process.stdin,
151-
self.process.stdout,
152-
self.process.stderr]:
155+
for stream in [self.process.stdin, self.process.stdout, self.process.stderr]:
153156
try:
154157
stream.close()
155158
except AttributeError:
@@ -161,11 +164,9 @@ def stop(self):
161164
except OSError:
162165
pass
163166

164-
def __del__(self):
167+
def __del__(self) -> None:
165168
# `subprocess.Popen` doesn't send signal on `__del__`;
166169
# so we attempt to close the launched process when `__del__`
167170
# is triggered.
168-
try:
171+
with contextlib.suppress(Exception):
169172
self.stop()
170-
except Exception:
171-
pass

py/tox.ini

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,5 +57,5 @@ deps =
5757
flake8-typing-imports==1.13.0
5858
commands =
5959
isort selenium/ test/
60-
black test/ selenium/common/ selenium/webdriver/safari -l 120
60+
black test/ selenium/common/ selenium/webdriver/safari selenium/webdriver/common/service.py -l 120
6161
flake8 selenium/ test/ --min-python-version=3.7

0 commit comments

Comments
 (0)