Skip to content

Commit 34e6b05

Browse files
committed
Harden xdist test manager isolation
Use a local re_manager_cmd fixture with worker-scoped ZMQ/Redis settings and route tests to it to prevent cross-worker collisions and startup cascades. Also switch queue reset calls to zmq_secure_request so control requests consistently target the active manager.
1 parent 4437a44 commit 34e6b05

7 files changed

Lines changed: 204 additions & 40 deletions

File tree

bluesky_httpserver/tests/conftest.py

Lines changed: 174 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
import os
2+
import sys
23
import time as ttime
34

45
import pytest
56
import requests
6-
from bluesky_queueserver.manager.comms import zmq_single_request
7-
from bluesky_queueserver.manager.tests.common import re_manager_cmd # noqa: F401
87
from bluesky_queueserver.manager.tests.common import set_qserver_zmq_encoding # noqa: F401
98
from bluesky_queueserver.manager.tests.common import (
109
ReManager,
1110
condition_manager_idle,
1211
wait_for_condition,
12+
zmq_secure_request,
1313
)
1414
from xprocess import ProcessStarter
1515

@@ -29,8 +29,14 @@ def _worker_index():
2929
return 0
3030

3131

32+
def _worker_name():
33+
return os.environ.get("PYTEST_XDIST_WORKER", "local")
34+
35+
3236
def _ports_for_worker():
33-
base = 60600 + _worker_index() * 100
37+
# Avoid the queue-server default ports (60615/60625), which may already be
38+
# occupied in shared/dev environments.
39+
base = 62000 + _worker_index() * 100
3440
return {
3541
"server_port": str(base + 10),
3642
"zmq_control_server": f"tcp://*:{base + 15}",
@@ -40,6 +46,58 @@ def _ports_for_worker():
4046
}
4147

4248

49+
def _redis_name_prefix(*, scope, sequence=None):
50+
parts = ["qs_unit_tests_httpserver", _worker_name(), scope]
51+
if sequence is not None:
52+
parts.append(str(sequence))
53+
return "_".join(parts)
54+
55+
56+
def _get_cli_option_value(params, option):
57+
option_with_eq = f"{option}="
58+
for n, value in enumerate(params):
59+
if value.startswith(option_with_eq):
60+
return value[len(option_with_eq) :]
61+
if value == option and n + 1 < len(params):
62+
return params[n + 1]
63+
return None
64+
65+
66+
def _server_to_client_zmq_addr(addr):
67+
if addr.startswith("tcp://*:"):
68+
return f"tcp://localhost:{addr.rsplit(':', 1)[1]}"
69+
if addr.startswith("tcp://0.0.0.0:"):
70+
return f"tcp://localhost:{addr.rsplit(':', 1)[1]}"
71+
return addr
72+
73+
74+
def _set_zmq_env(control_addr, info_addr):
75+
os.environ["QSERVER_ZMQ_CONTROL_ADDRESS"] = control_addr
76+
os.environ["QSERVER_ZMQ_INFO_ADDRESS"] = info_addr
77+
os.environ["_TEST_QSERVER_ZMQ_ADDRESS_"] = control_addr
78+
79+
80+
def _ensure_manager_addresses_in_params(params):
81+
ports = _ports_for_worker()
82+
83+
control_addr = _get_cli_option_value(params, "--zmq-control-addr")
84+
if control_addr is None:
85+
control_addr = ports["zmq_control_server"]
86+
params.append(f"--zmq-control-addr={control_addr}")
87+
88+
info_addr = _get_cli_option_value(params, "--zmq-info-addr")
89+
if info_addr is None:
90+
info_addr = ports["zmq_info_server"]
91+
params.append(f"--zmq-info-addr={info_addr}")
92+
93+
return {
94+
"control_server": control_addr,
95+
"control_client": _server_to_client_zmq_addr(control_addr),
96+
"info_server": info_addr,
97+
"info_client": _server_to_client_zmq_addr(info_addr),
98+
}
99+
100+
43101
def _xprocess_name(name):
44102
worker = os.environ.get("PYTEST_XDIST_WORKER", "local")
45103
return f"{name}_{worker}"
@@ -60,13 +118,17 @@ def _wait_for_manager_ready(timeout=10):
60118
raise TimeoutError("Timeout: RE Manager failed to start.")
61119

62120

63-
def _reset_queue_mode_and_clear_queue():
64-
resp, msg = zmq_single_request("queue_mode_set", params={"mode": "default"})
65-
if resp["success"] is not True:
121+
def _reset_queue_mode():
122+
resp, msg = zmq_secure_request("queue_mode_set", params={"mode": "default"})
123+
if not resp or resp.get("success") is not True:
66124
raise RuntimeError(msg)
67125

68-
resp, msg = zmq_single_request("queue_clear")
69-
if resp["success"] is not True:
126+
127+
def _reset_queue_mode_and_clear_queue():
128+
_reset_queue_mode()
129+
130+
resp, msg = zmq_secure_request("queue_clear")
131+
if not resp or resp.get("success") is not True:
70132
raise RuntimeError(msg)
71133

72134

@@ -147,15 +209,15 @@ class Starter(ProcessStarter):
147209
def re_manager(): # noqa: F811
148210
ports = _ports_for_worker()
149211

150-
os.environ["QSERVER_ZMQ_CONTROL_ADDRESS"] = ports["zmq_control_client"]
151-
os.environ["QSERVER_ZMQ_INFO_ADDRESS"] = ports["zmq_info_client"]
152-
os.environ["_TEST_QSERVER_ZMQ_ADDRESS_"] = ports["zmq_control_client"]
212+
_set_zmq_env(ports["zmq_control_client"], ports["zmq_info_client"])
153213

154214
manager = ReManager(
155215
params=[
156216
f"--zmq-control-addr={ports['zmq_control_server']}",
157217
f"--zmq-info-addr={ports['zmq_info_server']}",
158-
]
218+
f"--redis-name-prefix={_redis_name_prefix(scope='re_manager')}",
219+
],
220+
set_redis_name_prefix=False,
159221
)
160222
failed_to_start = False
161223

@@ -179,15 +241,15 @@ def re_manager(): # noqa: F811
179241
@pytest.fixture(scope="module")
180242
def re_manager_module():
181243
ports = _ports_for_worker()
182-
os.environ["QSERVER_ZMQ_CONTROL_ADDRESS"] = ports["zmq_control_client"]
183-
os.environ["QSERVER_ZMQ_INFO_ADDRESS"] = ports["zmq_info_client"]
184-
os.environ["_TEST_QSERVER_ZMQ_ADDRESS_"] = ports["zmq_control_client"]
244+
_set_zmq_env(ports["zmq_control_client"], ports["zmq_info_client"])
185245

186246
manager = ReManager(
187247
params=[
188248
f"--zmq-control-addr={ports['zmq_control_server']}",
189249
f"--zmq-info-addr={ports['zmq_info_server']}",
190-
]
250+
f"--redis-name-prefix={_redis_name_prefix(scope='re_manager_module')}",
251+
],
252+
set_redis_name_prefix=False,
191253
)
192254
failed_to_start = False
193255

@@ -208,6 +270,78 @@ def re_manager_module():
208270
manager.kill_manager()
209271

210272

273+
@pytest.fixture
274+
def re_manager_cmd(): # noqa: F811
275+
manager = None
276+
failed_to_start = False
277+
manager_sequence = 0
278+
279+
def _close_manager():
280+
nonlocal manager, failed_to_start
281+
282+
if manager is None:
283+
return
284+
285+
if failed_to_start:
286+
try:
287+
manager.kill_manager()
288+
except Exception:
289+
pass
290+
manager = None
291+
return
292+
293+
try:
294+
manager.stop_manager(timeout=30)
295+
except Exception:
296+
try:
297+
manager.kill_manager()
298+
except Exception:
299+
pass
300+
finally:
301+
manager = None
302+
303+
def create_re_manager(
304+
params=None, *, stdout=sys.stdout, stderr=sys.stdout, set_redis_name_prefix=True
305+
):
306+
nonlocal manager, failed_to_start, manager_sequence
307+
308+
failed_to_start = False
309+
manager_sequence += 1
310+
311+
_close_manager()
312+
313+
params = list(params or [])
314+
addrs = _ensure_manager_addresses_in_params(params)
315+
_set_zmq_env(addrs["control_client"], addrs["info_client"])
316+
317+
# Always force per-worker/per-create Redis prefixes to avoid collisions in parallel runs.
318+
if _get_cli_option_value(params, "--redis-name-prefix") is None:
319+
params.append(
320+
f"--redis-name-prefix={_redis_name_prefix(scope='re_manager_cmd', sequence=manager_sequence)}"
321+
)
322+
# We explicitly manage the Redis name prefix and do not want defaults from upstream fixture logic.
323+
set_redis_name_prefix = False
324+
325+
manager = ReManager(
326+
params=params,
327+
stdout=stdout,
328+
stderr=stderr,
329+
set_redis_name_prefix=set_redis_name_prefix,
330+
)
331+
332+
if not wait_for_condition(time=10, condition=condition_manager_idle):
333+
failed_to_start = True
334+
manager.kill_manager()
335+
raise TimeoutError("Timeout: RE Manager failed to start.")
336+
337+
_reset_queue_mode()
338+
return manager
339+
340+
yield create_re_manager
341+
342+
_close_manager()
343+
344+
211345
def setup_server_with_config_file(*, config_file_str, tmpdir, monkeypatch):
212346
"""
213347
Creates config file for the server in ``tmpdir/config/`` directory and
@@ -236,8 +370,8 @@ def add_plans_to_queue():
236370
Clear the queue and add 3 fixed plans to the queue.
237371
Raises an exception if clearing the queue or adding plans fails.
238372
"""
239-
resp1, _ = zmq_single_request("queue_clear")
240-
assert resp1["success"] is True, str(resp1)
373+
resp1, _ = zmq_secure_request("queue_clear")
374+
assert resp1 and (resp1.get("success") is True), str(resp1)
241375

242376
user_group = _user_group
243377
user = "HTTP unit test setup"
@@ -249,8 +383,10 @@ def add_plans_to_queue():
249383
}
250384
plan2 = {"name": "count", "args": [["det1", "det2"]], "item_type": "plan"}
251385
for plan in (plan1, plan2, plan2):
252-
resp2, _ = zmq_single_request("queue_item_add", {"item": plan, "user": user, "user_group": user_group})
253-
assert resp2["success"] is True, str(resp2)
386+
resp2, _ = zmq_secure_request(
387+
"queue_item_add", {"item": plan, "user": user, "user_group": user_group}
388+
)
389+
assert resp2 and (resp2.get("success") is True), str(resp2)
254390

255391

256392
def request_to_json(
@@ -278,12 +414,16 @@ def request_to_json(
278414
kwargs.update({"auth": auth, "headers": headers})
279415

280416
method = getattr(requests, request_type)
281-
resp = method(f"http://{SERVER_ADDRESS}:{SERVER_PORT}{request_prefix}{path}", **kwargs)
417+
resp = method(
418+
f"http://{SERVER_ADDRESS}:{SERVER_PORT}{request_prefix}{path}", **kwargs
419+
)
282420
resp = resp.json()
283421
return resp
284422

285423

286-
def wait_for_environment_to_be_created(timeout, polling_period=0.2, api_key=API_KEY_FOR_TESTS):
424+
def wait_for_environment_to_be_created(
425+
timeout, polling_period=0.2, api_key=API_KEY_FOR_TESTS
426+
):
287427
"""Wait for environment to be created with timeout."""
288428
time_start = ttime.time()
289429
while ttime.time() < time_start + timeout:
@@ -295,19 +435,25 @@ def wait_for_environment_to_be_created(timeout, polling_period=0.2, api_key=API_
295435
return False
296436

297437

298-
def wait_for_environment_to_be_closed(timeout, polling_period=0.2, api_key=API_KEY_FOR_TESTS):
438+
def wait_for_environment_to_be_closed(
439+
timeout, polling_period=0.2, api_key=API_KEY_FOR_TESTS
440+
):
299441
"""Wait for environment to be closed with timeout."""
300442
time_start = ttime.time()
301443
while ttime.time() < time_start + timeout:
302444
ttime.sleep(polling_period)
303445
resp = request_to_json("get", "/status", api_key=api_key)
304-
if (not resp["worker_environment_exists"]) and (resp["manager_state"] == "idle"):
446+
if (not resp["worker_environment_exists"]) and (
447+
resp["manager_state"] == "idle"
448+
):
305449
return True
306450

307451
return False
308452

309453

310-
def wait_for_queue_execution_to_complete(timeout, polling_period=0.2, api_key=API_KEY_FOR_TESTS):
454+
def wait_for_queue_execution_to_complete(
455+
timeout, polling_period=0.2, api_key=API_KEY_FOR_TESTS
456+
):
311457
"""Wait for for queue execution to complete."""
312458
time_start = ttime.time()
313459
while ttime.time() < time_start + timeout:
@@ -331,7 +477,9 @@ def wait_for_manager_state_idle(timeout, polling_period=0.2, api_key=API_KEY_FOR
331477
return False
332478

333479

334-
def wait_for_manager_state_idle_or_paused(timeout, polling_period=0.2, api_key=API_KEY_FOR_TESTS):
480+
def wait_for_manager_state_idle_or_paused(
481+
timeout, polling_period=0.2, api_key=API_KEY_FOR_TESTS
482+
):
335483
"""Wait until manager is in 'idle' state."""
336484
time_start = ttime.time()
337485
while ttime.time() < time_start + timeout:

bluesky_httpserver/tests/test_access_control.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import pprint
44

55
import pytest
6-
from bluesky_queueserver.manager.comms import zmq_single_request
6+
from bluesky_queueserver.manager.tests.common import zmq_secure_request
77

88
from bluesky_httpserver.authorization._defaults import (
99
_DEFAULT_RESOURCE_ACCESS_GROUP,
@@ -341,7 +341,9 @@ def test_authentication_and_authorization_02(
341341
Check that returned scopes match the default scopes.
342342
"""
343343
config = config_test_all_default_roles
344-
setup_server_with_config_file(config_file_str=config, tmpdir=tmpdir, monkeypatch=monkeypatch)
344+
setup_server_with_config_file(
345+
config_file_str=config, tmpdir=tmpdir, monkeypatch=monkeypatch
346+
)
345347
fastapi_server_fs()
346348

347349
username__to_role = {
@@ -357,7 +359,9 @@ def test_authentication_and_authorization_02(
357359
for username, role in username__to_role.items():
358360
print(f"Testing access for the username {username!r}")
359361

360-
resp1 = request_to_json("post", "/auth/provider/toy/token", login=(username, username + "_password"))
362+
resp1 = request_to_json(
363+
"post", "/auth/provider/toy/token", login=(username, username + "_password")
364+
)
361365
assert "access_token" in resp1
362366
token = resp1["access_token"]
363367

@@ -754,8 +758,8 @@ def test_resource_access_01(
754758
setup_server_with_config_file(config_file_str=config, tmpdir=tmpdir, monkeypatch=monkeypatch)
755759
fastapi_server_fs()
756760

757-
resp_clear, _ = zmq_single_request("queue_clear")
758-
assert resp_clear["success"] is True, str(resp_clear)
761+
resp_clear, _ = zmq_secure_request("queue_clear")
762+
assert resp_clear and (resp_clear.get("success") is True), str(resp_clear)
759763

760764
username, password = "bob", "bob_password"
761765

bluesky_httpserver/tests/test_auth_for_websockets.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@
44
import time as ttime
55

66
import pytest
7-
from bluesky_queueserver.manager.tests.common import re_manager_cmd # noqa F401
87
from websockets.sync.client import connect
98

109
from .conftest import fastapi_server_fs # noqa: F401
1110
from .conftest import (
1211
SERVER_ADDRESS,
1312
SERVER_PORT,
13+
re_manager_cmd,
1414
request_to_json,
1515
setup_server_with_config_file,
1616
wait_for_environment_to_be_closed,
@@ -68,7 +68,9 @@ def run(self):
6868
additional_headers = {}
6969

7070
try:
71-
with connect(websocket_uri, additional_headers=additional_headers) as websocket:
71+
with connect(
72+
websocket_uri, additional_headers=additional_headers
73+
) as websocket:
7274
while not self._exit:
7375
try:
7476
msg_json = websocket.recv(timeout=0.1, decode=False)

0 commit comments

Comments
 (0)