Simplify scenario building - #45
Conversation
There was a problem hiding this comment.
Pull request overview
This PR simplifies scenario building by removing the need for functools.partial when defining test scenarios. The refactoring introduces a context-based approach where scenarios execute steps directly, and the framework automatically collects step results through the ScenarioContext and @scenario_step decorator pattern.
Key changes:
- Introduced
ScenarioContextusing Python'scontextvarsto automatically track step results across scenario execution - Modified the
@register_scenariodecorator to wrap scenario functions and buildScenarioResultobjects from collected steps - Refactored all scenario functions to call client methods directly instead of building lists of partial functions
- Added a
flight_declarationcontext manager toFlightBlenderClientto simplify setup/teardown patterns - Updated method signatures in
FlightBlenderClientto removeoperation_idparameters (now tracked internally)
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 27 comments.
Show a summary per file
| File | Description |
|---|---|
src/openutm_verification/scenarios/registry.py |
Added _run_scenario_simple wrapper and enhanced @register_scenario decorator to automatically build ScenarioResult from collected steps |
src/openutm_verification/core/execution/scenario_runner.py |
Implemented ScenarioContext using contextvars to track step results across scenario execution |
src/openutm_verification/core/reporting/reporting_models.py |
Added SetupData model (though has structural issues with misplaced failed field) |
src/openutm_verification/core/clients/flight_blender/flight_blender_client.py |
Removed operation_id parameters from methods, added flight_declaration context manager, and setup_flight_declaration method |
src/openutm_verification/core/execution/dependencies.py |
Added data_files dependency provider for dependency injection |
src/openutm_verification/scenarios/test_*.py |
Refactored all 10 scenario files to use direct function calls instead of partial functions |
src/openutm_verification/scenarios/common.py |
Removed template functions (run_sdsp_scenario_template, run_air_traffic_scenario_template, run_scenario_template) and helper path functions, renamed private functions to public |
src/openutm_verification/cli/__init__.py |
Added exit code based on failed scenario count |
src/openutm_verification/core/execution/execution.py |
Modified to return failed scenario count |
config/default.yaml |
Enabled several scenarios for testing |
.github/workflows/main.yml |
Updated workflow triggers and added --wait flag to docker compose, plus if: always() for artifact upload |
tests/docker-compose.fb.yml |
Added healthcheck for flight-blender service |
Comments suppressed due to low confidence (3)
src/openutm_verification/scenarios/registry.py:16
- Import of 'dataclass' is not used.
from dataclasses import dataclass
src/openutm_verification/scenarios/registry.py:18
- Import of 'Any' is not used.
Import of 'List' is not used.
Import of 'Type' is not used.
Import of 'cast' is not used.
from typing import Any, List, Type, TypeVar, cast
src/openutm_verification/scenarios/registry.py:22
- Import of 'config' is not used.
from openutm_verification.core.execution.config_models import config
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…m_sim.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 18 comments.
Comments suppressed due to low confidence (2)
src/openutm_verification/core/clients/flight_blender/flight_blender_client.py:608
- The
@scenario_stepdecorator already wraps the function to return aStepResult, but this function also explicitly returns aStepResult. This creates a double-wrapping issue where aStepResultcontaining anotherStepResultwould be returned. Either:
- Remove the
@scenario_stepdecorator and handle the step result manually, or - Return the raw data and let the decorator create the
StepResult
The current implementation on line 79-82 checks if isinstance(result, StepResult) to handle this, but it's confusing and inconsistent with the pattern used in other methods.
@scenario_step("Verify SDSP Track")
def initialize_verify_sdsp_track(
self,
expected_heartbeat_interval_seconds: int,
expected_heartbeat_count: int,
session_id: str,
) -> StepResult:
src/openutm_verification/core/clients/flight_blender/flight_blender_client.py:681
- The
@scenario_stepdecorator already wraps the function to return aStepResult, but this function also explicitly returns aStepResult. This creates a double-wrapping issue where aStepResultcontaining anotherStepResultwould be returned. Either:
- Remove the
@scenario_stepdecorator and handle the step result manually, or - Return the raw data and let the decorator create the
StepResult
The current implementation on line 79-82 checks if isinstance(result, StepResult) to handle this, but it's confusing and inconsistent with the pattern used in other methods.
@scenario_step("Verify SDSP Heartbeat")
def initialize_verify_sdsp_heartbeat(
self,
expected_heartbeat_interval_seconds: int,
expected_heartbeat_count: int,
session_id: str,
) -> StepResult:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ender_client.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Remove the need for partials when building scenarios.