Skip to content

Commit f5b8e63

Browse files
bpkrothpre-commit-ci[bot]Copilot
authored
Split docker tests fixtures (#1003)
# Pull Request ## Title Split docker test fixtures ______________________________________________________________________ ## Description Splits docker test fixtures so that individual docker-compose projects can be setup and torn down independently. This improves single test performance (e.g., when SQL tests don't need to setup SSH test fixtures) and parallelism by allowing each to operate independently. ______________________________________________________________________ ## Type of Change - 🔄 Refactor - 🧪 Tests ______________________________________________________________________ ## Additional Notes (optional) Should be merged after #994 ______________________________________________________________________ --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent b9553d0 commit f5b8e63

7 files changed

Lines changed: 268 additions & 144 deletions

File tree

‎mlos_bench/mlos_bench/tests/conftest.py‎

Lines changed: 0 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,9 @@
44
#
55
"""Common fixtures for mock TunableGroups and Environment objects."""
66

7-
import os
87
import sys
9-
from collections.abc import Generator
10-
from typing import Any
118

129
import pytest
13-
from fasteners import InterProcessLock, InterProcessReaderWriterLock
14-
from pytest_docker.plugin import Services as DockerServices
15-
from pytest_docker.plugin import get_docker_services
1610

1711
from mlos_bench.environments.mock_env import MockEnv
1812
from mlos_bench.tests import SEED, resolve_host_name, tunable_groups_fixtures
@@ -73,141 +67,3 @@ def mock_env_no_noise(tunable_groups: TunableGroups) -> MockEnv:
7367
},
7468
tunables=tunable_groups,
7569
)
76-
77-
78-
# Fixtures to configure the pytest-docker plugin.
79-
@pytest.fixture(scope="session")
80-
def docker_setup() -> list[str] | str:
81-
"""Setup for docker services."""
82-
if sys.platform == "darwin" or os.environ.get("HOST_OSTYPE", "").lower().startswith("darwin"):
83-
# Workaround an oddity on macOS where the "docker-compose up"
84-
# command always recreates the containers.
85-
# That leads to races when multiple workers are trying to
86-
# start and use the same services.
87-
return ["up --build -d --no-recreate"]
88-
else:
89-
return ["up --build -d"]
90-
91-
92-
@pytest.fixture(scope="session")
93-
def docker_compose_file(pytestconfig: pytest.Config) -> list[str]:
94-
"""
95-
Returns the path to the docker-compose file.
96-
97-
Parameters
98-
----------
99-
pytestconfig : pytest.Config
100-
101-
Returns
102-
-------
103-
str
104-
Path to the docker-compose file.
105-
"""
106-
_ = pytestconfig # unused
107-
# TODO: move this closer to the necessary submodules so that different
108-
# docker tests can run independently.
109-
return [
110-
os.path.join(os.path.dirname(__file__), "services", "remote", "ssh", "docker-compose.yml"),
111-
os.path.join(os.path.dirname(__file__), "storage", "sql", "docker-compose.yml"),
112-
# Add additional configs as necessary here.
113-
]
114-
115-
116-
@pytest.fixture(scope="session")
117-
def docker_compose_project_name(short_testrun_uid: str) -> str:
118-
"""
119-
Returns the name of the docker-compose project.
120-
121-
Returns
122-
-------
123-
str
124-
Name of the docker-compose project.
125-
"""
126-
# Use the xdist testrun UID to ensure that the docker-compose project name
127-
# is unique across sessions, but shared amongst workers.
128-
return f"mlos_bench-test-{short_testrun_uid}"
129-
130-
131-
@pytest.fixture(scope="session")
132-
def docker_services_lock(
133-
shared_temp_dir: str,
134-
short_testrun_uid: str,
135-
) -> InterProcessReaderWriterLock:
136-
"""
137-
Gets a pytest session lock for xdist workers to mark when they're using the docker
138-
services.
139-
140-
Yields
141-
------
142-
A lock to ensure that setup/teardown operations don't happen while a
143-
worker is using the docker services.
144-
"""
145-
return InterProcessReaderWriterLock(
146-
f"{shared_temp_dir}/pytest_docker_services-{short_testrun_uid}.lock"
147-
)
148-
149-
150-
@pytest.fixture(scope="session")
151-
def docker_setup_teardown_lock(shared_temp_dir: str, short_testrun_uid: str) -> InterProcessLock:
152-
"""
153-
Gets a pytest session lock between xdist workers for the docker setup/teardown
154-
operations.
155-
156-
Yields
157-
------
158-
A lock to ensure that only one worker is doing setup/teardown at a time.
159-
"""
160-
return InterProcessLock(
161-
f"{shared_temp_dir}/pytest_docker_services-setup-teardown-{short_testrun_uid}.lock"
162-
)
163-
164-
165-
@pytest.fixture(scope="session")
166-
def locked_docker_services(
167-
docker_compose_command: Any,
168-
docker_compose_file: Any,
169-
docker_compose_project_name: Any,
170-
docker_setup: Any,
171-
docker_cleanup: Any,
172-
docker_setup_teardown_lock: InterProcessLock,
173-
docker_services_lock: InterProcessReaderWriterLock,
174-
) -> Generator[DockerServices, Any, None]:
175-
"""A locked version of the docker_services fixture to implement xdist single
176-
instance locking.
177-
"""
178-
# pylint: disable=too-many-arguments,too-many-positional-arguments
179-
# Mark the services as in use with the reader lock.
180-
docker_services_lock.acquire_read_lock()
181-
# Acquire the setup lock to prevent multiple setup operations at once.
182-
docker_setup_teardown_lock.acquire()
183-
# This "with get_docker_services(...)"" pattern is in the default fixture.
184-
# We call it instead of docker_services() to avoid pytest complaints about
185-
# calling fixtures directly.
186-
with get_docker_services(
187-
docker_compose_command,
188-
docker_compose_file,
189-
docker_compose_project_name,
190-
docker_setup,
191-
docker_cleanup,
192-
) as docker_services:
193-
# Release the setup/tear down lock in order to let the setup operation
194-
# continue for other workers (should be a no-op at this point).
195-
docker_setup_teardown_lock.release()
196-
# Yield the services so that tests within this worker can use them.
197-
yield docker_services
198-
# Now tests that use those services get to run on this worker...
199-
# Once the tests are done, release the read lock that marks the services as in use.
200-
docker_services_lock.release_read_lock()
201-
# Now as we prepare to execute the cleanup code on context exit we need
202-
# to acquire the setup/teardown lock again.
203-
# First we attempt to get the write lock so that we wait for other
204-
# readers to finish and guard against a lock inversion possibility.
205-
docker_services_lock.acquire_write_lock()
206-
# Next, acquire the setup/teardown lock
207-
# First one here is the one to do actual work, everyone else is basically a no-op.
208-
# Upon context exit, we should execute the docker_cleanup code.
209-
# And try to get the setup/tear down lock again.
210-
docker_setup_teardown_lock.acquire()
211-
# Finally, after the docker_cleanup code has finished, remove both locks.
212-
docker_setup_teardown_lock.release()
213-
docker_services_lock.release_write_lock()

‎mlos_bench/mlos_bench/tests/docker_fixtures_util.py‎

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
#
2+
# Copyright (c) Microsoft Corporation.
3+
# Licensed under the MIT License.
4+
#
5+
"""
6+
Helper functions for various docker test fixtures.
7+
8+
Test functions needing to use these should import them and then add them to their
9+
namespace in a conftest.py file.
10+
11+
The intent of keeping these separate from conftest.py is to allow individual test to
12+
setup their own docker-compose configurations that are independent.
13+
14+
As such, each conftest.py should set their own docker_compose_file fixture pointing to
15+
the appropriate docker-compose.yml file(s) and set a unique docker_compose_project_name.
16+
"""
17+
# pylint: disable=redefined-outer-name
18+
19+
import os
20+
import sys
21+
from collections.abc import Generator
22+
from typing import Any
23+
24+
import pytest
25+
from fasteners import InterProcessLock, InterProcessReaderWriterLock
26+
from pytest_docker.plugin import Services as DockerServices
27+
from pytest_docker.plugin import get_docker_services
28+
29+
30+
# Fixtures to configure the pytest-docker plugin.
31+
@pytest.fixture(scope="session")
32+
def docker_setup() -> list[str] | str:
33+
"""Setup for docker services."""
34+
if sys.platform == "darwin" or os.environ.get("HOST_OSTYPE", "").lower().startswith("darwin"):
35+
# Workaround an oddity on macOS where the "docker-compose up"
36+
# command always recreates the containers.
37+
# That leads to races when multiple workers are trying to
38+
# start and use the same services.
39+
return ["up --build -d --no-recreate"]
40+
else:
41+
return ["up --build -d"]
42+
43+
44+
@pytest.fixture(scope="session")
45+
def docker_compose_file(pytestconfig: pytest.Config) -> list[str]:
46+
"""
47+
Fixture for the path to the docker-compose file.
48+
49+
Parameters
50+
----------
51+
pytestconfig : pytest.Config
52+
53+
Returns
54+
-------
55+
list[str]
56+
List of paths to the docker-compose file(s).
57+
"""
58+
_ = pytestconfig # unused
59+
# Add additional configs as necessary here.
60+
# return []
61+
raise NotImplementedError("Please implement docker_compose_file in your conftest.py")
62+
63+
64+
@pytest.fixture(scope="session")
65+
def docker_compose_project_name(short_testrun_uid: str) -> str:
66+
"""
67+
Fixture for the name of the docker-compose project.
68+
69+
Returns
70+
-------
71+
str
72+
Name of the docker-compose project.
73+
"""
74+
# Use the xdist testrun UID to ensure that the docker-compose project name
75+
# is unique across sessions, but shared amongst workers.
76+
# return f"""mlos_bench-test-{short_testrun_uid}-{__name__.replace(".", "-")}"""
77+
raise NotImplementedError("Please implement docker_compose_project_name in your conftest.py")
78+
79+
80+
@pytest.fixture(scope="session")
81+
def docker_services_lock(
82+
shared_temp_dir: str,
83+
short_testrun_uid: str,
84+
) -> InterProcessReaderWriterLock:
85+
"""
86+
Gets a pytest session lock for xdist workers to mark when they're using the docker
87+
services.
88+
89+
Yields
90+
------
91+
A lock to ensure that setup/teardown operations don't happen while a
92+
worker is using the docker services.
93+
"""
94+
return InterProcessReaderWriterLock(
95+
f"{shared_temp_dir}/pytest_docker_services-{short_testrun_uid}.lock"
96+
)
97+
98+
99+
@pytest.fixture(scope="session")
100+
def docker_setup_teardown_lock(shared_temp_dir: str, short_testrun_uid: str) -> InterProcessLock:
101+
"""
102+
Gets a pytest session lock between xdist workers for the docker setup/teardown
103+
operations.
104+
105+
Yields
106+
------
107+
A lock to ensure that only one worker is doing setup/teardown at a time.
108+
"""
109+
return InterProcessLock(
110+
f"{shared_temp_dir}/pytest_docker_services-setup-teardown-{short_testrun_uid}.lock"
111+
)
112+
113+
114+
@pytest.fixture(scope="session")
115+
def locked_docker_services(
116+
docker_compose_command: Any,
117+
docker_compose_file: Any,
118+
docker_compose_project_name: Any,
119+
docker_setup: Any,
120+
docker_cleanup: Any,
121+
docker_setup_teardown_lock: InterProcessLock,
122+
docker_services_lock: InterProcessReaderWriterLock,
123+
) -> Generator[DockerServices, Any, None]:
124+
"""A locked version of the docker_services fixture to implement xdist single
125+
instance locking.
126+
"""
127+
# pylint: disable=too-many-arguments,too-many-positional-arguments
128+
# Mark the services as in use with the reader lock.
129+
docker_services_lock.acquire_read_lock()
130+
# Acquire the setup lock to prevent multiple setup operations at once.
131+
docker_setup_teardown_lock.acquire()
132+
# This "with get_docker_services(...)"" pattern is in the default fixture.
133+
# We call it instead of docker_services() to avoid pytest complaints about
134+
# calling fixtures directly.
135+
with get_docker_services(
136+
docker_compose_command,
137+
docker_compose_file,
138+
docker_compose_project_name,
139+
docker_setup,
140+
docker_cleanup,
141+
) as docker_services:
142+
# Release the setup/tear down lock in order to let the setup operation
143+
# continue for other workers (should be a no-op at this point).
144+
docker_setup_teardown_lock.release()
145+
# Yield the services so that tests within this worker can use them.
146+
yield docker_services
147+
# Now tests that use those services get to run on this worker...
148+
# Once the tests are done, release the read lock that marks the services as in use.
149+
docker_services_lock.release_read_lock()
150+
# Now as we prepare to execute the cleanup code on context exit we need
151+
# to acquire the setup/teardown lock again.
152+
# First we attempt to get the write lock so that we wait for other
153+
# readers to finish and guard against a lock inversion possibility.
154+
docker_services_lock.acquire_write_lock()
155+
# Next, acquire the setup/teardown lock
156+
# First one here is the one to do actual work, everyone else is basically a no-op.
157+
# Upon context exit, we should execute the docker_cleanup code.
158+
# And try to get the setup/tear down lock again.
159+
docker_setup_teardown_lock.acquire()
160+
# Finally, after the docker_cleanup code has finished, remove both locks.
161+
docker_setup_teardown_lock.release()
162+
docker_services_lock.release_write_lock()
163+
164+
165+
__all__ = [
166+
# These two should be implemented in the conftest.py of the local test suite
167+
# "docker_compose_file",
168+
# "docker_compose_project_name",
169+
"docker_setup",
170+
"docker_services_lock",
171+
"docker_setup_teardown_lock",
172+
"locked_docker_services",
173+
]

‎mlos_bench/mlos_bench/tests/environments/remote/conftest.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,16 @@
44
#
55
"""Fixtures for the RemoteEnv tests using SSH Services."""
66

7+
import mlos_bench.tests.docker_fixtures_util as docker_fixtures_util
78
import mlos_bench.tests.services.remote.ssh.fixtures as ssh_fixtures
89

910
# Expose some of those as local names so they can be picked up as fixtures by pytest.
11+
docker_compose_file = ssh_fixtures.docker_compose_file
12+
docker_compose_project_name = ssh_fixtures.docker_compose_project_name
13+
14+
docker_setup = docker_fixtures_util.docker_setup
15+
docker_services_lock = docker_fixtures_util.docker_services_lock
16+
docker_setup_teardown_lock = docker_fixtures_util.docker_setup_teardown_lock
17+
locked_docker_services = docker_fixtures_util.locked_docker_services
18+
1019
ssh_test_server = ssh_fixtures.ssh_test_server

‎mlos_bench/mlos_bench/tests/services/remote/ssh/conftest.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,18 @@
44
#
55
"""Fixtures for the SSH service tests."""
66

7+
import mlos_bench.tests.docker_fixtures_util as docker_fixtures_util
78
import mlos_bench.tests.services.remote.ssh.fixtures as ssh_fixtures
89

910
# Expose some of those as local names so they can be picked up as fixtures by pytest.
11+
docker_compose_file = ssh_fixtures.docker_compose_file
12+
docker_compose_project_name = ssh_fixtures.docker_compose_project_name
13+
14+
docker_setup = docker_fixtures_util.docker_setup
15+
docker_services_lock = docker_fixtures_util.docker_services_lock
16+
docker_setup_teardown_lock = docker_fixtures_util.docker_setup_teardown_lock
17+
locked_docker_services = docker_fixtures_util.locked_docker_services
18+
1019
ssh_test_server = ssh_fixtures.ssh_test_server
1120
alt_test_server = ssh_fixtures.alt_test_server
1221
reboot_test_server = ssh_fixtures.reboot_test_server

0 commit comments

Comments
 (0)