Skip to content

Commit 5cfa451

Browse files
gracexmatinclaude
andcommitted
docs(plan): require negative tests to assert the failure reason
The Task 1 review caught a weak assertion this plan mandated verbatim: unknown_fields_are_rejected checked only is_err(), which passes when an unrelated error fires. Fix the offending snippet and lift the rule into Global Constraints so the remaining tasks inherit the bar open_connector/config.rs:607-619 already sets. Also record that cargo is not on this machine's default PATH. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c042341 commit 5cfa451

1 file changed

Lines changed: 13 additions & 3 deletions

File tree

docs/superpowers/plans/2026-07-27-rss-feed-provider-m1-m2.md

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ Copied from the spec; every task's requirements implicitly include these.
2828
- **Config defaults (exact values):** `ttl_seconds: 900`, `max_concurrent: 6`, `request_timeout_seconds: 10`, `scan_timeout_seconds: 60`, `max_response_bytes: 5242880`, `user_agent: "skardi-rss/<CARGO_PKG_VERSION> (+https://github.com/SkardiLabs/skardi)"`. `ttl_seconds: 0` is valid (always-live). All bounds ≥ 1 except `ttl_seconds` (0 allowed).
2929
- **Failure fuse:** on a failed attempt the TTL re-arms to `clamp(ttl_seconds / 4, 30, 300)` seconds — bounded above zero even under `ttl_seconds: 0`.
3030
- **Every commit compiles and its tests pass** with `--features rss` and without (the workspace must stay green when the feature is off). Run `cargo fmt --all` before every commit (CI gates on it).
31+
- **Negative tests assert the reason, not just the failure.** Every test that expects an error asserts on a substring identifying *that* error (the offending field name, the failing stage, the blocked range). A bare `assert!(result.is_err())` passes when an unrelated error fires and is treated as a test that asserts nothing — the bar `open_connector/config.rs:607-619` sets.
32+
- **Toolchain note (this machine):** `cargo` is not on the default PATH. Prefix every invocation with `export PATH="/opt/homebrew/opt/rustup/bin:$PATH"`.
3133

3234
### Decisions made by this plan (approved with the plan; deviations from spec flagged)
3335

@@ -169,9 +171,17 @@ fn empty_user_agent_is_rejected() { /* user_agent: "" → err names user_agent *
169171

170172
#[test]
171173
fn unknown_fields_are_rejected() {
172-
// "feeds: [...]\nttl_secondsss: 5\n" → serde error (deny_unknown_fields),
173-
// both on RssConfig and on FeedSubscription entries.
174-
assert!(serde_yaml::from_str::<RssConfig>("feeds: []\nbogus: 1\n").is_err());
174+
// deny_unknown_fields must fire on both RssConfig and FeedSubscription
175+
// entries. Assert the error NAMES the offending key — a bare is_err()
176+
// would also pass if some unrelated parse error fired instead
177+
// (the bar open_connector/config.rs:607-619 sets).
178+
let err = serde_yaml::from_str::<RssConfig>("feeds: []\nbogus: 1\n").unwrap_err();
179+
assert!(err.to_string().contains("bogus"), "{err}");
180+
let err = serde_yaml::from_str::<RssConfig>(
181+
"feeds:\n - url: https://a.example/f.xml\n nam: x\n",
182+
)
183+
.unwrap_err();
184+
assert!(err.to_string().contains("nam"), "{err}");
175185
}
176186
```
177187

0 commit comments

Comments
 (0)