Skip to content

Commit 3348f8a

Browse files
committed
added review comment fixes listed below :
- H6 — C++ DependencyOrdering/ParallelLaunching now build a real score::hm::HealthMonitorBuilder + DeadlineMonitorBuilder and throw on build() failure, matching Rust's real API usage (previously pure print/sleep stubs). - L1/L2/L7 — deleted dead code in lifecycle_scenario.py (copy_test_app_to_daemon_workspace, create_launch_manager_config, read_launch_manager_config, create_daemon_integrated_config) that targeted a non-existent Bazel package and was never called. - L3 — replaced unsound regex-based JSON array parsing in the C++ scenario file with proper score::json library parsing scoped to the test object. - L4 — daemon now starts in its own process group (start_new_session=True) and stop() signals the whole group via os.killpg, preventing orphaned supervised children; log file now appends instead of truncating on restart. - L5 — recovery test now matches on the exact deployed binary path instead of a fragile bare-name pgrep -f substring match. - M1/M2/M4(remainder)/M5 — config values that coincided with scenario hardcoded defaults changed to distinctive values (e.g. watchdog_interval_ms 100→137), so a broken config-read path is now distinguishable from a default fallback. - M3 — I/O redirection paths are now genuinely config-driven in both C++ and Rust (previously hardcoded literals). - M6, M8, M9 — ConfigurationManagement, DebugAndTerminal, ControlInterfaceCommands scenario classes now branch on actual config values in both languages instead of printing fixed self-fulfilling output. - M7 — log timestamp is now a real ISO-8601/epoch timestamp, checked via regex instead of a literal string. - M10 — dependency-ordering test now verifies checkpoints are emitted in non-decreasing order, not just present. - M11 — trimmed over-claimed partially_verifies requirement lists in 3 test files down to IDs with matching assertions.
1 parent c16d90d commit 3348f8a

17 files changed

Lines changed: 520 additions & 324 deletions

feature_integration_tests/test_cases/daemon_helpers.py

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -276,18 +276,23 @@ def start(self, startup_timeout: float = 2.0) -> None:
276276
if self.process is not None:
277277
raise RuntimeError("Daemon already started")
278278

279-
# Create log file
279+
# Create/append to log file so logs from a prior start() (e.g. before a
280+
# restart) are preserved instead of being discarded.
280281
self.log_file = self.working_dir / "launch_manager.log"
281-
self._log_fd = open(self.log_file, "w")
282+
self._log_fd = open(self.log_file, "a")
282283

283-
# Start daemon process
284+
# Start daemon process in its own process group so stop() can signal
285+
# the whole supervision tree, not just the daemon itself. Otherwise
286+
# supervised child processes are orphaned on daemon crash/restart and
287+
# linger to pollute later pgrep-based assertions.
284288
cmd = [str(self.daemon_binary), str(self.config_file)]
285289
self.process = subprocess.Popen(
286290
cmd,
287291
cwd=self.working_dir,
288292
stdout=self._log_fd,
289293
stderr=subprocess.STDOUT,
290294
text=True,
295+
start_new_session=True,
291296
)
292297

293298
# Wait for daemon to initialize
@@ -315,18 +320,22 @@ def stop(self, shutdown_timeout: float = 5.0) -> None:
315320
return
316321

317322
try:
318-
# Send SIGTERM for graceful shutdown if still running.
323+
# Send SIGTERM to the whole process group for graceful shutdown if
324+
# still running, so supervised children are not orphaned.
319325
if self.process.poll() is None:
320326
try:
321-
self.process.send_signal(signal.SIGTERM)
327+
os.killpg(self.process.pid, signal.SIGTERM)
322328
except ProcessLookupError:
323329
pass
324330

325331
try:
326332
self.process.wait(timeout=shutdown_timeout)
327333
except subprocess.TimeoutExpired:
328-
# Force kill if graceful shutdown fails.
329-
self.process.kill()
334+
# Force kill the whole process group if graceful shutdown fails.
335+
try:
336+
os.killpg(self.process.pid, signal.SIGKILL)
337+
except ProcessLookupError:
338+
pass
330339
self.process.wait()
331340
finally:
332341
self.process = None

feature_integration_tests/test_cases/lifecycle_scenario.py

Lines changed: 0 additions & 196 deletions
Original file line numberDiff line numberDiff line change
@@ -17,175 +17,12 @@
1717
shared ``temp_dir`` fixture so individual test classes do not have to duplicate it.
1818
"""
1919

20-
import json
21-
import shutil
2220
from collections.abc import Generator
2321
from pathlib import Path
2422
from typing import Any
2523

2624
import pytest
2725
from fit_scenario import FitScenario, temp_dir_common
28-
from testing_utils import BazelTools
29-
30-
31-
def read_launch_manager_config(config_path: Path) -> dict[str, Any]:
32-
"""
33-
Read and parse the launch manager configuration JSON file.
34-
35-
Parameters
36-
----------
37-
config_path : Path
38-
Path to the launch manager configuration file.
39-
40-
Returns
41-
-------
42-
dict
43-
Parsed launch manager configuration.
44-
"""
45-
return json.loads(config_path.read_text())
46-
47-
48-
def create_launch_manager_config(config_path: Path, components: dict[str, Any], run_targets: dict[str, Any]) -> Path:
49-
"""
50-
Create a launch manager configuration JSON file.
51-
52-
Parameters
53-
----------
54-
config_path : Path
55-
Path where the configuration file should be created.
56-
components : dict
57-
Component definitions for the launch manager.
58-
run_targets : dict
59-
Run target definitions.
60-
61-
Returns
62-
-------
63-
Path
64-
Path to the created configuration file.
65-
"""
66-
config = {
67-
"schema_version": 1,
68-
"defaults": {
69-
"deployment_config": {
70-
"bin_dir": "/tmp/lifecycle_test/bin/",
71-
"ready_recovery_action": {"restart": {"number_of_attempts": 1, "delay_before_restart": 0.5}},
72-
"sandbox": {
73-
"uid": 0,
74-
"gid": 0,
75-
"supplementary_group_ids": [],
76-
"scheduling_policy": "SCHED_OTHER",
77-
"scheduling_priority": 1,
78-
},
79-
},
80-
"component_properties": {
81-
"application_profile": {
82-
"application_type": "Reporting",
83-
"is_self_terminating": False,
84-
},
85-
"depends_on": [],
86-
"process_arguments": [],
87-
"ready_condition": {"process_state": "Running"},
88-
},
89-
"run_target": {
90-
"transition_timeout": 5,
91-
"recovery_action": {"switch_run_target": {"run_target": "fallback_run_target"}},
92-
},
93-
},
94-
"components": components,
95-
"run_targets": run_targets,
96-
"initial_run_target": "startup",
97-
"fallback_run_target": {
98-
"description": "Fallback state",
99-
"depends_on": [],
100-
"transition_timeout": 1.5,
101-
},
102-
}
103-
config_path.write_text(json.dumps(config, indent=2))
104-
return config_path
105-
106-
107-
def create_daemon_integrated_config(
108-
config_path: Path,
109-
bin_dir: Path,
110-
components: dict[str, Any],
111-
run_targets: dict[str, Any] | None = None,
112-
enable_health_monitoring: bool = True,
113-
) -> Path:
114-
"""
115-
Create a Launch Manager configuration for daemon integration tests.
116-
117-
Parameters
118-
----------
119-
config_path : Path
120-
Path where the configuration file should be created.
121-
bin_dir : Path
122-
Directory containing application binaries.
123-
components : dict
124-
Component definitions with supervised applications.
125-
run_targets : dict, optional
126-
Run target definitions. If None, uses default startup/running/fallback.
127-
enable_health_monitoring : bool
128-
Whether to enable alive supervision for components.
129-
130-
Returns
131-
-------
132-
Path
133-
Path to the created configuration file.
134-
"""
135-
if run_targets is None:
136-
run_targets = {
137-
"startup": {"description": "System startup", "depends_on": []},
138-
"running": {"description": "Normal operation", "depends_on": []},
139-
"fallback": {"description": "Fallback mode", "depends_on": [], "transition_timeout": 5},
140-
}
141-
142-
alive_supervision = {}
143-
if enable_health_monitoring:
144-
alive_supervision = {
145-
"alive_supervision": {
146-
"reporting_cycle": 0.1,
147-
"min_indications": 1,
148-
"max_indications": 3,
149-
"failed_cycles_tolerance": 2,
150-
}
151-
}
152-
153-
config = {
154-
"schema_version": 1,
155-
"defaults": {
156-
"deployment_config": {
157-
"bin_dir": str(bin_dir) + "/",
158-
"ready_recovery_action": {"restart": {"number_of_attempts": 3, "delay_before_restart": 0.5}},
159-
"sandbox": {
160-
"uid": 0,
161-
"gid": 0,
162-
"supplementary_group_ids": [],
163-
"scheduling_policy": "SCHED_OTHER",
164-
"scheduling_priority": 1,
165-
},
166-
},
167-
"component_properties": {
168-
"application_profile": {
169-
"application_type": "Reporting",
170-
"is_self_terminating": False,
171-
**alive_supervision,
172-
},
173-
"depends_on": [],
174-
"process_arguments": [],
175-
"ready_condition": {"process_state": "Running"},
176-
},
177-
"run_target": {
178-
"transition_timeout": 10,
179-
"recovery_action": {"switch_run_target": {"run_target": "fallback"}},
180-
},
181-
},
182-
"components": components,
183-
"run_targets": run_targets,
184-
"initial_run_target": "startup",
185-
"fallback_run_target": {"description": "Fallback state", "depends_on": [], "transition_timeout": 1.5},
186-
}
187-
config_path.write_text(json.dumps(config, indent=2))
188-
return config_path
18926

19027

19128
def add_supervised_component(
@@ -235,39 +72,6 @@ def add_supervised_component(
23572
return component
23673

23774

238-
def copy_test_app_to_daemon_workspace(daemon_info: dict[str, Any], app_name: str, version: str = "rust") -> Path:
239-
"""
240-
Copy a test application binary to the daemon workspace.
241-
242-
Parameters
243-
----------
244-
daemon_info : dict
245-
Daemon information from launch_manager_daemon fixture.
246-
app_name : str
247-
Name of the test application (e.g., "supervised_test_app").
248-
version : str
249-
Implementation version: "rust" or "cpp".
250-
251-
Returns
252-
-------
253-
Path
254-
Path to the copied binary in daemon workspace.
255-
"""
256-
# Build the test application
257-
tools = BazelTools(option_prefix=version)
258-
target_suffix = "_rust" if version == "rust" else "_cpp"
259-
target = f"//feature_integration_tests/test_apps:{app_name}{target_suffix}"
260-
tools.build(target)
261-
source_path = tools.find_target_path(target)
262-
263-
# Copy to daemon bin directory
264-
dest_path = daemon_info["bin_dir"] / (app_name if version == "rust" else f"{app_name}_cpp")
265-
shutil.copy2(source_path, dest_path)
266-
dest_path.chmod(0o755)
267-
268-
return dest_path
269-
270-
27175
class LifecycleScenario(FitScenario):
27276
"""
27377
Base class for lifecycle feature integration tests.

feature_integration_tests/test_cases/tests/lifecycle/test_conditional_launching.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ def test_config(self, temp_dir: Path) -> dict[str, Any]:
6969
"test": {
7070
"test_duration_ms": 300,
7171
"wait_conditions": ["path:/tmp/ready", "env:STARTUP_COMPLETE", "process:init_done"],
72-
"polling_interval_ms": 50,
73-
"timeout_ms": 5000,
72+
"polling_interval_ms": 173,
73+
"timeout_ms": 6421,
7474
}
7575
}
7676

@@ -121,9 +121,9 @@ def test_polling_interval_configured(
121121
assert results.return_code == ResultCode.SUCCESS
122122

123123
if version == "cpp":
124-
assert "Polling interval: 50ms" in results.stdout, "Polling interval not configured"
124+
assert "Polling interval: 173ms" in results.stdout, "Polling interval not configured"
125125
else:
126-
polling_logs = logs_info_level.get_logs(field="message", pattern="Polling interval: 50ms")
126+
polling_logs = logs_info_level.get_logs(field="message", pattern="Polling interval: 173ms")
127127
assert len(polling_logs) > 0, "Polling interval not configured"
128128

129129
def test_condition_timeout_configured(
@@ -135,9 +135,9 @@ def test_condition_timeout_configured(
135135
assert results.return_code == ResultCode.SUCCESS
136136

137137
if version == "cpp":
138-
assert "Condition timeout: 5000ms" in results.stdout, "Condition timeout not configured"
138+
assert "Condition timeout: 6421ms" in results.stdout, "Condition timeout not configured"
139139
else:
140-
timeout_logs = logs_info_level.get_logs(field="message", pattern="Condition timeout: 5000ms")
140+
timeout_logs = logs_info_level.get_logs(field="message", pattern="Condition timeout: 6421ms")
141141
assert len(timeout_logs) > 0, "Condition timeout not configured"
142142

143143
def test_dependency_check(self, results: ScenarioResult, logs_info_level: LogContainer, version: str) -> None:

feature_integration_tests/test_cases/tests/lifecycle/test_configuration_management.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,16 @@ def test_modular_config_support(self, results: ScenarioResult, logs_info_level:
7272
assert results.return_code == ResultCode.SUCCESS
7373

7474
if version == "cpp":
75-
assert "Modular configuration loaded" in results.stdout, "Modular configuration not supported"
75+
for module in ["base", "extended", "runtime"]:
76+
assert f"Configuration module loaded: {module}" in results.stdout, (
77+
f"Configuration module {module} not loaded"
78+
)
7679
else:
77-
config_logs = logs_info_level.get_logs(field="message", value="Modular configuration loaded")
78-
assert len(config_logs) > 0, "Modular configuration not supported"
80+
for module in ["base", "extended", "runtime"]:
81+
config_logs = logs_info_level.get_logs(
82+
field="message", pattern=f"Configuration module loaded: {module}"
83+
)
84+
assert len(config_logs) > 0, f"Configuration module {module} not loaded"
7985

8086
def test_oci_compatibility(self, results: ScenarioResult, logs_info_level: LogContainer, version: str) -> None:
8187
"""

feature_integration_tests/test_cases/tests/lifecycle/test_control_commands.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ def test_config(self, temp_dir: Path) -> dict[str, Any]:
5656
return {
5757
"test": {
5858
"test_duration_ms": 200,
59-
"commands": ["start", "stop", "status", "activate_run_target"],
59+
"commands": ["pause", "resume", "diagnostics", "activate_run_target"],
6060
}
6161
}
6262

@@ -69,14 +69,23 @@ def test_control_commands_available(
6969
assert results.return_code == ResultCode.SUCCESS
7070

7171
if version == "cpp":
72-
assert "Control commands available: start, stop, activate_run_target" in results.stdout, (
72+
assert "Control commands available: pause, resume, diagnostics, activate_run_target" in results.stdout, (
7373
"Control commands not available"
7474
)
75+
for command in ["pause", "resume", "diagnostics", "activate_run_target"]:
76+
assert f"Executing configured command: {command}" in results.stdout, (
77+
f"Configured command {command} was not executed"
78+
)
7579
else:
7680
commands_logs = logs_info_level.get_logs(
77-
field="message", pattern="Control commands available: start, stop, activate_run_target"
81+
field="message", pattern="Control commands available: pause, resume, diagnostics, activate_run_target"
7882
)
7983
assert len(commands_logs) > 0, "Control commands not available"
84+
for command in ["pause", "resume", "diagnostics", "activate_run_target"]:
85+
exec_logs = logs_info_level.get_logs(
86+
field="message", pattern=f"Executing configured command: {command}"
87+
)
88+
assert len(exec_logs) > 0, f"Configured command {command} was not executed"
8089

8190
def test_query_commands_available(
8291
self, results: ScenarioResult, logs_info_level: LogContainer, version: str

feature_integration_tests/test_cases/tests/lifecycle/test_control_interface_support.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def test_config(self, temp_dir: Path) -> dict[str, Any]:
5353
return {
5454
"test": {
5555
"test_duration_ms": 200,
56-
"condition_name": "app_ready",
56+
"condition_name": "custom_startup_barrier",
5757
}
5858
}
5959

@@ -66,9 +66,13 @@ def test_custom_condition_signaled(
6666
assert results.return_code == ResultCode.SUCCESS
6767

6868
if version == "cpp":
69-
assert "Signaling custom condition: app_ready" in results.stdout, "Custom condition was not signaled"
69+
assert "Signaling custom condition: custom_startup_barrier" in results.stdout, (
70+
"Custom condition was not signaled"
71+
)
7072
else:
71-
condition_logs = logs_info_level.get_logs(field="message", pattern="Signaling custom condition: app_ready")
73+
condition_logs = logs_info_level.get_logs(
74+
field="message", pattern="Signaling custom condition: custom_startup_barrier"
75+
)
7276
assert len(condition_logs) > 0, "Custom condition was not signaled"
7377

7478
def test_control_interface_integration(

0 commit comments

Comments
 (0)