microsoft / microsoft/PyRIT

TEST Cover cancelled scenario readback cleanup

Open
#2,637 0 comments 0 reactions 0 assignees View on GitHub
feature-request
Dominant language
Python
Stars
4.5k
Forks
893
Avg merge
3d 50m
Merged PRs (30d)
165

Description

#### Is your feature request related to a problem? Please describe.

`ScenarioRunService` has deterministic coverage for cancellation, terminal progress, and deferred active-task cleanup after success or failure, but it does not directly cover the cancelled readback boundary:

1. a scenario run is blocked inside `run_async()`;
2. `cancel_run_async()` cancels and awaits that task;
3. persistence reports the run as `ScenarioRunState.CANCELLED`;
4. `get_run()` reads the terminal database state;
5. the completed/cancelled entry is removed from `_active_tasks`.

No production defect has been reproduced. The missing regression matters because a stale active-task entry could make the in-memory lifecycle state disagree with the database, retain task/scenario objects longer than necessary, or affect subsequent status reads.

#### Describe the solution you'd like

Add a deterministic unit test in `tests/unit/backend/test_scenario_run_service.py` that:

- starts or installs a scenario task whose `run_async()` is blocked on an `asyncio.Event`;
- waits until execution is definitely blocked before cancelling;
- calls and awaits `cancel_run_async()`;
- configures the persisted scenario header/result to report `CANCELLED`;
- calls `get_run()`;
- asserts the returned state is the terminal persisted `CANCELLED` state;
- verifies the stale active-task entry is cleaned up.

Prefer asserting observable behavior where possible. If direct inspection of `_active_tasks` is necessary to prove cleanup, keep that assertion narrowly scoped and document the intended internal invariant through the test name and setup rather than changing the public API solely for testing.

The test should control task scheduling with events rather than sleeps, verify cancellation is fully awaited, and ensure cleanup happens exactly once.

#### Describe alternatives you've considered, if relevant

Existing tests separately cover cancelled progress deltas and active-task cleanup after successful or failed runs, but neither proves the combined cancelled-readback path. A broad integration test would be slower and less deterministic than a focused service unit test.

Changing production code without first reproducing a failure is not proposed. If the test exposes a mismatch between persisted terminal state and in-memory cleanup, the smallest behavior-preserving fix should be included with the regression.

#### Additional context

This boundary was identified during the September 12, 2026 deterministic resilience audit with medium-high confidence. The audit found no exact existing issue or test covering it.

Suggested validation:

- the new focused `ScenarioRunService` test;
- the complete backend scenario-run service test module;
- cancellation and progress route tests if production code changes;
- Ruff, typing, and `git diff --check`.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in tests/unit/backend/test_scenario_run_service.py and read the existing cancellation, terminal-progress, and active-task cleanup tests around ScenarioRunService. Use an asyncio.Event to control the blocked run_async() task, then exercise cancel_run_async() and get_run(); done means the persisted state reads as CANCELLED and the stale _active_tasks entry is cleaned up, with the focused module and listed validation checks passing.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, testing-qa
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.