cal/major-domo-database
1
0
Fork 0
Code Issues 6 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

fix: update test limit to respect MAX_LIMIT=500 (#110) #112

Merged
cal merged 1 commits from issue/110-fix-test-batting-sbaplayer-career-totals-returns-4 into main 2026-04-08 12:55:57 +00:00
Conversation 0 Commits 1 Files Changed 1 +1 -1
Claude commented 2026-04-08 08:02:48 +00:00
Collaborator
Copy Link

Closes #110

Summary

test_batting_sbaplayer_career_totals was sending limit=999 to GET /api/v3/plays/batting, but MAX_LIMIT = 500 (set in dependencies.py). FastAPI's Query(..., le=MAX_LIMIT) validation on the limit parameter correctly rejected the out-of-range value with a 422.

Fix: Changed limit=999 to limit=500 in the test. A player's season count will never approach 500, so the test logic (career PA ≥ max single-season PA) is unaffected.

Files Changed

  • tests/integration/test_stratplay_routes.py — line 572: limit: 999 → limit: 500

Other observations

  • Issue #111 proposes raising MAX_LIMIT to 1000 for play endpoints specifically. If that lands, limit=500 in this test will remain valid (500 ≤ 1000).
Closes #110 ## Summary `test_batting_sbaplayer_career_totals` was sending `limit=999` to `GET /api/v3/plays/batting`, but `MAX_LIMIT = 500` (set in `dependencies.py`). FastAPI's `Query(..., le=MAX_LIMIT)` validation on the `limit` parameter correctly rejected the out-of-range value with a 422. **Fix**: Changed `limit=999` to `limit=500` in the test. A player's season count will never approach 500, so the test logic (career PA ≥ max single-season PA) is unaffected. ## Files Changed - `tests/integration/test_stratplay_routes.py` — line 572: `limit: 999` → `limit: 500` ## Other observations - Issue #111 proposes raising `MAX_LIMIT` to 1000 for play endpoints specifically. If that lands, `limit=500` in this test will remain valid (500 ≤ 1000).
Claude added 1 commit 2026-04-08 08:02:49 +00:00 
Closes #110

The test was sending limit=999 which exceeds MAX_LIMIT (500), causing
FastAPI to return 422. Changed to limit=500, which is sufficient to
cover all seasons for any player.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude added the
ai-reviewing
 label 2026-04-08 09:56:29 +00:00
Claude reviewed 2026-04-08 09:57:23 +00:00
Claude left a comment
Author
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • tests/integration/test_stratplay_routes.py (modified)

Findings

Correctness

No issues found. MAX_LIMIT = 500 is confirmed at app/dependencies.py:63. The old limit=999 exceeded that cap, causing FastAPI's Query(..., le=MAX_LIMIT) validation to return 422. Changing to limit=500 aligns with the constraint. The test's assertion logic (career PA ≥ max single-season PA) is unaffected — no SBA player will ever have 500 distinct seasons, so the limit is never the binding constraint.

Security

No issues found.

Style & Conventions

No issues found. Single-line change, no formatting noise.

Suggestions

  • If issue #111 lands and MAX_LIMIT is raised to 1000 for play endpoints, limit=500 will remain valid (500 ≤ 1000). No follow-up needed.

Verdict: APPROVED

Minimal, correct fix. limit=500 satisfies le=MAX_LIMIT while being orders of magnitude above any realistic season count per player. Note: posting as COMMENT because Gitea blocks self-approval.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `tests/integration/test_stratplay_routes.py` (modified) ### Findings #### Correctness No issues found. `MAX_LIMIT = 500` is confirmed at `app/dependencies.py:63`. The old `limit=999` exceeded that cap, causing FastAPI's `Query(..., le=MAX_LIMIT)` validation to return 422. Changing to `limit=500` aligns with the constraint. The test's assertion logic (career PA ≥ max single-season PA) is unaffected — no SBA player will ever have 500 distinct seasons, so the limit is never the binding constraint. #### Security No issues found. #### Style & Conventions No issues found. Single-line change, no formatting noise. #### Suggestions - If issue #111 lands and `MAX_LIMIT` is raised to 1000 for play endpoints, `limit=500` will remain valid (`500 ≤ 1000`). No follow-up needed. ### Verdict: APPROVED Minimal, correct fix. `limit=500` satisfies `le=MAX_LIMIT` while being orders of magnitude above any realistic season count per player. Note: posting as COMMENT because Gitea blocks self-approval. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-04-08 09:57:45 +00:00
cal force-pushed issue/110-fix-test-batting-sbaplayer-career-totals-returns-4 from b980367b8c to 36b962e5d5 2026-04-08 12:55:38 +00:00 Compare
cal merged commit f9e24eb4bc into main 2026-04-08 12:55:57 +00:00
cal deleted branch issue/110-fix-test-batting-sbaplayer-career-totals-returns-4 2026-04-08 12:55:57 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
ai-changes-requested

   
ai-failed

   
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-working
No Label
ai-changes-requested
ai-failed
ai-pr-opened
ai-reviewed
ai-reviewing
ai-working
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
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: cal/major-domo-database#112
No description provided.
Delete Branch "issue/110-fix-test-batting-sbaplayer-career-totals-returns-4"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?