test: add coverage for missing finished_at key in pipeline data #10

Merged

ldraney merged 1 commit from fix/failure-detection-nits into main

2026-06-14 11:32:31 +00:00

ldraney commented

2026-06-14 01:55:30 +00:00

Owner

Summary

Add test coverage for when a pipeline dict has no finished_at key at all (not just zero -- completely missing). The collector already handles this gracefully via .get("finished_at", 0), but no test verified the behavior.

Changes

tests/test_woodpecker_failure.py: Added test_failure_with_missing_finished_at_ignored -- constructs a pipeline dict without finished_at key, verifies no KeyError is raised and the failure timestamp gauge stays at 0.

Test Plan

All 23 tests pass (python -m pytest tests/ -v)
ruff format and ruff check pass

Nit from post-merge review of PR #9

🤖 Generated with Claude Code

## Summary Add test coverage for when a pipeline dict has no `finished_at` key at all (not just zero -- completely missing). The collector already handles this gracefully via `.get("finished_at", 0)`, but no test verified the behavior. ## Changes - `tests/test_woodpecker_failure.py`: Added `test_failure_with_missing_finished_at_ignored` -- constructs a pipeline dict without `finished_at` key, verifies no KeyError is raised and the failure timestamp gauge stays at 0. ## Test Plan - [x] All 23 tests pass (`python -m pytest tests/ -v`) - [x] ruff format and ruff check pass ## Related - Nit from post-merge review of PR #9 🤖 Generated with [Claude Code](https://claude.com/claude-code)

ldraney added 1 commit

2026-06-14 01:55:30 +00:00

test: add coverage for missing finished_at key in pipeline data a8815618f7

The collector already handles absent finished_at via .get() defaulting
to 0, but no test verified this. Add test_failure_with_missing_finished_at_ignored
to confirm no KeyError is raised and the failure timestamp gauge stays at 0.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney commented

2026-06-14 01:56:36 +00:00

Author

Owner

PR #10 Review

DOMAIN REVIEW

Stack: Python / pytest / prometheus_client / asyncio
Scope: 1 file changed (tests/test_woodpecker_failure.py), +34/-16

New test (test_failure_with_missing_finished_at_ignored):

Correctly constructs a pipeline dict without a finished_at key at all (not zero, not None -- absent)
Verifies no KeyError is raised and the failure timestamp gauge stays at default 0
Follows the exact same mock/assert pattern as the sibling test_failure_with_zero_finished_at_ignored test
Good docstring explaining intent
Logical placement directly before the zero-finished_at test, creating a clear missing-key / zero-value pair

Ruff reformatting (remaining hunks):

Multi-line .labels(repo=..., tier=...) calls collapsed to single lines (within line length)
One long assert line wrapped into parenthesized form
Pure formatting, no behavioral changes

BLOCKERS

None.

NITS

None. The test is well-structured and the ruff reformatting is consistent.

SOP COMPLIANCE

PR body has Summary, Changes, Test Plan, Related sections
No secrets or credentials committed
No unnecessary file changes (formatting changes are ruff compliance, not scope creep)
Commit message is descriptive (test: add coverage for missing finished_at key in pipeline data)
Single file changed, tightly scoped to the nit identified in PR #9 review

PROCESS OBSERVATIONS

Clean nit-fix workflow: post-merge review of PR #9 identified a coverage gap, this PR addresses it with a single focused test
No parent issue exists, which is appropriate for a small post-merge nit fix
Test count growth (23 tests per PR body) shows healthy coverage discipline

VERDICT: APPROVED

## PR #10 Review ### DOMAIN REVIEW **Stack:** Python / pytest / prometheus_client / asyncio **Scope:** 1 file changed (`tests/test_woodpecker_failure.py`), +34/-16 **New test (`test_failure_with_missing_finished_at_ignored`):** - Correctly constructs a pipeline dict without a `finished_at` key at all (not zero, not None -- absent) - Verifies no KeyError is raised and the failure timestamp gauge stays at default 0 - Follows the exact same mock/assert pattern as the sibling `test_failure_with_zero_finished_at_ignored` test - Good docstring explaining intent - Logical placement directly before the zero-finished_at test, creating a clear missing-key / zero-value pair **Ruff reformatting (remaining hunks):** - Multi-line `.labels(repo=..., tier=...)` calls collapsed to single lines (within line length) - One long `assert` line wrapped into parenthesized form - Pure formatting, no behavioral changes ### BLOCKERS None. ### NITS None. The test is well-structured and the ruff reformatting is consistent. ### SOP COMPLIANCE - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] No secrets or credentials committed - [x] No unnecessary file changes (formatting changes are ruff compliance, not scope creep) - [x] Commit message is descriptive (`test: add coverage for missing finished_at key in pipeline data`) - [x] Single file changed, tightly scoped to the nit identified in PR #9 review ### PROCESS OBSERVATIONS - Clean nit-fix workflow: post-merge review of PR #9 identified a coverage gap, this PR addresses it with a single focused test - No parent issue exists, which is appropriate for a small post-merge nit fix - Test count growth (23 tests per PR body) shows healthy coverage discipline ### VERDICT: APPROVED