test(surgery): cover the SurgeryPipeline::apply error path
- Dominant language
- Rust
- Stars
- 467
- Forks
- 54
- Avg merge
- 4h 25m
- Merged PRs (30d)
- 310
Description
## Summary
Add a test for `SurgeryPipeline::apply`'s error path: the error-message shape, that the failing op's name appears, and that ops queued after the failure do not run.
## Background
`apply` wraps an op failure as a formatted string and short-circuits on the first error. The five existing tests cover only the empty and success paths, so nothing pins the error-message format or the short-circuit behavior. The module already has a `NoOp` stub to model a failing op on.
## Proposed Solution
Add a `FailingOp` stub next to `NoOp`, then assert that the returned message contains the op name and the `surgery op \`...\` failed:` prefix, and that an op queued after the failing one records zero invocations. No MLX and no checkpoint needed.
## Implementation Notes
- `src/lib/mlxcel-surgery/src/pipeline.rs:80-86` (confirmed): `apply` maps each op error with `format!("surgery op \`{}\` failed: {e}", op.name())` and propagates with `?`, so a failure short-circuits the loop.
- Existing tests and the `NoOp` stub live at `:118-198`.
## Acceptance Criteria
- [ ] The short-circuit and the message shape are pinned by tests.
---
## Original Suggestion
### Title: test(surgery): cover the SurgeryPipeline::apply error path
`SurgeryPipeline::apply`'s error path has no test: nothing asserts the error-message shape, that the failing op's name appears, or that ops after the failure are not run.
## Evidence
- `src/lib/mlxcel-surgery/src/pipeline.rs:80-86` — failures wrap as `format!("surgery op \`{}\` failed: {e}", op.name())` and short-circuit
- The five existing tests (`:142-198`) cover empty/ok paths only, and the module already has a `NoOp` stub (`:118-198`) to model a `FailingOp` on
## Suggested fix
Add a `FailingOp` stub next to `NoOp` plus assertions: message contains the op name and prefix; an op queued after the failing one records zero invocations. No MLX, no checkpoint.
## Acceptance criteria
- [ ] The short-circuit and the message shape are pinned by tests
Contributor guide
Research direction
Start in src/lib/mlxcel-surgery/src/pipeline.rs, reading SurgeryPipeline::apply at lines 80-86 and the existing tests and NoOp stub at lines 118-198. Add the requested failing-op coverage and run the pipeline tests. Done means the error-message shape and failing operation name are asserted, and an operation queued afterward records zero invocations.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- testing
- Issue type
- Feature
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100