CI red on main — 2 stale test assertions blocking all PR merges #500

New issue

Closed

opened 2026-04-18 05:34:51 +00:00 by forgejo_admin · 0 comments

forgejo_admin commented

2026-04-18 05:34:51 +00:00

Owner

Type

Bug

Lineage

Standalone — discovered 2026-04-17 while validating PR #499 (monthly-fee Payment Links). Inspection of Woodpecker pipelines #500, #501, #502, #503 shows main has been CI-red across the last 3+ pushes with the same 2 failures.

Repo

forgejo_admin/basketball-api

What Broke

Two test assertions in the basketball-api test suite are stale / reference missing state. Both fail on every CI run on main and on every PR branch. CI has been red on main continuously:

Failure 1 — stale subject assertion:

FAILED tests/test_first_payment_email.py::test_send_first_payment_email
AssertionError: assert 'Great First Practice' in 'Your First Monthly Payment | Westside Kings & Queens'

The test asserts the email subject contains "Great First Practice". The actual subject (from the current services/email.py monthly-fee flow) is "Your First Monthly Payment | Westside Kings & Queens". Someone changed the subject copy and left the test assertion stale.

Failure 2 — missing migration file:

FAILED tests/test_westside_streamlit_ro_role.py::test_migration_file_exists
AssertionError: Migration file missing: alembic/versions/041_add_westside_streamlit_ro_role.py

The test hardcodes a migration filename path. Either the migration was renamed, renumbered (alembic collision), or never added to main.

Repro Steps

git checkout main && git pull
python -m venv /tmp/venv && /tmp/venv/bin/pip install .[dev]
/tmp/venv/bin/python -m pytest tests/test_first_payment_email.py::test_send_first_payment_email tests/test_westside_streamlit_ro_role.py::test_migration_file_exists
Observe both fail.

Alternatively, open Woodpecker pipeline #502 (latest push to main) and inspect the test step.

Expected Behavior

Both tests pass. CI green on main. Every subsequent PR pipeline either passes cleanly or surfaces a real new failure instead of hiding behind inherited red.

Environment

Repo: forgejo_admin/basketball-api, branch main (HEAD as of 2026-04-17)
Python 3.12-slim (Woodpecker test step image)
stripe==14.4.1
postgres:16-alpine (CI sidecar)
Observed on Woodpecker pipelines #500, #501, #502, #503

Blast Radius

Every PR merge currently either ignores CI red or merges while red. That hides real regressions. The 2 failures mask any new test failure a PR introduces — reviewers count "2 failed" and rubber-stamp as "pre-existing," but a 3rd failure slips in unnoticed until post-merge. Per feedback_qa_ci_blockers, CI must actually gate merges; current state violates that contract.

Scope / Fix Direction

test_send_first_payment_email: decide whether the test assertion needs to update to match the current subject, or the subject copy was changed accidentally and should revert. Update the test OR the copy to match intent. git log -p services/email.py for the subject line will show when it changed.
test_migration_file_exists: locate the migration (git log on alembic/versions/ around the 041_* slot) or confirm it was never added. Fix the test to either reference the correct filename or test the role existence more durably (e.g., introspect the alembic chain for the revision that adds the role, not a hardcoded filename).
After both fixes, verify a clean CI run on main.

Acceptance Criteria

Both failing tests root-caused (one-line narrative each: what changed, what broke)
Fix applied to test OR to underlying code, whichever reflects actual intent
CI green on main (post-merge of the fix)
No new regressions introduced by the fix

Constraints

Do NOT suppress/skip the tests as a shortcut (feedback_enterprise_no_workarounds)
Do NOT widen test tolerances to assert True equivalents — that defeats the test
If the subject copy change was intentional, update the test. If it was accidental, revert

Checklist

PR opened
Each test fix has a one-line why-it-broke in the PR description
CI pipeline green on main after merge

Woodpecker pipelines: #500, #501, #502, #503 (all red with these 2 failures)
Discovered during PR #499 QA review
Memory: feedback_qa_ci_blockers.md, feedback_enterprise_no_workarounds.md

### Type Bug ### Lineage Standalone — discovered 2026-04-17 while validating PR #499 (monthly-fee Payment Links). Inspection of Woodpecker pipelines #500, #501, #502, #503 shows main has been CI-red across the last 3+ pushes with the same 2 failures. ### Repo `forgejo_admin/basketball-api` ### What Broke Two test assertions in the basketball-api test suite are stale / reference missing state. Both fail on every CI run on main and on every PR branch. CI has been red on main continuously: **Failure 1 — stale subject assertion:** ``` FAILED tests/test_first_payment_email.py::test_send_first_payment_email AssertionError: assert 'Great First Practice' in 'Your First Monthly Payment | Westside Kings & Queens' ``` The test asserts the email subject contains `"Great First Practice"`. The actual subject (from the current `services/email.py` monthly-fee flow) is `"Your First Monthly Payment | Westside Kings & Queens"`. Someone changed the subject copy and left the test assertion stale. **Failure 2 — missing migration file:** ``` FAILED tests/test_westside_streamlit_ro_role.py::test_migration_file_exists AssertionError: Migration file missing: alembic/versions/041_add_westside_streamlit_ro_role.py ``` The test hardcodes a migration filename path. Either the migration was renamed, renumbered (alembic collision), or never added to main. ### Repro Steps 1. `git checkout main && git pull` 2. `python -m venv /tmp/venv && /tmp/venv/bin/pip install .[dev]` 3. `/tmp/venv/bin/python -m pytest tests/test_first_payment_email.py::test_send_first_payment_email tests/test_westside_streamlit_ro_role.py::test_migration_file_exists` 4. Observe both fail. Alternatively, open Woodpecker pipeline #502 (latest push to main) and inspect the `test` step. ### Expected Behavior Both tests pass. CI green on main. Every subsequent PR pipeline either passes cleanly or surfaces a real new failure instead of hiding behind inherited red. ### Environment - Repo: `forgejo_admin/basketball-api`, branch `main` (HEAD as of 2026-04-17) - Python 3.12-slim (Woodpecker test step image) - stripe==14.4.1 - postgres:16-alpine (CI sidecar) - Observed on Woodpecker pipelines #500, #501, #502, #503 ### Blast Radius Every PR merge currently either ignores CI red or merges while red. That hides real regressions. The 2 failures mask any new test failure a PR introduces — reviewers count "2 failed" and rubber-stamp as "pre-existing," but a 3rd failure slips in unnoticed until post-merge. Per `feedback_qa_ci_blockers`, CI must actually gate merges; current state violates that contract. ### Scope / Fix Direction 1. **`test_send_first_payment_email`:** decide whether the test assertion needs to update to match the current subject, or the subject copy was changed accidentally and should revert. Update the test OR the copy to match intent. `git log -p services/email.py` for the subject line will show when it changed. 2. **`test_migration_file_exists`:** locate the migration (git log on `alembic/versions/` around the `041_*` slot) or confirm it was never added. Fix the test to either reference the correct filename or test the role existence more durably (e.g., introspect the alembic chain for the revision that adds the role, not a hardcoded filename). 3. After both fixes, verify a clean CI run on main. ### Acceptance Criteria - [ ] Both failing tests root-caused (one-line narrative each: what changed, what broke) - [ ] Fix applied to test OR to underlying code, whichever reflects actual intent - [ ] CI green on main (post-merge of the fix) - [ ] No new regressions introduced by the fix ### Constraints - Do NOT suppress/skip the tests as a shortcut (`feedback_enterprise_no_workarounds`) - Do NOT widen test tolerances to `assert True` equivalents — that defeats the test - If the subject copy change was intentional, update the test. If it was accidental, revert ### Checklist - [ ] PR opened - [ ] Each test fix has a one-line why-it-broke in the PR description - [ ] CI pipeline green on main after merge ### Related - Woodpecker pipelines: #500, #501, #502, #503 (all red with these 2 failures) - Discovered during PR #499 QA review - Memory: `feedback_qa_ci_blockers.md`, `feedback_enterprise_no_workarounds.md`