fix(test): update exec-default and timeout tests to expect skip-on-failure#170
Conversation
…ilure (#168) Two SSR tests still expected fatal errors after #166 aligned the other SSR tests to error collection. All SSR failures are per-page — page is skipped, original HTML preserved, build continues. - "defaults to exec mode": assert original HTML preserved instead of fatal error. Still proves exec mode is used (not stream). - "enforces ssrCfg.Timeout": assert page skipped with original HTML instead of fatal error. Timeout kills the command but the build continues. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
zeroedin
left a comment
There was a problem hiding this comment.
PR #170 Review — Align Exec-Default and Timeout Tests with Skip-on-Failure Model
Scope: internal/pipeline/build_test.go (+21/-13). Updates two remaining tests that still expected fatal errors from BuildPhase2.
Changes
1. "BuildPhase2 defaults to exec mode when mode is empty"
Old: Expect(err).To(HaveOccurred()) + error message contains "nonexistent-ssr-tool" / "exec" / "not found"
New: Expect(err).NotTo(HaveOccurred()) + result has key + original HTML preserved with ds-card
The test's purpose shifts from "prove exec mode by checking the error message" to "prove exec mode by checking the page survived with original HTML." The new approach still proves exec mode was used (not stream) — if stream mode were used, NewStreamRenderer would fail differently (at process startup, before any page processing).
2. "BuildPhase2 exec mode enforces ssrCfg.Timeout"
Old: Expect(err).To(HaveOccurred()) — timeout kills process, build aborts with error
New: Expect(err).NotTo(HaveOccurred()) + timed-out page preserved with original HTML
Consistent with #164/#167: timeout is a per-page failure, not a build abort. The page that timed out gets its original HTML preserved; the build continues.
Completeness Check
With PRs #167 + #169 + #170, every BuildPhase2 test with a failing/nonexistent SSR command now uses the skip-on-failure model:
| Test | Old Expectation | New Expectation |
|---|---|---|
| Phase 2 invokes command per page | fatal error | skip, preserve HTML |
| Command not found | fatal error | skip, preserve HTML |
| No local DSD fallback | conditional | unconditional, preserve HTML |
| Exec-default mode (#170) | fatal error | skip, preserve HTML |
| Timeout (#170) | fatal error | skip, preserve HTML |
| Skip failed pages (#164) | unconditional | unchanged |
| Preserve original HTML (#164) | unconditional | unchanged |
All seven tests now agree: SSR failure = per-page skip + original HTML preserved + no build abort.
Verdict
Correct. Completes the error model alignment started in #167. No remaining tests expect fatal errors from SSR command failures. No issues.
Summary
Fixes #168
Two SSR tests still expected fatal errors after #166/#167 aligned the other SSR tests to error collection. All SSR failures are per-page — page is skipped, original HTML preserved, build continues.
Test plan
Expect(err).To(HaveOccurred())for SSR command failures inbuild_test.go🤖 Generated with Claude Code