Epilogue nits: CI when clause + test style fixes #13

New issue

Closed

opened 2026-03-14 18:36:20 +00:00 by forgejo_admin · 0 comments

forgejo_admin commented

2026-03-14 18:36:20 +00:00

Contributor

Lineage

plan-pal-e-agency → Epilogue (nits #11-13)

Repo

forgejo_admin/forgejo-mcp

User Story

As a maintainer
I want CI config and test code to follow best practices
So that the test suite is reliable and CI pipelines are clean

Context

QA review of PR #12 found 3 non-blocking nits. These are minor improvements batched from the plan-pal-e-agency epilogue.

File Targets

Files to modify:

.woodpecker.yml — remove redundant when clause on test step (nit #11)
tests/test_new_tools.py (or equivalent) — change return_value to side_effect for pagination test mock (nit #12). If this is just a style preference and both work correctly, skip it.
Test setup that uses limit=100 directly — awareness only, no change needed unless there's a cleaner pattern (nit #13). Skip if not actionable.

Acceptance Criteria

Redundant when clause removed from test step in .woodpecker.yml
Mock style reviewed — fix if side_effect is semantically more correct, skip if equivalent
Tests still pass after changes

Test Expectations

Run: pytest tests/ -v — all tests pass
CI pipeline passes

Constraints

Minimal changes only — these are nits, not refactors
If a nit is not clearly actionable, skip it and note why in the PR

Checklist

PR opened
Tests pass
No unrelated changes

plan-pal-e-agency — epilogue nits #11-13

### Lineage `plan-pal-e-agency` → Epilogue (nits #11-13) ### Repo `forgejo_admin/forgejo-mcp` ### User Story As a maintainer I want CI config and test code to follow best practices So that the test suite is reliable and CI pipelines are clean ### Context QA review of PR #12 found 3 non-blocking nits. These are minor improvements batched from the plan-pal-e-agency epilogue. ### File Targets Files to modify: - `.woodpecker.yml` — remove redundant `when` clause on test step (nit #11) - `tests/test_new_tools.py` (or equivalent) — change `return_value` to `side_effect` for pagination test mock (nit #12). If this is just a style preference and both work correctly, skip it. - Test setup that uses `limit=100` directly — awareness only, no change needed unless there's a cleaner pattern (nit #13). Skip if not actionable. ### Acceptance Criteria - [ ] Redundant `when` clause removed from test step in `.woodpecker.yml` - [ ] Mock style reviewed — fix if `side_effect` is semantically more correct, skip if equivalent - [ ] Tests still pass after changes ### Test Expectations - [ ] Run: `pytest tests/ -v` — all tests pass - [ ] CI pipeline passes ### Constraints - Minimal changes only — these are nits, not refactors - If a nit is not clearly actionable, skip it and note why in the PR ### Checklist - [ ] PR opened - [ ] Tests pass - [ ] No unrelated changes ### Related - `plan-pal-e-agency` — epilogue nits #11-13