forgejo_admin/basketball-api
1
0
Fork 1
Code Issues 10 Pull requests 2 Projects Releases Packages Wiki Activity Actions

Fix 5 failing tests: auth_config + graduating_class #76

Merged
forgejo_admin merged 1 commit from 75-fix-5-failing-tests-auth-config-graduati into main 2026-03-13 20:55:25 +00:00
Conversation 2 Commits 1 Files changed 1 +4 -1
forgejo_admin commented 2026-03-13 20:49:31 +00:00
Owner
Copy link

Summary

Fixes 5 failing tests in tests/test_tryouts.py caused by a missing auth_config on the public_client fixture and missing graduating_class in two expected_fields sets.

Changes

  • tests/test_tryouts.py: Added from basketball_api.main import _auth_config and set app.state.auth_config = _auth_config in the public_client fixture, matching the pattern from conftest.py's client fixture. This fixes 3 test_requires_auth tests that were getting HTTP 500 instead of 401.
  • tests/test_tryouts.py: Added "graduating_class" to the expected_fields sets in TestApiCheckIn::test_returns_roster_player_format and TestApiAssignNumber::test_returns_roster_player_format.

Test Plan

  • python -m pytest tests/test_tryouts.py -v -- all 70 tests pass (0 failures)
  • ruff check and ruff format clean
  • No regressions -- only test file changed

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive

Related

## Summary Fixes 5 failing tests in `tests/test_tryouts.py` caused by a missing `auth_config` on the `public_client` fixture and missing `graduating_class` in two `expected_fields` sets. ## Changes - `tests/test_tryouts.py`: Added `from basketball_api.main import _auth_config` and set `app.state.auth_config = _auth_config` in the `public_client` fixture, matching the pattern from `conftest.py`'s `client` fixture. This fixes 3 `test_requires_auth` tests that were getting HTTP 500 instead of 401. - `tests/test_tryouts.py`: Added `"graduating_class"` to the `expected_fields` sets in `TestApiCheckIn::test_returns_roster_player_format` and `TestApiAssignNumber::test_returns_roster_player_format`. ## Test Plan - [x] `python -m pytest tests/test_tryouts.py -v` -- all 70 tests pass (0 failures) - [x] `ruff check` and `ruff format` clean - [x] No regressions -- only test file changed ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related - Closes #75
Fix 5 failing tests: add auth_config to public_client, add graduating_class to expected fields
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
480b55a1a3 
- Set app.state.auth_config in public_client fixture so pal_e_auth returns
  401 instead of 500, fixing test_requires_auth in TestAssignNumbers,
  TestCheckin, and TestUncheckin.
- Add "graduating_class" to expected_fields sets in
  TestApiCheckIn::test_returns_roster_player_format and
  TestApiAssignNumber::test_returns_roster_player_format.

Closes #75

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
forgejo_admin commented 2026-03-13 20:49:53 +00:00
Author
Owner
Copy link

Self-Review: LGTM

Diff: 1 file, +4 / -1. Minimal and scoped to the test file only.

Findings: None. All 3 changes are correct:

  1. _auth_config import added to existing import line.
  2. app.state.auth_config = _auth_config in public_client fixture matches the established pattern from conftest.py::client.
  3. "graduating_class" added to both expected_fields sets that were missing it.

Verification: 70/70 tests pass. Ruff clean.

## Self-Review: LGTM **Diff:** 1 file, +4 / -1. Minimal and scoped to the test file only. **Findings:** None. All 3 changes are correct: 1. `_auth_config` import added to existing import line. 2. `app.state.auth_config = _auth_config` in `public_client` fixture matches the established pattern from `conftest.py::client`. 3. `"graduating_class"` added to both `expected_fields` sets that were missing it. **Verification:** 70/70 tests pass. Ruff clean.
forgejo_admin commented 2026-03-13 20:54:55 +00:00
Author
Owner
Copy link

PR #76 Review

BLOCKERS

None.

NITS

None.

CODE REVIEW

1. _auth_config import and public_client fixture fix (line 12, line 136)

The _auth_config import is moved from a local import inside conftest.py:client() to a top-level import in test_tryouts.py. This is correct -- _auth_config is a module-level singleton in basketball_api.main (line 39), so importing it at the top of the test file is safe and consistent with importing app from the same module.

The public_client fixture now sets app.state.auth_config = _auth_config at line 136, exactly matching the pattern in conftest.py:client() (line 83). This ensures that auth-dependent middleware returns proper 401 responses instead of 500 "Auth not configured" errors. The fix is correct and addresses the 3 test_requires_auth failures.

2. graduating_class added to expected_fields (lines 766, 873)

The graduating_class field exists in the RosterPlayer Pydantic model (src/basketball_api/routes/tryouts.py:444) and is populated in the response builder (tryouts.py:517). The field was already present in the TestPublicRoster::test_all_fields_present expected set (line 673) but was missing from the two sets in TestApiCheckIn and TestApiAssignNumber. Adding it to both is the correct fix.

3. Scope verification

Only tests/test_tryouts.py is modified (1 file, +4/-1). No production code touched. The diff is minimal and precisely targeted at the 5 failing tests.

4. CI verification

Woodpecker pipeline #96 (pull_request event) shows success. All tests pass.

SOP COMPLIANCE

  • Branch named after issue (75-fix-5-failing-tests-auth-config-graduati references issue #75)
  • PR body follows template (Summary, Changes, Test Plan, Related sections present)
  • Related section references closing issue (Closes #75)
  • No secrets committed
  • No unnecessary file changes (single test file, 4 lines added, 1 changed)
  • Commit messages are descriptive
  • Related section references plan slug -- N/A for a standalone bug fix; no plan slug expected

VERDICT: APPROVED

## PR #76 Review ### BLOCKERS None. ### NITS None. ### CODE REVIEW **1. `_auth_config` import and `public_client` fixture fix (line 12, line 136)** The `_auth_config` import is moved from a local import inside `conftest.py:client()` to a top-level import in `test_tryouts.py`. This is correct -- `_auth_config` is a module-level singleton in `basketball_api.main` (line 39), so importing it at the top of the test file is safe and consistent with importing `app` from the same module. The `public_client` fixture now sets `app.state.auth_config = _auth_config` at line 136, exactly matching the pattern in `conftest.py:client()` (line 83). This ensures that auth-dependent middleware returns proper 401 responses instead of 500 "Auth not configured" errors. The fix is correct and addresses the 3 `test_requires_auth` failures. **2. `graduating_class` added to `expected_fields` (lines 766, 873)** The `graduating_class` field exists in the `RosterPlayer` Pydantic model (`src/basketball_api/routes/tryouts.py:444`) and is populated in the response builder (`tryouts.py:517`). The field was already present in the `TestPublicRoster::test_all_fields_present` expected set (line 673) but was missing from the two sets in `TestApiCheckIn` and `TestApiAssignNumber`. Adding it to both is the correct fix. **3. Scope verification** Only `tests/test_tryouts.py` is modified (1 file, +4/-1). No production code touched. The diff is minimal and precisely targeted at the 5 failing tests. **4. CI verification** Woodpecker pipeline #96 (pull_request event) shows **success**. All tests pass. ### SOP COMPLIANCE - [x] Branch named after issue (`75-fix-5-failing-tests-auth-config-graduati` references issue #75) - [x] PR body follows template (Summary, Changes, Test Plan, Related sections present) - [x] Related section references closing issue (`Closes #75`) - [x] No secrets committed - [x] No unnecessary file changes (single test file, 4 lines added, 1 changed) - [x] Commit messages are descriptive - [ ] Related section references plan slug -- N/A for a standalone bug fix; no plan slug expected ### VERDICT: APPROVED
forgejo_admin deleted branch 75-fix-5-failing-tests-auth-config-graduati 2026-03-13 20:55:25 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
domain:backend

   
domain:devops

   
domain:frontend

   
status:approved

QA passed, awaiting merge approval

  
status:in-progress

Dev agent is actively working

  
status:needs-fix

QA found issues, back to dev

  
status:qa

PR submitted, awaiting QA review

  
type:bug

Bug fix

  
type:devops

Infrastructure/CI/config work

  
type:feature

New functionality

No labels
domain:backend
domain:devops
domain:frontend
status:approved
status:in-progress
status:needs-fix
status:qa
type:bug
type:devops
type:feature
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo_admin/basketball-api!76
No description provided.