Skip to content

Fix type annotations#8493

Merged
ogenstad merged 1 commit intorelease-1.8from
pog-api-component-test-annotations
Feb 27, 2026
Merged

Fix type annotations#8493
ogenstad merged 1 commit intorelease-1.8from
pog-api-component-test-annotations

Conversation

@ogenstad
Copy link
Contributor

@ogenstad ogenstad commented Feb 27, 2026

Why

Improve typing within API component tests

What changed

  • Add typehints
  • Remove violations from pyproject.toml

Summary by CodeRabbit

  • Tests

    • Enhanced type annotations across test fixtures and test functions for improved code clarity and type safety.
  • Chores

    • Updated linting configuration to remove granular ignore rules for test files, enabling stricter type-checking standards.

Note: This release contains internal quality improvements with no user-facing changes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 27, 2026

Walkthrough

This pull request adds comprehensive type annotations to test fixtures and test functions across the backend/tests/component/api directory. Changes include updating fixture signatures in conftest.py to explicitly type parameters such as nats, redis, helper, and various schema and data fixtures with specific types like Branch, SchemaBranch, Node, and TestHelper. Similarly, numerous test function signatures are updated to include explicit type hints for parameters previously left untyped. Additional imports for these types are added to relevant modules. The pyproject.toml file is modified to remove granular ANN001 ignore entries for the affected test files, enabling stricter type-checking on these modules.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix type annotations' directly corresponds to the main change in the PR: adding type hints and removing annotation-related violations.
Description check ✅ Passed The description addresses the 'Why' section with the goal to improve typing, and 'What changed' section lists the key changes. However, it lacks detail in implementation notes, testing instructions, and impact assessment that would align with the template.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the group/backend Issue related to the backend (API Server, Git Agent) label Feb 27, 2026
@codspeed-hq
Copy link

codspeed-hq bot commented Feb 27, 2026

Merging this PR will not alter performance

✅ 12 untouched benchmarks


Comparing pog-api-component-test-annotations (f8064ec) with release-1.8 (42c651f)

Open in CodSpeed

@ogenstad ogenstad marked this pull request as ready for review February 27, 2026 08:30
@ogenstad ogenstad requested a review from a team as a code owner February 27, 2026 08:30
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
backend/tests/component/api/test_20_graphql.py (1)

223-223: ⚠️ Potential issue | 🟠 Major

Use an explicit status expectation at Line 223.

assert response.status_code only checks truthiness; it won’t fail for most non-2xx responses.

Proposed fix
-    assert response.status_code
+    assert response.status_code == 200
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/tests/component/api/test_20_graphql.py` at line 223, Replace the
truthiness check on response.status_code with an explicit expectation: update
the assertion in the test (the variable response in the test function where
response.status_code is asserted) to compare against the exact expected HTTP
status (e.g., assert response.status_code == 200 or assert response.status_code
in (200, 201) depending on the endpoint contract) or use response.ok if you
intentionally want any 2xx to pass; change only the assertion to make the intent
explicit.
backend/tests/component/api/test_00_auth.py (1)

273-273: ⚠️ Potential issue | 🟠 Major

Replace tautological assertion at Line 273.

This assertion always passes and does not verify the error content, so regressions can slip through.

Proposed fix
-    assert api_response.json()["errors"][0]["message"] == api_response.json()["errors"][0]["message"]
+    error = api_response.json()["errors"][0]
+    assert "message" in error
+    assert isinstance(error["message"], str) and error["message"]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/tests/component/api/test_00_auth.py` at line 273, The current
assertion compares api_response.json()["errors"][0]["message"] to itself which
always passes; replace it with a real check: assert that
api_response.status_code equals the expected HTTP status for this failure and
assert that api_response.json()["errors"][0]["message"] equals (or contains) the
specific expected error message for this test case (e.g., the expected
authentication error string), using the api_response variable and the
"errors"[0]["message"] path to locate the value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@backend/tests/component/api/test_00_auth.py`:
- Line 273: The current assertion compares
api_response.json()["errors"][0]["message"] to itself which always passes;
replace it with a real check: assert that api_response.status_code equals the
expected HTTP status for this failure and assert that
api_response.json()["errors"][0]["message"] equals (or contains) the specific
expected error message for this test case (e.g., the expected authentication
error string), using the api_response variable and the "errors"[0]["message"]
path to locate the value.

In `@backend/tests/component/api/test_20_graphql.py`:
- Line 223: Replace the truthiness check on response.status_code with an
explicit expectation: update the assertion in the test (the variable response in
the test function where response.status_code is asserted) to compare against the
exact expected HTTP status (e.g., assert response.status_code == 200 or assert
response.status_code in (200, 201) depending on the endpoint contract) or use
response.ok if you intentionally want any 2xx to pass; change only the assertion
to make the intent explicit.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 42c651f and f8064ec.

📒 Files selected for processing (12)
  • backend/tests/component/api/conftest.py
  • backend/tests/component/api/test_00_auth.py
  • backend/tests/component/api/test_01_auth_cookies.py
  • backend/tests/component/api/test_03_menu.py
  • backend/tests/component/api/test_10_query.py
  • backend/tests/component/api/test_11_artifact.py
  • backend/tests/component/api/test_12_file.py
  • backend/tests/component/api/test_15_diff.py
  • backend/tests/component/api/test_20_graphql.py
  • backend/tests/component/api/test_50_internals.py
  • backend/tests/component/api/test_60_storage.py
  • pyproject.toml
💤 Files with no reviewable changes (1)
  • pyproject.toml

@ogenstad ogenstad merged commit a744b0f into release-1.8 Feb 27, 2026
43 checks passed
@ogenstad ogenstad deleted the pog-api-component-test-annotations branch February 27, 2026 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

group/backend Issue related to the backend (API Server, Git Agent)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants