Reorganize and split tests dir and conftest files #3893
Conversation
liorsve
force-pushed
the
reorganize-tests
branch
2 times, most recently
from
ff535da to
05872bc
Compare
python/tests/sync_tests/conftest.py
Outdated
| ServerCredentials, | ||
| ) | ||
| from glide.exceptions import ClosingError | ||
| from glide.glide_client import GlideClient, GlideClusterClient |
There was a problem hiding this comment.
why is it needed? it should be separated - and only the sync client to be used here
python/tests/sync_tests/conftest.py
Outdated
| cluster_mode: bool, | ||
| protocol: ProtocolVersion, | ||
| ) -> Generator[TSyncGlideClient, None, None]: | ||
| "Get async socket client for tests" |
There was a problem hiding this comment.
| "Get async socket client for tests" | |
| "Get sync client for tests" |
python/tests/async_tests/conftest.py
Outdated
| return await GlideClient.create(config) | ||
|
|
||
|
|
||
| async def auth_client(client: TGlideClient, password: str, username: str = "default"): |
There was a problem hiding this comment.
why "username: str = "default")" isn't copied also for the sync auth_client fn?
liorsve
force-pushed
the
python-sync-client
branch
from
9cb682c to
77918d2
Compare
python/tests/async_tests/conftest.py
Outdated
| async def kill_connections(client: TGlideClient): | ||
| """ | ||
| Kills all connections to the given TGlideClient server connected. | ||
| """ | ||
| if isinstance(client, GlideClient): | ||
| await client.custom_command(["CLIENT", "KILL", "TYPE", "normal"]) | ||
| elif isinstance(client, GlideClusterClient): | ||
| await client.custom_command( | ||
| ["CLIENT", "KILL", "TYPE", "normal"], route=AllNodes() | ||
| ) |
There was a problem hiding this comment.
I think that one of the goals of this PR is to reduce code and logic duplication, so instead of duplicating all of these test helper functions, we can do something like the following in a shared file, changing these helpers to be blocking also for the async client (since it's only helper functions and already we call them from the tests in a blocking manner):
TGlideClient = Union[GlideClient, GlideClusterClient, GlideClientSync, GlideClusterClientSync]
def kill_connections(client: TGlideClient):
"""
Kills all connections using .custom_command, and runs it in sync or async depending on client type.
"""
if isinstance(client, GlideClient):
res = client.custom_command(["CLIENT", "KILL", "TYPE", "normal"])
elif isinstance(client, GlideClusterClient):
res = client.custom_command(["CLIENT", "KILL", "TYPE", "normal"], route=AllNodes())
else:
raise TypeError("Unknown client type")
if asyncio.iscoroutine(res):
return asyncio.run(res) # convert async call into sync
return res # sync result
There was a problem hiding this comment.
fixed, I added an alias
TAnyGlideClient = Union[TGlideClient, TSyncGlideClient] for this in utils.py
liorsve
force-pushed
the
python-sync-client
branch
2 times, most recently
from
122d41d to
dda9443
Compare
Signed-off-by: Lior Sventitzky <[email protected]>
liorsve
force-pushed
the
reorganize-tests
branch
4 times, most recently
from
69984d9 to
6c62195
Compare
python/tests/sync_tests/conftest.py
Outdated
| create_client_config, | ||
| ) | ||
|
|
||
| Logger.set_logger_config(DEFAULT_TEST_LOG_LEVEL) |
There was a problem hiding this comment.
can it be set only once for sync and async in the main conftest file?
There was a problem hiding this comment.
removed the logger config from the sync_tests/conftest.py file, left it in async conftest
Signed-off-by: Lior Sventitzky <[email protected]>
Issue link
This Pull Request is linked to issue: #3634
New tests dir structure -
the
create_client_configfunction and some constants were moved from thetests/conftest.pyfile toutilsto avoid circular import problems.Checklist
Before submitting the PR make sure the following are checked: