feat(cli): extract JUnit formatter + step-cmd hints + message= fallback#900
Merged
Conversation
Cherry-picks the three salvageable deltas from closed PR #894 (which overlapped heavily with merged #891 after a long rebase drift): 1. Extracts `formatStoryboardResultsAsJUnit` from `bin/adcp.js` into `src/lib/testing/storyboard/junit.ts` (@internal). Now unit-testable as a pure function — 10 tests pinning XML structure, hint integration, and escaping added in `test/lib/storyboard-junit-formatter.test.js`. 2. Wires `printStepHints` into `adcp storyboard step` — a printer site that #891 missed. Parameterizes the indent so both the phase-nested (` `) and column-zero step printers share the helper. 3. Adds `<failure message=…>` attribute fallback to `hints?.[0]?.message` when `step.error` is absent. Closes an edge opened by the #883 gate widening (validation-only failures with no task-level error): CI dashboards that only read the attribute now surface the hint. Drops the now-unused `formatHintsForFailureBody` export from `adcp-step-hints.js` and its tests — the JUnit module owns its own hint rendering. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code-reviewer flagged: the file-level `@internal` JSDoc was binding to
the next declaration (the import of `StoryboardResult`), so
stripInternal removed the import but left the function signature
referencing an undeclared name. Emitted `.d.ts` was:
export declare function formatStoryboardResultsAsJUnit(
results: StoryboardResult[]
): string;
— any downstream TS consumer hit TS2304. Fixed by moving the @internal
annotation onto the function itself; d.ts now correctly emits
`export {};`.
Also drops the re-export from `storyboard/index.ts` — stripInternal
already strips it from the public type surface and the CLI reaches into
`dist/lib/testing/storyboard/junit.js` directly, so the re-export was
dead for every consumer.
Testing-reviewer nits addressed:
- New test: empty results produces valid empty testsuites element
- New test: multi-storyboard + multi-phase aggregation (catches off-by-
ones in totalTests / suiteTests reducers)
- New test: `step.error === ''` (empty string, not undefined) → first
hint message fallback kicks in
- Replaced brittle escape-pipeline-reconstruction regex with explicit
entity assertions; scoped `'` check to the `message=` attribute
where it's actually load-bearing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the salvageable deltas from #894 (closed as obsolete). Follow-up to #870 / #875 / #879 / #883 / #891.
Summary
Three cherry-picked bits from the abandoned #894:
Extract
formatStoryboardResultsAsJUnitfrom `bin/adcp.js` into `src/lib/testing/storyboard/junit.ts` (@internal). Was 70 lines of inline JS that required spawning the CLI to test. Now a pure function with unit coverage at `test/lib/storyboard-junit-formatter.test.js` — XML structure, hint integration, escaping.Wire hints into `adcp storyboard step` — a printer site I missed in feat(cli): render StoryboardStepResult.hints in console + JUnit (#879) #891. The phase-nested storyboard printer uses a 3-space indent; the single-step command prints at column zero. Parameterized `printStepHints(hints, indent)` so both callers share the helper.
`` attribute fallback to `hints?.[0]?.message` when `step.error` is absent. Closes an edge opened by the Runner hints: extend gate beyond !passed to cover schema-rejected-200 responses #883 gate widening — validation-only failures under a 200-OK envelope have no task-level error, so CI dashboards that only read the attribute used to see `validation failed` instead of the diagnosis.
Cleanup
Drops the unused `formatHintsForFailureBody` export from `adcp-step-hints.js` now that the JUnit module owns its own hint rendering. The CLI `adcp-step-hints.js` is now single-purpose (human console only).
Test plan
🤖 Generated with Claude Code