fix docstring, add getting available port number #448
Conversation
| driver2 = webdriver.Remote('http://localhost:4723/wd/hub', desired_caps) | ||
| self.assertEqual(2, len(self.driver.all_sessions)) | ||
| finally: | ||
| if driver2: |
|
|
||
| def tearDown(self): | ||
| self.driver.quit() | ||
|
|
||
| def testAllSessions(self): | ||
| def test_all_sessions(self): | ||
| port = desired_capabilities.get_available_port() |
There was a problem hiding this comment.
should this method be located in desired_capabilities?
I'd rather put in test helpers?
There was a problem hiding this comment.
yes, let me update them to the proper place later.
Current this test is not stable (I rebuild once to make the CI green)
| import socket | ||
| sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) | ||
|
|
||
| for port in range(8102, 8200): |
KazuCocoa
force-pushed
the
km/tweak-all-sessions
branch
from
75d00a0
to
ee31c3b
Compare
test/functional/test_helper.py
Outdated
| def get_available_port(in_range): | ||
| """Returns available local port number. | ||
| """ | ||
| range(8102, 8200) |
There was a problem hiding this comment.
what this operator is for?
test/functional/test_helper.py
Outdated
| """ | ||
| range(8102, 8200) | ||
| if not isinstance(in_range, range): | ||
| raise ValueError('{} should be range'.format(in_range)) |
There was a problem hiding this comment.
should be of type range
test/functional/test_helper.py
Outdated
| @@ -0,0 +1,15 @@ | |||
| def get_available_port(in_range): | |||
test/functional/test_helper.py
Outdated
| """Returns available local port number. | ||
| """ | ||
| range(8102, 8200) | ||
| if not isinstance(in_range, range): |
There was a problem hiding this comment.
in python3 range creates a generator by default. I would rather provide just start and end port numbers with default values of 8102 and 8200 as method arguments
test/functional/test_helper.py
Outdated
| sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) | ||
|
|
||
| for port in in_range: | ||
| if sock.connect_ex(('localhost', port)) != 0: |
There was a problem hiding this comment.
try:
if sock.connect_ex(('localhost', port)) != 0:
return port
finally:
sock.close()
KazuCocoa
changed the title
test/functional/test_helper.py
Outdated
| @@ -0,0 +1,15 @@ | |||
| def get_available_port_range(from_range, to_range): | |||
There was a problem hiding this comment.
...port_from_range
There was a problem hiding this comment.
from_port, to_port
|
removed wip |
test/functional/test_helper.py
Outdated
| def get_available_port_range(from_range, to_range): | ||
| """Returns available local port number. | ||
| """ | ||
| if not isinstance(from_range, int) or not isinstance(to_range, int): |
There was a problem hiding this comment.
explicit types verifications is not very pythonic
simply cast values to int in range call and let it throw if this is not possible
| self.driver1.quit() | ||
| desired_caps['wdaLocalPort'] = port | ||
|
|
||
| class get_all_sessions(object): |
There was a problem hiding this comment.
This class name doesn't mean actual behavior and it's a little tricky.
I'd rather have your original code style from maintenance point of view if it works.
WebDriverWait(driver2, 10).until(len(driver.all_sessions) == 2)
or
TIMEOUT = 10
WebDriverWait(driver2, TIMEOUT).until(len(driver.all_sessions) == 2)
There was a problem hiding this comment.
len(driver.all_sessions) == 2 did not work in WebDriverWait, unfortunately
test/functional/test_helper.py
Outdated
| """ | ||
| r = range(from_port, to_port) | ||
|
|
||
| import socket |
There was a problem hiding this comment.
why we don't import socket at the start of the module?
There was a problem hiding this comment.
Just copied and pasted, but can move it to outside. Will do with renaming the class name
| self.driver1.quit() | ||
| desired_caps['wdaLocalPort'] = port | ||
|
|
||
| class GetAllSessions(object): |
There was a problem hiding this comment.
usually class names are nouns. How about SessionCountIsTwoCondition ?
| self.driver1.quit() | ||
| desired_caps['wdaLocalPort'] = port | ||
|
|
||
| class session_counts_is_two(object): |
There was a problem hiding this comment.
snake case to follow https://selenium-python.readthedocs.io/waits.html 's name style
No description provided.