added test cases for launch processes and conditional launching - #298
added test cases for launch processes and conditional launching#298Saumya-R wants to merge 7 commits into
Conversation
|
The created documentation from the pull request is available at: docu-html |
There was a problem hiding this comment.
Pull request overview
This PR adds a new Lifecycle “conditional launching” feature-integration test scenario and a Python FIT suite that runs it against both the Rust and C++ scenario runners. It also introduces a Rust “lifecycle-only” Bazel binary variant intended to avoid pulling in persistency-related dependencies for this specific suite, and updates test harness/configuration to support selective scenario builds.
Changes:
- Added Lifecycle conditional-launching scenarios to the Rust and C++ FIT scenario trees.
- Added a Python requirements-based FIT suite for conditional launching (parametrized for Rust/C++) plus a shared lifecycle base scenario fixture.
- Added a Rust lifecycle-only Bazel scenario binary and updated FIT build selection/docs.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Updates pytest configuration section to tool.pytest.ini_options. |
| feature_integration_tests/test_scenarios/rust/src/scenarios/mod.rs | Registers the new lifecycle scenario group in the Rust scenario tree. |
| feature_integration_tests/test_scenarios/rust/src/scenarios/lifecycle/mod.rs | Adds a lifecycle scenario group (Rust). |
| feature_integration_tests/test_scenarios/rust/src/scenarios/lifecycle/conditional_launching.rs | Implements the Rust conditional-launching scenario input validation/logging. |
| feature_integration_tests/test_scenarios/rust/src/main.rs | Adds lifecycle_only compilation mode and a reduced root group for lifecycle-only builds. |
| feature_integration_tests/test_scenarios/rust/BUILD | Adds rust_lifecycle_test_scenarios Bazel target using --cfg=lifecycle_only. |
| feature_integration_tests/test_scenarios/cpp/src/scenarios/mod.cpp | Registers a lifecycle scenario group (C++). |
| feature_integration_tests/test_scenarios/cpp/src/scenarios/lifecycle/conditional_launching.h | Declares the C++ conditional-launching scenario factory. |
| feature_integration_tests/test_scenarios/cpp/src/scenarios/lifecycle/conditional_launching.cpp | Implements the C++ conditional-launching scenario parsing/logging. |
| feature_integration_tests/test_cases/tests/lifecycle/test_conditional_launching.py | Adds Python FIT assertions for lifecycle conditional launching (Rust/C++). |
| feature_integration_tests/test_cases/lifecycle_scenario.py | Introduces a shared LifecycleScenario base class with a common temp_dir fixture. |
| feature_integration_tests/test_cases/conftest.py | Skips building unselected scenario variants based on the pytest mark expression. |
| feature_integration_tests/test_cases/BUILD | Adds lifecycle_scenario.py to FIT runfiles. |
| feature_integration_tests/README.md | Documents running lifecycle conditional-launching FITs and the lifecycle-only Rust target. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Saumya-R
changed the title
816b818 to
1c7d861
Compare
PiotrKorkus
left a comment
PiotrKorkus
left a comment
There was a problem hiding this comment.
also remove duplicated dependency/ordering tests across daemon files
7bc8cd9 to
d1cddfa
Compare
Copyright fixes added test case to reject unknown conditions fixed the readme added launch manager processes adding additional test cases for lifecycle removing the changes in pyproject and requirements txt added review comment fixes: - cleared the non-required. - removed the dead LIFECYCLE_TESTS_SUMMARY.md doc reference. - dropped the class-level 13-req partially_verifies blanket claim; each requirement is now tagged on the specific test that actually exercises it (add_test_properties moved onto individual methods) - deleted test_config_defines_startup_retry_policy Adding review comments added the following fixes : - The scenario now actually polls and checks each condition instead of printing it - Replaced plain std::cout text with the same structured JSON log shape the Rust tracing subscriber emits - TestConditionalLaunchingScenario now creates a real flag file, sets a real env var, and spawns a real sleep process - Added TestConditionalLaunchingScenarioTimesOutOnUnmetConditions, a negative test where none of the conditions are ever satisfied - removed test_startup_launches_supervised_apps and test_dependency_gates_rust_startup from test_process_launching_with_daemon.py - TestConditionalLaunchingBlocksOnMissingDependency spins up its own launch_manager with cpp withheld to prove real gating added TestConditionalLaunchingScenarioRejectsUnsupportedPrefix
3d81256 to
37ddbd4
Compare
Signed-off-by: Saumya Rai <saumya.rai@qorix.ai>
| assert started, f"{app_name} was not launched in conditional startup" | ||
|
|
||
|
|
||
| class TestConditionalLaunchingDependencyOrdering: |
There was a problem hiding this comment.
This class is silently deselected in both CI targets.
conftest.py:96-105 only attaches the rust/cpp marker to items whose callspec contains version. This class deliberately opts out of parametrization, so it carries neither marker — and both bazel targets run with -m rust / -m cpp. Reproduced with the repo's pinned pytest 9.0.1 on a minimal repro of the same hook: -m rust → 1/3 collected, -m cpp → 1/3 collected.
Net effect: waitfor_support, dependency_check, cond_process_start, process_ordering and define_swc_dependencies have zero executing coverage.
The root cause is conceptual: everywhere else in the suite version means "which scenario-binary language variant", but in the daemon tests it means "which supervised app to inspect", and it is simultaneously overloaded as the bazel target selector. Either mark these classes explicitly with both @pytest.mark.rust and @pytest.mark.cpp, or — better — stop using the language marker to route daemon tests at all, since no scenario binary is involved in them (fit_rust_lifecycle builds rust_lifecycle_test_scenarios, which these tests never touch).
| ) | ||
|
|
||
|
|
||
| class TestConditionalLaunchingBlocksOnMissingDependency: |
There was a problem hiding this comment.
Same deselection applies here — and this is the test that was written to answer the review.
TestConditionalLaunchingBlocksOnMissingDependency is non-parametrized, so it gets neither the rust nor the cpp marker and is deselected by both bazel targets. The negative-gating test added specifically in response to "what are we missing to fully verify those reqs?" never runs in CI.
See the fix options in the comment on line 57.
| effective_uid, _ = proc_ids | ||
| assert effective_uid != 0, f"{app_name} is unexpectedly running as root" | ||
|
|
||
| @add_test_properties( |
There was a problem hiding this comment.
retries_configurable is claimed against a config with retries disabled.
lifecycle_daemon_config.json:8-12 sets "ready_recovery_action": {"restart": {"number_of_attempts": 0}}. The app comes back because recovery_action switches to fallback_run_target, which also depends on both apps — that is run-target switching, not a retry. Nothing in this test varies or observes a retry count, so "configurable" is unverified.
configs/lifecycle_daemon_retries_config.json was clearly written for exactly this case but is completely unreferenced (see the comment on that file). Either wire it up and write a real retry-exhaustion test, or drop the retries_configurable tag here.
| "shutdown_timeout": 2.0, | ||
| "ready_recovery_action": { | ||
| "restart": { | ||
| "number_of_attempts": 0 |
There was a problem hiding this comment.
number_of_attempts: 0 disables retries.
test_supervised_app_recovery (test_process_launching_with_daemon.py:382-420) claims feat_req__lifecycle__retries_configurable against this config. With 0 attempts, recovery is entirely via the switch_run_target branch below, so no retry behaviour is exercised.
| @@ -0,0 +1,109 @@ | |||
| { | |||
There was a problem hiding this comment.
This entire 109-line config is unreferenced.
It is not in configs/BUILD, not in exports_files, not used by any test, and its cpp_retry_exhaustion_app binary is never staged into bin_dir. It looks written precisely for the retry test that retries_configurable currently claims — so either wire it up and add that test, or delete the file along with the requirement tag on test_supervised_app_recovery.
| def capture_stderr(self) -> bool: | ||
| return True | ||
|
|
||
| def test_unsupported_prefix_is_rejected_immediately( |
There was a problem hiding this comment.
The scenario binary is executed twice.
This calls self._run_command(...) in the test body, but the autouse print_to_report -> logs -> results chain has already run the scenario. TestConditionalLaunchingScenarioDetectsConditionArrivingLate correctly overrides results to avoid exactly this; the two Rejects* classes did not.
| def capture_stderr(self) -> bool: | ||
| return True | ||
|
|
||
| def test_empty_conditions_are_rejected_immediately( |
There was a problem hiding this comment.
Same double execution as line 313 — _run_command in the test body on top of the autouse results fixture.
| # Build the lifecycle-only Rust FIT from the same main.rs entrypoint. | ||
| # This target exists as a workaround: the full Rust FIT graph currently | ||
| # hits unresolved ScoreDebug issues in score_persistency/rust_kvs. | ||
| # Keep this until score_persistency is fixed upstream. |
There was a problem hiding this comment.
Untracked condition gating a permanent second build graph.
"Keep this until score_persistency is fixed upstream" has no issue link, so nothing will ever tell us when this can go away. Please link the upstream issue (or file one) in the comment.
|
|
||
| for (const auto& element : array_res.value().get()) { | ||
| const auto converted = convert(element); | ||
| if (!converted.has_value()) { |
There was a problem hiding this comment.
Stray extra indentation here and at ~204 and ~260-261 — the format.yml workflow will likely flag these.
| ) | ||
|
|
||
| test_suite( | ||
| name = "fit_cpp", |
There was a problem hiding this comment.
No fit_cpp_orch counterpart to fit_rust_orch.
Correct today, since the only test in tests/basic/ is @pytest.mark.rust — but the asymmetry is silent. A future cpp-marked basic test would simply never run, with nothing to indicate why.
This branch introduces lifecycle integration coverage for Launching Processes and Conditional Launching using a real launch_manager daemon setup, updates test infrastructure to reduce false negatives, and refreshes documentation/roadmap artifacts.
feat_req__lifecycle__parallel_launch_support,feat_req__lifecycle__process_launch_args,feat_req__lifecycle__uid_gid_support,feat_req__lifecycle__launch_priority_support,feat_req__lifecycle__scheduling_policy,feat_req__lifecycle__retries_configurable,feat_req__lifecycle__secpol_non_root,feat_req__lifecycle__monitor_abnormal_termTestProcessLaunchingWithDaemon)pytest.mark.manual)feat_req__lifecycle__liveliness_detection,feat_req__lifecycle__smart_watchdog_configTestHealthMonitoringWithDaemon)feat_req__lifecycle__launch_support,feat_req__lifecycle__waitfor_support,feat_req__lifecycle__cond_process_start,feat_req__lifecycle__dependency_check,feat_req__lifecycle__process_orderingTestConditionalLaunchingWithDaemon,TestConditionalLaunchingBlocksOnMissingDependency)feat_req__lifecycle__total_wait_time_support,feat_req__lifecycle__polling_interval,feat_req__lifecycle__path_condition_check,feat_req__lifecycle__env_variable_cond_check,feat_req__lifecycle__dependency_check,feat_req__lifecycle__validate_conditionsTestConditionalLaunchingScenario,TestConditionalLaunchingScenarioTimesOutOnUnmetConditions,TestConditionalLaunchingScenarioRejectsUnsupportedPrefix)