QA nits from #490: silent test skip + import formatting #492

New issue

Open

opened 2026-04-17 20:37:17 +00:00 by forgejo_admin · 0 comments

forgejo_admin commented

2026-04-17 20:37:17 +00:00

Owner

Type

Bug

Lineage

QA nits from PR #490 review (2026-04-17). Per feedback_nits_to_epilogue.md, nits must be tracked — filed here instead of dismissed.

Repo

forgejo_admin/basketball-api

User Story

As an engineer, I trust every regression test is actually asserting behavior (not silently skipping), and imports follow a single consistent convention across the module.

What Broke

Two minor quality issues in the #490 diff:

Silent pytest.skip hole — tests/test_checkout_session_ttl.py::test_register_promo_discount_path_sets_expires_at uses pytest.skip(...) on a branch that, if refactored, could silently no-op forever without flagging. A future change could make this test a permanent pass-by-default.
Inconsistent import style — routes/checkout.py, routes/jersey.py, services/tournament_checkout.py have split from basketball_api.config import ... imports that could be collapsed to match the combined-form convention already used in admin.py and register.py.

Repro Steps

Open tests/test_checkout_session_ttl.py, locate test_register_promo_discount_path_sets_expires_at, observe pytest.skip(...) without a branch-taken assertion
Open the 3 files above, observe multi-line from basketball_api.config import X, Y patterns where the single-line form fits cleaner
Compare against admin.py / register.py combined imports

Expected Behavior

The skip-path test uses pytest.fail(...) or a stricter assertion guaranteeing the tested branch was exercised
Imports unified to the combined single-line form across all 5 files

Environment

Commit: post-squash of PR #490 (landed 2026-04-17)
Files: tests/test_checkout_session_ttl.py, routes/checkout.py, routes/jersey.py, services/tournament_checkout.py

Acceptance Criteria

Test no longer silently skips — either asserts the branch was taken or hard-fails if the path isn't reachable
Imports consistent across all sanctioned session-create files

project-pal-e-platform
forgejo_admin/basketball-api #488, #490 — parent fix PR

### Type Bug ### Lineage QA nits from PR #490 review (2026-04-17). Per `feedback_nits_to_epilogue.md`, nits must be tracked — filed here instead of dismissed. ### Repo `forgejo_admin/basketball-api` ### User Story As an engineer, I trust every regression test is actually asserting behavior (not silently skipping), and imports follow a single consistent convention across the module. ### What Broke Two minor quality issues in the #490 diff: 1. **Silent pytest.skip hole** — `tests/test_checkout_session_ttl.py::test_register_promo_discount_path_sets_expires_at` uses `pytest.skip(...)` on a branch that, if refactored, could silently no-op forever without flagging. A future change could make this test a permanent pass-by-default. 2. **Inconsistent import style** — `routes/checkout.py`, `routes/jersey.py`, `services/tournament_checkout.py` have split `from basketball_api.config import ...` imports that could be collapsed to match the combined-form convention already used in `admin.py` and `register.py`. ### Repro Steps 1. Open `tests/test_checkout_session_ttl.py`, locate `test_register_promo_discount_path_sets_expires_at`, observe `pytest.skip(...)` without a branch-taken assertion 2. Open the 3 files above, observe multi-line `from basketball_api.config import X, Y` patterns where the single-line form fits cleaner 3. Compare against `admin.py` / `register.py` combined imports ### Expected Behavior - The skip-path test uses `pytest.fail(...)` or a stricter assertion guaranteeing the tested branch was exercised - Imports unified to the combined single-line form across all 5 files ### Environment - Commit: post-squash of PR #490 (landed 2026-04-17) - Files: `tests/test_checkout_session_ttl.py`, `routes/checkout.py`, `routes/jersey.py`, `services/tournament_checkout.py` ### Acceptance Criteria - [ ] Test no longer silently skips — either asserts the branch was taken or hard-fails if the path isn't reachable - [ ] Imports consistent across all sanctioned session-create files ### Related - `project-pal-e-platform` - `forgejo_admin/basketball-api #488, #490` — parent fix PR