test: expand Forgejo review fixtures

nimmo commented

2026-07-20 21:21:17 +01:00

Owner

Implements #8.\n\nAdds realistic mocked Forgejo PR, files, comments, and API-error fixtures, plus a fixture-driven end-to-end review test.\n\nCloses #8.

nimmo added 1 commit

2026-07-20 21:21:17 +01:00

test: add Forgejo review fixtures

AI pull-request review / review (pull_request) Successful in 2m52s

Details

4b867fe649

nimmo commented

2026-07-20 21:24:11 +01:00

Author

Owner

Automated code review

Reviewed commit: e120ada67201e1968e02fc478992fec53c946b40

Verdict: Ready to merge

No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory.

Overall assessment

Refactors reviewer tests to consume JSON fixtures and introduces a mock API handler. Adds a targeted assertion verifying that API secrets are not leaked in HTTP error messages.

Blocking findings

None.

Important findings

None.

Suggestions

Fragile URL matching in test mock (tests/test_reviewer.py:365; high confidence)
if url.endswith("/pulls/42"): ... if url.endswith("/issues/42/comments"):
Impact: Mock may fail to match expected endpoints if API routing changes, base URLs are updated, or query parameters are appended in production.
Suggested fix: Use urllib.parse.urlparse to extract and compare exact path components, or define explicit route constants for maintainability.

Tests and validation

unit-tests: success

Questions

Does the fixture_api function need to handle pagination tokens or additional query parameters that might be appended to Forgejo API URLs in production?
Are there plans to add fixtures for edge cases such as binary file diffs, large PR batches exceeding max_diff_bytes, or rate-limit responses?

Review limitations

Analysis is limited to the provided diff and fixture files; internal implementation of reviewer.py functions like api_request and ollama_review was not reviewed.
Test execution results were not validated locally.

Diff coverage

tests/fixtures/forgejo_api_error.json: reviewed — included in a context-limited batch
tests/fixtures/pull_request.json: reviewed — included in a context-limited batch
tests/fixtures/pull_request_comments.json: reviewed — included in a context-limited batch
tests/fixtures/pull_request_files.json: reviewed — included in a context-limited batch
tests/test_reviewer.py: reviewed — included in a context-limited batch

Review metadata

Reviewer: forgejo-ai-review v1
Model: qwen3.6:35b-a3b-q8_0
Guidance loaded: .forgejo/ai-review.yml, README.md
Commits considered: 4
Previous automated review: updated
Findings: 1

## Automated code review Reviewed commit: `e120ada67201e1968e02fc478992fec53c946b40` ## Verdict: Ready to merge No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory. ### Overall assessment Refactors reviewer tests to consume JSON fixtures and introduces a mock API handler. Adds a targeted assertion verifying that API secrets are not leaked in HTTP error messages. ### Blocking findings None. ### Important findings None. ### Suggestions - **Fragile URL matching in test mock** (tests/test_reviewer.py:365; high confidence) if url.endswith("/pulls/42"): ... if url.endswith("/issues/42/comments"): Impact: Mock may fail to match expected endpoints if API routing changes, base URLs are updated, or query parameters are appended in production. Suggested fix: Use `urllib.parse.urlparse` to extract and compare exact path components, or define explicit route constants for maintainability. ### Tests and validation - unit-tests: **success** ### Questions - Does the `fixture_api` function need to handle pagination tokens or additional query parameters that might be appended to Forgejo API URLs in production? - Are there plans to add fixtures for edge cases such as binary file diffs, large PR batches exceeding `max_diff_bytes`, or rate-limit responses? ### Review limitations - Analysis is limited to the provided diff and fixture files; internal implementation of `reviewer.py` functions like `api_request` and `ollama_review` was not reviewed. - Test execution results were not validated locally. ### Diff coverage - `tests/fixtures/forgejo_api_error.json`: **reviewed** — included in a context-limited batch - `tests/fixtures/pull_request.json`: **reviewed** — included in a context-limited batch - `tests/fixtures/pull_request_comments.json`: **reviewed** — included in a context-limited batch - `tests/fixtures/pull_request_files.json`: **reviewed** — included in a context-limited batch - `tests/test_reviewer.py`: **reviewed** — included in a context-limited batch <details><summary>Review metadata</summary> - Reviewer: forgejo-ai-review v1 - Model: qwen3.6:35b-a3b-q8_0 - Guidance loaded: .forgejo/ai-review.yml, README.md - Commits considered: 4 - Previous automated review: updated - Findings: 1 </details>

nimmo added 1 commit

2026-07-20 21:25:20 +01:00

test: exercise all fixture files

AI pull-request review / review (pull_request) Successful in 1m54s

Details

5e3dc2683d

nimmo added 1 commit

2026-07-20 21:28:03 +01:00

test: name mocked Forgejo API responses

AI pull-request review / review (pull_request) Successful in 2m20s

Details

8a73ff6a0d