fix: update test assertion to use settings.frontend_url #203

Merged

forgejo_admin merged 1 commit from 202-fix-test-reg-link-copy-uses-base-url-to into main

2026-03-28 16:44:48 +00:00

Conversation 2 Commits 1 Files changed 1 +2 -2

forgejo_admin commented

2026-03-28 12:13:12 +00:00

Owner

Copy link

Summary

Fix regression in test_reg_link_copy_uses_base_url where the test still asserted against settings.base_url after PR #196 changed the implementation to use settings.frontend_url

Changes

tests/test_tryouts.py: Updated assertion on line 419 from settings.base_url to settings.frontend_url
tests/test_tryouts.py: Updated docstring on line 407 to reference settings.frontend_url

Test Plan

Tests pass locally — 72 passed in pytest tests/test_tryouts.py -x -q
ruff format and ruff check clean
No regressions — full tryouts test suite passes

Review Checklist

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

Related Notes

Closes #202
forgejo_admin/basketball-api #196 — PR that introduced the regression

## Summary - Fix regression in `test_reg_link_copy_uses_base_url` where the test still asserted against `settings.base_url` after PR #196 changed the implementation to use `settings.frontend_url` ## Changes - `tests/test_tryouts.py`: Updated assertion on line 419 from `settings.base_url` to `settings.frontend_url` - `tests/test_tryouts.py`: Updated docstring on line 407 to reference `settings.frontend_url` ## Test Plan - [x] Tests pass locally — 72 passed in `pytest tests/test_tryouts.py -x -q` - [x] `ruff format` and `ruff check` clean - [x] No regressions — full tryouts test suite passes ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - Closes #202 - `forgejo_admin/basketball-api #196` — PR that introduced the regression

forgejo_admin added 1 commit

2026-03-28 12:13:12 +00:00

fix: update test assertion to use settings.frontend_url

All checks were successful

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

9d943d3598

The implementation changed from settings.base_url to settings.frontend_url
in PR #196 but the test was not updated. Fixes #202.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-28 12:13:58 +00:00

Author

Owner

Copy link

QA Review

Diff: 1 file, +2/-2. Two-line fix in tests/test_tryouts.py.

Findings

Assertion fix: settings.base_url replaced with settings.frontend_url on line 419. Matches the implementation change in PR #196 (tryouts.py line 796). Correct.
Docstring fix: Updated to reference settings.frontend_url. Accurate.
Test suite: 72/72 passed per PR body. No regressions.
Lint: ruff format + ruff check clean.
SOP compliance: PR body has all required sections, Closes #202 present, commit message is descriptive.
No secrets, no extraneous changes.

Nits

None.

VERDICT: APPROVE

## QA Review **Diff:** 1 file, +2/-2. Two-line fix in `tests/test_tryouts.py`. ### Findings - **Assertion fix:** `settings.base_url` replaced with `settings.frontend_url` on line 419. Matches the implementation change in PR #196 (`tryouts.py` line 796). Correct. - **Docstring fix:** Updated to reference `settings.frontend_url`. Accurate. - **Test suite:** 72/72 passed per PR body. No regressions. - **Lint:** ruff format + ruff check clean. - **SOP compliance:** PR body has all required sections, `Closes #202` present, commit message is descriptive. - **No secrets, no extraneous changes.** ### Nits None. **VERDICT: APPROVE**

forgejo_admin referenced this pull request

2026-03-28 12:19:39 +00:00

Validate session 2026-03-28 merges (2 PRs) #201

forgejo_admin commented

2026-03-28 16:32:46 +00:00

Author

Owner

Copy link

PR #203 Review

DOMAIN REVIEW

Tech stack: Python / FastAPI / pytest

This is a 2-line test fix: updating test_tryouts.py to assert against settings.frontend_url instead of settings.base_url, aligning the test with the implementation change made in PR #196.

Verification:

routes/tryouts.py:796 uses settings.frontend_url for the registration link -- the test assertion now matches.
config.py:29 defines frontend_url as a settings field -- no import issues.
The other settings.base_url reference in test_tryouts.py:612 (photo URLs) is correct as-is, since routes/tryouts.py:509 serves photos from settings.base_url (the API origin). This PR correctly scopes only the registration link assertion.

The diff is minimal and correct. The docstring update on line 407 and the assertion update on line 419 are both accurate.

BLOCKERS

None.

NITS

Test method name stale: test_reg_link_copy_uses_base_url still says "base_url" but the test now verifies frontend_url. Consider renaming to test_reg_link_copy_uses_frontend_url for clarity. Non-blocking since the docstring was updated and the name is still discoverable.

SOP COMPLIANCE

Branch named after issue (202-fix-test-reg-link-copy-uses-base-url-to starts with 202)
PR body has Summary, Changes, Test Plan, Related sections
Related references parent PR #196 and closes #202 -- no plan slug needed (bug fix, not plan work)
No secrets committed
No unnecessary file changes (1 file, 2 additions, 2 deletions)
Commit messages are descriptive

PROCESS OBSERVATIONS

Clean test regression fix. PR #196 changed behavior but missed updating the test assertion -- this is exactly the kind of thing a CI gate with pytest in the pipeline would catch automatically. If the test was not running in CI at the time of PR #196, that is worth investigating as a process gap.

VERDICT: APPROVED

## PR #203 Review ### DOMAIN REVIEW **Tech stack**: Python / FastAPI / pytest This is a 2-line test fix: updating `test_tryouts.py` to assert against `settings.frontend_url` instead of `settings.base_url`, aligning the test with the implementation change made in PR #196. **Verification**: - `routes/tryouts.py:796` uses `settings.frontend_url` for the registration link -- the test assertion now matches. - `config.py:29` defines `frontend_url` as a settings field -- no import issues. - The other `settings.base_url` reference in `test_tryouts.py:612` (photo URLs) is correct as-is, since `routes/tryouts.py:509` serves photos from `settings.base_url` (the API origin). This PR correctly scopes only the registration link assertion. The diff is minimal and correct. The docstring update on line 407 and the assertion update on line 419 are both accurate. ### BLOCKERS None. ### NITS 1. **Test method name stale**: `test_reg_link_copy_uses_base_url` still says "base_url" but the test now verifies `frontend_url`. Consider renaming to `test_reg_link_copy_uses_frontend_url` for clarity. Non-blocking since the docstring was updated and the name is still discoverable. ### SOP COMPLIANCE - [x] Branch named after issue (`202-fix-test-reg-link-copy-uses-base-url-to` starts with `202`) - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] Related references parent PR #196 and closes #202 -- no plan slug needed (bug fix, not plan work) - [x] No secrets committed - [x] No unnecessary file changes (1 file, 2 additions, 2 deletions) - [x] Commit messages are descriptive ### PROCESS OBSERVATIONS Clean test regression fix. PR #196 changed behavior but missed updating the test assertion -- this is exactly the kind of thing a CI gate with `pytest` in the pipeline would catch automatically. If the test was not running in CI at the time of PR #196, that is worth investigating as a process gap. ### VERDICT: APPROVED

forgejo_admin merged commit 6dc341b16f into main

2026-03-28 16:44:48 +00:00

forgejo_admin deleted branch 202-fix-test-reg-link-copy-uses-base-url-to

2026-03-28 16:44:48 +00:00

forgejo_admin referenced this pull request from a commit

2026-03-28 16:44:50 +00:00

fix: update test assertion to use settings.frontend_url (#203)

forgejo_admin referenced this pull request

2026-03-28 19:10:37 +00:00

Validate session 2026-03-28 merges (2 PRs) #201

forgejo_admin referenced this pull request

2026-03-28 19:16:23 +00:00

Validate session 2026-03-28 merges (2 PRs) #201

forgejo_admin referenced this pull request

2026-03-28 19:17:07 +00:00

settings.base_url still used in 7 locations after frontend_url migration #215

Reviewers

No reviewers

Labels

Clear labels

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

No labels

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!203

Reference in a new issue

Repository

forgejo_admin/basketball-api

Title

Body

No description provided.

Delete branch "202-fix-test-reg-link-copy-uses-base-url-to"

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?

Rows
Columns