Skip to content

LCORE-1375: use Optional data type in tests#1238

Merged
tisnik merged 2 commits into
lightspeed-core:mainfrom
tisnik:lcore-1375-fixed-tests
Mar 1, 2026
Merged

LCORE-1375: use Optional data type in tests#1238
tisnik merged 2 commits into
lightspeed-core:mainfrom
tisnik:lcore-1375-fixed-tests

Conversation

@tisnik
Copy link
Copy Markdown
Contributor

@tisnik tisnik commented Mar 1, 2026

Description

LCORE-1375: use Optional data type in 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
  • Benchmarks improvement

Tools used to create PR

  • Assisted-by: N/A
  • Generated by: N/A

Related Tickets & Documents

  • Related Issue #LCORE-1375

Summary by CodeRabbit

  • Tests
    • Updated type annotations in test files for improved consistency and clarity. No changes to test functionality or user-facing features.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a6024a and a3182bc.

📒 Files selected for processing (5)
  • tests/e2e/features/steps/token_counters.py
  • tests/e2e/utils/prow_utils.py
  • tests/unit/app/endpoints/test_rlsapi_v1.py
  • tests/unit/models/config/test_splunk_configuration.py
  • tests/unit/observability/test_splunk.py

Walkthrough

This PR refactors type annotations across multiple test files, replacing Python 3.10+ union syntax (X | None) with the Optional[X] style from the typing module. No functional or behavioral changes are introduced; all modifications are purely type annotation updates with corresponding import additions.

Changes

Cohort / File(s) Summary
E2E Test Type Annotations
tests/e2e/features/steps/token_counters.py, tests/e2e/utils/prow_utils.py
Converted union-style type hints to Optional[...] syntax. Added Optional import and updated return type and parameter annotations in _get_end_event_data and run_e2e_ops.
Unit Test Type Annotations
tests/unit/app/endpoints/test_rlsapi_v1.py, tests/unit/models/config/test_splunk_configuration.py, tests/unit/observability/test_splunk.py
Updated type annotations across multiple test functions and helper methods from union syntax to Optional[...]. Added Optional import where needed and updated parameter types for custom_prompt, url, token_path, and index parameters.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3
✅ 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 accurately describes the main change: refactoring test files to use Optional type annotations instead of union syntax (str | None → Optional[str]), which is consistently applied across all modified files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@tisnik tisnik merged commit 8184f7a into lightspeed-core:main Mar 1, 2026
22 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