fix(test): remove conditional guards from SSR and shortcode tests#169
Conversation
Tests must assert one path directly, not accept either outcome via if/else guards. Conditional guards let tests pass without the behavior being implemented. - SSR skip behavior: cat is a valid command that must succeed. Assert success directly instead of accepting either success or error. - Pipeline shortcode bridging: unregistered shortcode must error. Assert the error directly instead of accepting either error or success. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
zeroedin
left a comment
There was a problem hiding this comment.
PR #169 Review — Remove Conditional Guards from SSR and Shortcode Tests
Scope: build_test.go (+6/-15), plugin_template_test.go (+11/-16). Removes if err != nil / if err == nil branching from two tests, making assertions unconditional.
Changes
1. Build test — "Build() runs Phase 2 when SSR config is present"
Old: if err != nil { check error references SSR } else { check result } — both branches pass, test never fails.
New: unconditional Expect(err).NotTo(HaveOccurred()) + result assertions. This is correct — cat is a valid stdin→stdout pass-through, so Build() with Command: "cat" should succeed. The old conditional guard was left over from a time when cat might have failed (the CLI-arg model where cat would try to open a file).
2. Plugin template test — "BuildWithContent renders plugin shortcode"
Old: if err != nil { check error msg } else { check result } — both branches pass.
New: unconditional Expect(err).To(HaveOccurred()) + error message assertion. The test uses {% greeting "World" %} without registering the shortcode — the build MUST fail with an unknown tag error. The old conditional was hedging against the possibility that the build might silently pass through, but the correct spec behavior is that unregistered tags are errors, not silent pass-throughs.
Verification
Both changes align with established patterns:
catas SSR command succeeds (proven in scanner tests and PR #150's per-page model)- Unregistered shortcode in Liquid template is a parse/render error (Liquid doesn't silently ignore unknown tags)
Verdict
Clean cleanup. Conditional test guards that allow both outcomes to pass are effectively untestable — they can never fail. Both tests now have a single expected outcome with unconditional assertions. No issues.
Summary
Remove
if err != nil / elseconditional guards that let tests pass without the behavior being implemented. Tests must assert one path directly.SSR skip behavior test (
build_test.go):catis a valid SSR command that reads stdin and writes stdout — it must succeed. Changed fromif err != nil { check error } else { check success }to directExpect(err).NotTo(HaveOccurred()).Pipeline shortcode bridging test (
plugin_template_test.go): An unregistered shortcode ({% greeting %}) must cause a build error. Changed fromif err != nil { check error } else { check result }to directExpect(err).To(HaveOccurred()).Test plan
if err != nilorif err == nilguards in test files (after spec(ssr): resolve conflicting error expectations in SSR tests #167 merges)Skip()calls in test files🤖 Generated with Claude Code