Skip to content

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Oct 26, 2025

Description

LCORE-885: type hints in integration tests

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Related Tickets & Documents

  • Related Issue #LCORE-885

Summary by CodeRabbit

  • Tests
    • Added explicit type annotations across integration tests and fixtures (fixtures, mocks, request/auth helpers).
    • Annotated async and sync test functions with precise parameter and return types.
    • Clarified test signatures for integration and OpenAPI validation suites to improve readability and type safety.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 26, 2025

Walkthrough

Added or refined type annotations across integration tests: fixtures in tests/integration/conftest.py now have explicit Generator (and Engine/Session) types and supporting imports; test files gained parameter and return type annotations (including Request, MockerFixture, AsyncMockType, and None). No behavioral changes.

Changes

Cohort / File(s) Summary
Fixture type annotations
tests/integration/conftest.py
Added typing imports (Generator, Engine, Session, sessionmaker) and explicit return/parameter type annotations for fixtures: reset_configuration_state() -> Generator, test_config_fixture() -> Generator, test_db_engine_fixture() -> Generator, and test_db_session_fixture(test_db_engine: Engine) -> Generator[Session, None, None].
Info endpoint integration tests
tests/integration/endpoints/test_info_integration.py
Added typing imports (Generator, Any, MockerFixture, AsyncMockType, AppConfig), and annotated fixtures and tests: mock_llama_stack_client_fixture(mocker: MockerFixture) -> Generator, test_request_fixture() -> Request, test_auth_fixture(test_request: Request) -> tuple[str, str, bool, str], and three async test functions with typed params and -> None returns.
OpenAPI tests
tests/integration/test_openapi_json.py
Added explicit -> None return annotations to three tests: test_openapi_top_level_info, test_servers_section_present, and test_paths_and_responses_exist.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Changes are limited to adding/improving type annotations and imports.
  • Quick checks: ensure imports are used and typing-only imports are guarded (if needed), and that test fixtures still resolve correctly.

Files to spot-check:

  • tests/integration/conftest.py (imports and fixture signatures)
  • tests/integration/endpoints/test_info_integration.py (mock typing and async test signatures)

Possibly related PRs

Poem

🐰 I nibble on types, neat and light,
I hop through fixtures late at night,
Generators hum, mocks take their place,
Tests now wear a clearer face,
Hooray — precise types make my code take flight! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "LCORE-885: type hints in integration tests" accurately and directly describes the main changes in the pull request. The changeset focuses on adding explicit type annotations across three integration test files: fixtures in conftest.py are enhanced with Generator return types and typed parameters, test functions in test_info_integration.py gain comprehensive type hints for both parameters and return values, and test functions in test_openapi_json.py receive explicit None return type annotations. The title is concise, specific, and avoids vague terminology or noise, allowing a teammate reviewing the commit history to immediately understand that this PR is about enhancing type safety in integration tests.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7814bb1 and 62ea957.

📒 Files selected for processing (3)
  • tests/integration/conftest.py (4 hunks)
  • tests/integration/endpoints/test_info_integration.py (6 hunks)
  • tests/integration/test_openapi_json.py (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • tests/integration/endpoints/test_info_integration.py
  • tests/integration/conftest.py
  • tests/integration/test_openapi_json.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build-pr
  • GitHub Check: e2e_tests (azure)
  • GitHub Check: e2e_tests (ci)

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.

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.

Actionable comments posted: 4

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0265842 and 7814bb1.

📒 Files selected for processing (3)
  • tests/integration/conftest.py (5 hunks)
  • tests/integration/endpoints/test_info_integration.py (7 hunks)
  • tests/integration/test_openapi_json.py (3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: All modules start with descriptive module-level docstrings explaining purpose
Use logger = logging.getLogger(name) for module logging after import logging
Define type aliases at module level for clarity
All functions require docstrings with brief descriptions
Provide complete type annotations for all function parameters and return types
Use typing_extensions.Self in model validators where appropriate
Use modern union syntax (str | int) and Optional[T] or T | None consistently
Function names use snake_case with descriptive, action-oriented prefixes (get_, validate_, check_)
Avoid in-place parameter modification; return new data structures instead of mutating arguments
Use appropriate logging levels: debug, info, warning, error with clear messages
All classes require descriptive docstrings explaining purpose
Class names use PascalCase with conventional suffixes (Configuration, Error/Exception, Resolver, Interface)
Abstract base classes should use abc.ABC and @AbstractMethod for interfaces
Provide complete type annotations for all class attributes
Follow Google Python docstring style for modules, classes, and functions, including Args, Returns, Raises, Attributes sections as needed

Files:

  • tests/integration/test_openapi_json.py
  • tests/integration/endpoints/test_info_integration.py
  • tests/integration/conftest.py
tests/{unit,integration}/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/{unit,integration}/**/*.py: Use pytest for all unit and integration tests
Do not use unittest in tests; pytest is the standard

Files:

  • tests/integration/test_openapi_json.py
  • tests/integration/endpoints/test_info_integration.py
  • tests/integration/conftest.py
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Use pytest-mock to create AsyncMock objects for async interactions in tests
Use the shared auth mock constant: MOCK_AUTH = ("mock_user_id", "mock_username", False, "mock_token") in tests

Files:

  • tests/integration/test_openapi_json.py
  • tests/integration/endpoints/test_info_integration.py
  • tests/integration/conftest.py
tests/**/conftest.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use conftest.py for shared pytest fixtures

Files:

  • tests/integration/conftest.py
🔇 Additional comments (2)
tests/integration/test_openapi_json.py (1)

35-35: LGTM! Type annotations correctly added.

The -> None return type annotations for test functions are correct and complete. The parameter types are already well-defined.

Also applies to: 51-51, 102-104

tests/integration/endpoints/test_info_integration.py (1)

37-37: LGTM! Fixture return types are correct.

The return type annotations for test_request_fixture and test_auth_fixture accurately reflect the values being returned/yielded.

Also applies to: 49-49

@tisnik tisnik force-pushed the lcore-885-type-hints-in-integration-tests branch from 7814bb1 to 313092d Compare October 26, 2025 16:03
@tisnik tisnik force-pushed the lcore-885-type-hints-in-integration-tests branch from 313092d to 62ea957 Compare October 26, 2025 16:04
@tisnik
Copy link
Contributor Author

tisnik commented Oct 26, 2025

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 26, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@tisnik tisnik merged commit 9906579 into lightspeed-core:main Oct 26, 2025
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant