Skip to content

Commit 37ddbd4

Browse files
committed
removed duplicate parametrization
1 parent 8824fc9 commit 37ddbd4

6 files changed

Lines changed: 190 additions & 165 deletions

File tree

‎feature_integration_tests/test_cases/daemon_helpers.py‎

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,21 +108,43 @@ def get_binary_path(target: str) -> Path:
108108
return _resolve_target_path(target)
109109

110110

111-
def _pgrep_cmdline_pattern(binary_path: str) -> str:
111+
def pgrep_cmdline_pattern(binary_path: str) -> str:
112112
"""Build POSIX ERE pattern matching binary with optional arguments."""
113113
return rf"^{re.escape(binary_path)}([[:space:]]|$)"
114114

115115

116-
def _is_running(binary_path: Path) -> bool:
116+
def is_running(binary_path: str | Path) -> bool:
117117
result = subprocess.run(
118-
["pgrep", "-f", _pgrep_cmdline_pattern(str(binary_path))],
118+
["pgrep", "-f", pgrep_cmdline_pattern(str(binary_path))],
119119
capture_output=True,
120120
text=True,
121121
check=False,
122122
)
123123
return result.returncode == 0
124124

125125

126+
def first_pid(binary_path: str | Path) -> str | None:
127+
result = subprocess.run(
128+
["pgrep", "-f", pgrep_cmdline_pattern(str(binary_path))],
129+
capture_output=True,
130+
text=True,
131+
check=False,
132+
)
133+
if result.returncode != 0:
134+
return None
135+
lines = [line for line in result.stdout.splitlines() if line]
136+
return lines[0] if lines else None
137+
138+
139+
def wait_until(predicate, timeout_s: float, interval_s: float = 0.2) -> bool:
140+
deadline = time.time() + timeout_s
141+
while time.time() < deadline:
142+
if predicate():
143+
return True
144+
time.sleep(interval_s)
145+
return False
146+
147+
126148
_SETCAP_CAPS = "cap_setuid,cap_setgid,cap_sys_nice+ep"
127149

128150

@@ -220,12 +242,39 @@ def _grant_sandbox_capabilities(binary_path: Path) -> tuple[bool, str]:
220242

221243

222244
def _wait_for_apps(apps: dict[str, Path], timeout_s: float = 8.0, interval_s: float = 0.2) -> bool:
245+
return wait_until(lambda: all(is_running(path) for path in apps.values()), timeout_s, interval_s)
246+
247+
248+
_RUNTIME_ROOT_LOCK_PATH = Path("/tmp/lifecycle_fit.lock")
249+
_RUNTIME_ROOT_LOCK_TIMEOUT_S = 120.0
250+
251+
252+
def _acquire_runtime_root_lock(timeout_s: float = _RUNTIME_ROOT_LOCK_TIMEOUT_S):
253+
"""Acquire the exclusive lock guarding the shared /tmp/lifecycle_fit runtime_root.
254+
255+
Polls with a non-blocking flock instead of blocking indefinitely: an unbounded
256+
`LOCK_EX` would hang forever (and silently eat the whole CI job's timeout budget)
257+
if the lock is ever legitimately held longer than expected, giving no diagnostic
258+
signal about what's actually stuck. No lifecycle test suite should need this lock
259+
for anywhere near `timeout_s`, so failing loudly here is strictly more useful than
260+
an indefinite wait.
261+
"""
262+
lock_file = _RUNTIME_ROOT_LOCK_PATH.open("w", encoding="utf-8")
223263
deadline = time.time() + timeout_s
224-
while time.time() < deadline:
225-
if all(_is_running(path) for path in apps.values()):
226-
return True
227-
time.sleep(interval_s)
228-
return False
264+
while True:
265+
try:
266+
fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
267+
return lock_file
268+
except OSError:
269+
if time.time() >= deadline:
270+
lock_file.close()
271+
raise RuntimeError(
272+
f"Could not acquire {_RUNTIME_ROOT_LOCK_PATH} within {timeout_s}s; "
273+
"another lifecycle daemon run appears to be holding it. If no other "
274+
"lifecycle test is genuinely running, a stale process may be stuck "
275+
"holding this lock."
276+
) from None
277+
time.sleep(0.5)
229278

230279

231280
@dataclass
@@ -276,8 +325,7 @@ def start_launch_manager_daemon(
276325
instance here rather than the shared `launch_manager_daemon` fixture.
277326
"""
278327

279-
lock_file = Path("/tmp/lifecycle_fit.lock").open("w", encoding="utf-8")
280-
fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX)
328+
lock_file = _acquire_runtime_root_lock()
281329

282330
work_dir = tmp_path_factory.mktemp("lm-daemon")
283331
etc_dir = work_dir / "etc"

‎feature_integration_tests/test_cases/tests/lifecycle/test_conditional_launching.py‎

Lines changed: 41 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -18,52 +18,49 @@
1818
"""
1919

2020
import json
21-
import re
22-
import subprocess
23-
import time
2421
from pathlib import Path
2522
from typing import Any
2623

2724
import pytest
28-
from daemon_helpers import launch_manager_daemon, start_launch_manager_daemon, stop_launch_manager_daemon
25+
from daemon_helpers import (
26+
first_pid,
27+
is_running,
28+
launch_manager_daemon,
29+
start_launch_manager_daemon,
30+
stop_launch_manager_daemon,
31+
wait_until,
32+
)
2933
from test_properties import add_test_properties
3034

31-
pytestmark = [
32-
pytest.mark.daemon,
33-
pytest.mark.parametrize("version", ["rust", "cpp"], scope="class"),
34-
]
35+
pytestmark = [pytest.mark.daemon]
3536

3637

38+
@pytest.mark.parametrize("version", ["rust", "cpp"], scope="class")
3739
class TestConditionalLaunchingWithDaemon:
3840
"""Verify dependency-based conditional launching with real daemon behavior."""
3941

40-
@staticmethod
41-
def _pgrep_cmdline_pattern(binary_path: str) -> str:
42-
"""Build POSIX ERE pattern matching binary with optional arguments."""
43-
return rf"^{re.escape(binary_path)}([[:space:]]|$)"
42+
@add_test_properties(
43+
partially_verifies=["feat_req__lifecycle__launch_support"],
44+
test_type="requirements-based",
45+
derivation_technique="requirements-analysis",
46+
)
47+
def test_startup_launches_conditioned_processes(self, launch_manager_daemon: dict[str, Any], version: str) -> None:
48+
"""Verify supervised processes are launched as part of conditional startup."""
49+
daemon_info = launch_manager_daemon
50+
app_name = "rust_supervised_app" if version == "rust" else "cpp_supervised_app"
51+
app_path = str(daemon_info["apps"][version])
4452

45-
@staticmethod
46-
def _is_running(binary_path: str) -> bool:
47-
result = subprocess.run(
48-
["pgrep", "-f", TestConditionalLaunchingWithDaemon._pgrep_cmdline_pattern(binary_path)],
49-
capture_output=True,
50-
text=True,
51-
check=False,
52-
)
53-
return result.returncode == 0
53+
started = wait_until(lambda: is_running(app_path), timeout_s=8.0)
54+
assert started, f"{app_name} was not launched in conditional startup"
5455

55-
@staticmethod
56-
def _first_pid(binary_path: str) -> str | None:
57-
result = subprocess.run(
58-
["pgrep", "-f", TestConditionalLaunchingWithDaemon._pgrep_cmdline_pattern(binary_path)],
59-
capture_output=True,
60-
text=True,
61-
check=False,
62-
)
63-
if result.returncode != 0:
64-
return None
65-
lines = [line for line in result.stdout.splitlines() if line]
66-
return lines[0] if lines else None
56+
57+
class TestConditionalLaunchingDependencyOrdering:
58+
"""Verify cpp-before-rust ordering and its declaration in config.
59+
60+
Not parametrized on `version`: both tests inspect the fixed cpp/rust pair
61+
(or the static config) regardless of which scenario variant is under test
62+
elsewhere, so parametrizing here would only re-run identical assertions.
63+
"""
6764

6865
@staticmethod
6966
def _proc_start_ticks(pid: str) -> int | None:
@@ -78,29 +75,6 @@ def _proc_start_ticks(pid: str) -> int | None:
7875
except ValueError:
7976
return None
8077

81-
@staticmethod
82-
def _wait_until(predicate, timeout_s: float, interval_s: float = 0.2) -> bool:
83-
deadline = time.time() + timeout_s
84-
while time.time() < deadline:
85-
if predicate():
86-
return True
87-
time.sleep(interval_s)
88-
return False
89-
90-
@add_test_properties(
91-
partially_verifies=["feat_req__lifecycle__launch_support"],
92-
test_type="requirements-based",
93-
derivation_technique="requirements-analysis",
94-
)
95-
def test_startup_launches_conditioned_processes(self, launch_manager_daemon: dict[str, Any], version: str) -> None:
96-
"""Verify supervised processes are launched as part of conditional startup."""
97-
daemon_info = launch_manager_daemon
98-
app_name = "rust_supervised_app" if version == "rust" else "cpp_supervised_app"
99-
app_path = str(daemon_info["apps"][version])
100-
101-
started = self._wait_until(lambda: self._is_running(app_path), timeout_s=8.0)
102-
assert started, f"{app_name} was not launched in conditional startup"
103-
10478
@add_test_properties(
10579
partially_verifies=[
10680
"feat_req__lifecycle__process_ordering",
@@ -112,21 +86,20 @@ def test_startup_launches_conditioned_processes(self, launch_manager_daemon: dic
11286
def test_rust_launch_is_conditioned_on_cpp_dependency(
11387
self,
11488
launch_manager_daemon: dict[str, Any],
115-
version: str,
11689
) -> None:
11790
"""Verify rust app starts no earlier than its configured C++ dependency."""
11891
daemon_info = launch_manager_daemon
11992
cpp_path = str(daemon_info["apps"]["cpp"])
12093
rust_path = str(daemon_info["apps"]["rust"])
12194

122-
started = self._wait_until(
123-
lambda: self._is_running(cpp_path) and self._is_running(rust_path),
95+
started = wait_until(
96+
lambda: is_running(cpp_path) and is_running(rust_path),
12497
timeout_s=8.0,
12598
)
12699
assert started, "cpp_supervised_app and rust_supervised_app should both be running"
127100

128-
cpp_pid = self._first_pid(cpp_path)
129-
rust_pid = self._first_pid(rust_path)
101+
cpp_pid = first_pid(cpp_path)
102+
rust_pid = first_pid(rust_path)
130103
assert cpp_pid is not None, "Could not resolve PID for cpp_supervised_app"
131104
assert rust_pid is not None, "Could not resolve PID for rust_supervised_app"
132105

@@ -144,11 +117,7 @@ def test_rust_launch_is_conditioned_on_cpp_dependency(
144117
test_type="requirements-based",
145118
derivation_technique="requirements-analysis",
146119
)
147-
def test_dependency_is_declared_in_lifecycle_config(
148-
self,
149-
launch_manager_daemon: dict[str, Any],
150-
version: str,
151-
) -> None:
120+
def test_dependency_is_declared_in_lifecycle_config(self, launch_manager_daemon: dict[str, Any]) -> None:
152121
"""Verify runtime configuration defines rust conditional dependency on cpp."""
153122
config_path = Path(__file__).resolve().parents[3] / "configs" / "lifecycle_daemon_config.json"
154123
config = json.loads(config_path.read_text(encoding="utf-8"))
@@ -160,7 +129,6 @@ def test_dependency_is_declared_in_lifecycle_config(
160129
)
161130

162131

163-
@pytest.mark.daemon
164132
class TestConditionalLaunchingBlocksOnMissingDependency:
165133
"""Verify rust startup is actually gated on cpp, not merely correlated with it.
166134
@@ -170,30 +138,10 @@ class TestConditionalLaunchingBlocksOnMissingDependency:
170138
Safe to run its own daemon here because the bazel targets for these tests
171139
are tagged tags = ["exclusive"], guaranteeing no other lifecycle daemon is
172140
using the shared runtime_root/lock at the same time.
173-
"""
174141
175-
@staticmethod
176-
def _pgrep_cmdline_pattern(binary_path: str) -> str:
177-
return rf"^{re.escape(binary_path)}([[:space:]]|$)"
178-
179-
@classmethod
180-
def _is_running(cls, binary_path: str) -> bool:
181-
result = subprocess.run(
182-
["pgrep", "-f", cls._pgrep_cmdline_pattern(binary_path)],
183-
capture_output=True,
184-
text=True,
185-
check=False,
186-
)
187-
return result.returncode == 0
188-
189-
@staticmethod
190-
def _wait_until(predicate, timeout_s: float, interval_s: float = 0.2) -> bool:
191-
deadline = time.time() + timeout_s
192-
while time.time() < deadline:
193-
if predicate():
194-
return True
195-
time.sleep(interval_s)
196-
return False
142+
Not parametrized on `version`: dependency gating is independent of which
143+
scenario variant is under test elsewhere, so this runs exactly once.
144+
"""
197145

198146
@add_test_properties(
199147
partially_verifies=[
@@ -205,12 +153,9 @@ def _wait_until(predicate, timeout_s: float, interval_s: float = 0.2) -> bool:
205153
derivation_technique="requirements-analysis",
206154
)
207155
def test_rust_stays_down_until_cpp_dependency_becomes_available(
208-
self, tmp_path_factory: pytest.TempPathFactory, version: str
156+
self, tmp_path_factory: pytest.TempPathFactory
209157
) -> None:
210158
"""Verify rust does not start while cpp is withheld, and does once cpp is unblocked."""
211-
if version != "rust":
212-
pytest.skip("dependency-gating check is independent of the 'version' axis; runs once")
213-
214159
daemon_info = start_launch_manager_daemon(
215160
tmp_path_factory,
216161
blocked_apps=frozenset({"cpp"}),
@@ -221,15 +166,15 @@ def test_rust_stays_down_until_cpp_dependency_becomes_available(
221166
rust_path = str(daemon_info["apps"]["rust"])
222167

223168
# cpp cannot execute (mode 0o000): rust must not appear while it's withheld.
224-
rust_started_early = self._wait_until(lambda: self._is_running(rust_path), timeout_s=4.0)
169+
rust_started_early = wait_until(lambda: is_running(rust_path), timeout_s=4.0)
225170
assert not rust_started_early, (
226171
"rust_supervised_app started even though its cpp_supervised_app dependency "
227172
"was withheld (non-executable); dependency gating was not enforced"
228173
)
229174

230175
# Unblock cpp: rust should now be allowed to start.
231176
cpp_path.chmod(0o755)
232-
rust_started = self._wait_until(lambda: self._is_running(rust_path), timeout_s=8.0)
177+
rust_started = wait_until(lambda: is_running(rust_path), timeout_s=8.0)
233178
assert rust_started, (
234179
"rust_supervised_app did not start after its cpp_supervised_app dependency became available"
235180
)

‎feature_integration_tests/test_cases/tests/lifecycle/test_conditional_launching_scenario.py‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,3 +333,65 @@ def test_unsupported_prefix_is_rejected_immediately(
333333
f"Rejection took {elapsed_s:.2f}s, close to the full {self._TIMEOUT_MS}ms timeout - "
334334
"the prefix should be validated up front, not discovered by waiting it out."
335335
)
336+
337+
338+
@add_test_properties(
339+
partially_verifies=["feat_req__lifecycle__validate_conditions"],
340+
test_type="requirements-based",
341+
derivation_technique="requirements-analysis",
342+
)
343+
class TestConditionalLaunchingScenarioRejectsEmptyConditions(LifecycleScenario):
344+
"""Verify an empty wait_conditions list is rejected as invalid configuration up front,
345+
distinct from a legitimate condition that simply times out unmet.
346+
347+
Without this, a future refactor of parse_wait_conditions/parse_string_array_field could
348+
silently start treating an empty list as "nothing to wait for, immediate success" instead
349+
of a configuration error - a regression this test would otherwise be the only thing to
350+
catch, since the happy-path and unsupported-prefix tests never exercise this branch.
351+
"""
352+
353+
@pytest.fixture(scope="class")
354+
def scenario_name(self) -> str:
355+
return "lifecycle.conditional_launching"
356+
357+
_TIMEOUT_MS = 2000
358+
359+
@pytest.fixture(scope="class")
360+
def test_config(self) -> dict[str, Any]:
361+
return {
362+
"test": {
363+
"wait_conditions": [],
364+
"polling_interval_ms": 20,
365+
"timeout_ms": self._TIMEOUT_MS,
366+
},
367+
}
368+
369+
def expect_command_failure(self) -> bool:
370+
return True
371+
372+
def capture_stderr(self) -> bool:
373+
return True
374+
375+
def test_empty_conditions_are_rejected_immediately(
376+
self,
377+
command: list[str],
378+
execution_timeout: float,
379+
version: str,
380+
) -> None:
381+
"""Verify validation rejects an empty wait_conditions list outright, before entering
382+
the wait loop, rather than only surfacing the same error after the full timeout."""
383+
start = time.monotonic()
384+
result = self._run_command(command, execution_timeout)
385+
elapsed_s = time.monotonic() - start
386+
387+
assert result.return_code != ResultCode.SUCCESS, (
388+
f"Expected failure for an empty wait_conditions list, got: {result}"
389+
)
390+
assert result.stderr is not None
391+
assert "Wait conditions were not provided" in result.stderr, (
392+
f"Expected a missing/empty wait_conditions validation error on stderr, got: {result.stderr}"
393+
)
394+
assert elapsed_s < (self._TIMEOUT_MS / 1000) / 2, (
395+
f"Rejection took {elapsed_s:.2f}s, close to the full {self._TIMEOUT_MS}ms timeout - "
396+
"an empty condition list should be validated up front, not discovered by waiting it out."
397+
)

0 commit comments

Comments
 (0)