Dynamic phase breakpoints: is it worth a hook in the engine?
- Dominant language
- Python
- Stars
- 722
- Forks
- 237
- Avg merge
- 11h 31m
- Merged PRs (30d)
- 4
Description
# Dynamic phase breakpoints: working today in user space, worth a hook in the engine?
**Suggested category:** Ideas
---
## TL;DR
- I want to dynamically halt a running sequence before a given phase, decided **at runtime**, e.g. to probe hardware and then continue. Breakpoint on a step in TestStand, `OfferBreak` in OpenTAP , if somebody is familiar with those.
- The engine has no equivalent today. `run_under_pdb` is the closest thing and it is static and phase-internal.
- **It already works with zero core changes**, via `apply_to_all_phases` to splice in a gate phase, `run_if` to keep unarmed gates out of the record entirely, and the existing `station_server` plug RPC to arm it mid-run. Abort while halted behaves correctly.
- The user-space version cannot guard groups, branches or subtests, and it rewrites the node tree.
- **Proposal:** one `offer_break(node)` call in `TestExecutor._execute_node` plus a `BreakHandler` interface installed via `TestOptions`, defaulting to `None`. One `is None` check per node, no behavioural change unless you opt in.
- **Ask:** do you want this in the engine, or should it stay a user-space recipe? Either way, would you take the recipe as an `examples/` script? I am happy to do the implementation, tests and CLA.
---
## What I am after
On the bench I regularly want to halt a running sequence *before* a specific phase, probe a signal with a scope or a DMM, and then let the sequence continue. The key word is dynamic: I want to decide to break while the test is already running, not commit to a `user_input` phase when I write the test.
This is the same concept as a breakpoint on a step in TestStand, or `OfferBreak` / `TestPlan.BreakOffered` in OpenTAP. Both engines treat "a UI may block plan execution at a step boundary" as a first-class concept.
I could not find any prior discussion of this in the tracker, so apologies if I missed it.
## What exists today
As far as I can tell the only debug affordance in the engine is `PhaseOptions.run_under_pdb`, which is decided at declaration time and drops you into pdb *inside* the phase. Useful, but it is not a between-phases halt and it cannot be armed at runtime.
## A working implementation with no core changes
Before proposing anything, I built it in user space against 1.6.1, and it works. Two existing features carry it:
**1. `PhaseSequence.apply_to_all_phases` recurses the whole node tree.** Since `PhaseDescriptor.apply_to_all_phases` just returns `func(self)`, returning a two-node sequence splices a gate phase in front of every phase, everywhere, in one call:
```python
def with_breakpoints(*nodes):
return phase_collections.PhaseSequence(*nodes).apply_to_all_phases(
lambda phase: phase_collections.PhaseSequence(_make_gate(phase.name), phase))
```
**2. `run_if` is evaluated before any `PhaseState` or `PhaseRecord` is created**, so an unarmed gate is completely invisible in the record:
```python
@htf.PhaseOptions(name='breakpoint:%s' % target,
timeout_s=60 * 60 * 24,
run_if=lambda: BP.should_break(target))
@htf.plug(prompts=user_input.UserInput)
def _gate(test, prompts):
BP.wait_for_resume(target, prompts)
```
For arming it at runtime I used the existing `station_server` plug endpoint, which will call any public method on any plug with `enable_remote = True`. Arming a breakpoint from an external client mid-run:
```
POST /tests//plugs/htf_breakpoints.BreakpointPlug
{"method": "arm", "args": ["phase_two"]}
```
and the resulting record:
```
trigger_phase 1 ms
phase_one 2000 ms
breakpoint:phase_two 1049 ms <- halted here until resumed
phase_two 1 ms
```
No trace of the gates in front of `trigger_phase` or `phase_one`, and the halt is recorded where it happened, which turns out to be a nice audit property. `UserInput.respond` is already remote-callable, so the stock operator UI's Okay button works as the resume button unmodified. Aborting while halted also behaves correctly: `Outcome.ABORTED` within 0.1 s.
## Where the user-space version runs out
- It only guards `PhaseDescriptor` nodes. I cannot break before a `PhaseGroup`, a `BranchSequence`, a `Subtest` or a `Checkpoint`, because `apply_to_all_phases` hands me phases only.
- Every guarded phase becomes a nested `PhaseSequence`, so the node tree I hand to `htf.Test` no longer looks like the one I wrote. That is fine for me and unpleasant for anyone reading the frontend's phase list.
- It is opt-in per test at construction time. There is no way to attach a breakpoint to an already-constructed `Test`.
## The engine hook I would propose
One call at the existing central dispatch point, `TestExecutor._execute_node`, which already sees every node type uniformly:
```python
def _execute_node(self, node, subtest_rec, in_teardown):
self._offer_break(node) # no-op when no handler is installed
...
```
with a small handler interface installed through `TestOptions`, defaulting to `None`:
```python
class BreakHandler(abc.ABC):
@abc.abstractmethod
def offer_break(self, node, test_state, abort_event) -> None:
"""May block until resumed. Must return promptly if abort_event is set."""
```
Default behaviour is one `is None` check per node, and no behavioural change for anyone who does not opt in. The naming deliberately echoes OpenTAP so the concept is recognisable.
## Design notes I have already worked through
These are the sharp edges, in case they shorten the review:
- **The wait must not silently break abort.** `PhaseExecutor.stop()` only kills `_current_phase_thread` and returns early if there is none, so anything blocking on the executor thread is uninterruptible. I measured this: an abort requested at t=0.5 s while blocked in a `run_if` was only honoured at t=5.0 s. That is why the interface passes `abort_event`. `TestExecutor._abort` is already a `threading.Event`, so handing it to the handler is cheap and makes a cancellable wait the obvious implementation.
- **Handlers should poll rather than block forever.** `KillableThread` injects an asynchronous exception that is only observed when the thread runs bytecode.
- **Break state belongs on `TestState`, not module scope.** My prototype used a process-global controller, which is wrong for multiple `Test` instances in one process. The upstream version should be reachable through `TestApi`.
- **`timeout_s=None` means `DEFAULT_PHASE_TIMEOUT_S`, not "no timeout".** Relevant only to the user-space variant, but it is a trap worth documenting either way.
## Questions
1. Is a break/pause hook something you want in the engine at all, or does this belong permanently in user space as a recipe? A "no, keep it out of core" is a perfectly good answer and I will not be offended by it.
2. If yes, is `_execute_node` the right seam, or would you rather see it in `PhaseExecutor` so that it only ever affects phases?
3. Does a hook that blocks the executor thread create problems for internal users I cannot see, particularly around teardown and the `_stopping` handling that was recently touched in #1311?
4. Would you take the user-space version as an `examples/` script regardless of the outcome on the hook? That is a much smaller ask and it would at least make the pattern discoverable.
## What I am offering
If the answer to (1) is yes, I am happy to do the whole thing: the hook, a reference handler, unit tests covering break plus abort plus timeout interaction, an `examples/` script, and the CLA. I would rather agree on the seam here than open a PR and have the shape be wrong.
Happy to publish my current implementation as a gist or a branch if that makes the discussion more concrete.
Contributor guide
Research direction
Start by reading TestExecutor._execute_node, TestOptions, TestState, and PhaseExecutor.stop(), then review the teardown changes referenced in #1311. Compare the existing apply_to_all_phases and station_server approach with the proposed engine hook. The issue is done only after the project agrees whether the hook belongs in core, identifies the seam, and defines an implementation and test scope.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100