Skip to content

Add force_exec_validation option to catch runtime errors across Python versions#2719

Merged
koxudaxi merged 1 commit intomainfrom
add-force-exec-validation-option
Dec 21, 2025
Merged

Add force_exec_validation option to catch runtime errors across Python versions#2719
koxudaxi merged 1 commit intomainfrom
add-force-exec-validation-option

Conversation

@koxudaxi
Copy link
Copy Markdown
Owner

@koxudaxi koxudaxi commented Dec 21, 2025

Summary by CodeRabbit

  • Tests
    • Enhanced test suite with new runtime execution validation for generated code against the current Python version.
    • Added comprehensive validation tests for OpenAPI schemas and JSON Schema examples.
    • Extended test infrastructure with new utilities for forced execution validation, improving verification of code generation.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 21, 2025

Warning

Rate limit exceeded

@koxudaxi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 22 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between f604158 and 2407f23.

📒 Files selected for processing (2)
  • tests/main/conftest.py (9 hunks)
  • tests/main/test_exec_validation.py (1 hunks)

Walkthrough

This change introduces runtime execution validation to the test suite by exposing the current Python version through a new constant and pytest fixture, and by extending validation functions with a force_exec_validation parameter that conditionally enables exec-based validation when target and runtime versions align. A comprehensive test module validates the new functionality across OpenAPI and JSON Schema examples.

Changes

Cohort / File(s) Summary
Test Infrastructure Updates
tests/main/conftest.py
Added CURRENT_PYTHON_VERSION constant and current_python_version() fixture to expose the runtime Python version. Introduced get_current_version_args() helper to construct version-aware CLI arguments. Extended _validate_output_files() and _should_skip_exec() with force_exec_validation and force_exec parameters respectively to conditionally enable execution-based validation. Propagated the new parameter through run_main_and_assert() and run_main_url_and_assert() public test helpers.
New Execution Validation Tests
tests/main/test_exec_validation.py
Introduced new test module with four test classes: TestExecValidationOpenAPI (3 tests validating OpenAPI code generation), TestExecValidationJSONSchema (3 tests validating JSON Schema code generation), TestForceExecValidation (1 test verifying force execution when target version differs from runtime), and TestCurrentVersionHelper (3 tests validating helper function correctness). All tests use force execution validation against the current Python version.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Conftest parameter threading: Verify force_exec_validation is correctly propagated through the call chain (run_main_and_assert()_validate_output_files()_should_skip_exec()), and that the logic properly skips validation only when forced and versions align.
  • Version checking logic: Review the logic in _should_skip_exec() that gates execution based on target vs. runtime version comparison and the force_exec flag.
  • Test coverage completeness: Verify that the new test suite provides adequate coverage of execution validation scenarios across different schema types and version configurations.

Poem

🐰 A version-aware warren we've built with care,
where execution dances without despair,
Python's versions now aligned just right,
validating code through the digital night—
test suites hop forward, targets in sight! 🎯

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 title accurately describes the main change: introducing a force_exec_validation option to enable runtime error detection across different Python versions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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.

@koxudaxi koxudaxi force-pushed the add-force-exec-validation-option branch from f604158 to 64d29f4 Compare December 21, 2025 02:14
Copy link
Copy Markdown

@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: 3

🧹 Nitpick comments (2)
tests/main/test_exec_validation.py (2)

131-152: LGTM!

These helper tests correctly validate get_current_version_args behavior and the CURRENT_PYTHON_VERSION format. Note: The pipeline errors for ANN001 on lines 134, 139, 149 appear to be incorrect since these methods only have self as a parameter and don't use output_file.

However, consider adding return type annotations for consistency:

-    def test_get_current_version_args_basic(self):
+    def test_get_current_version_args_basic(self) -> None:

29-30: Consider suppressing PLR6301 for test classes.

The PLR6301 linting errors suggest converting test methods to static methods or functions. However, grouping tests in classes is a common pytest pattern for organization. You can suppress this rule for test files.

Options:

  1. Add # noqa: PLR6301 to each method
  2. Configure ruff/pylint to ignore PLR6301 in test files
  3. Convert classes to standalone functions (less organized but lint-clean)

The class-based organization provides good grouping by schema type, so option 2 is recommended.

Also applies to: 40-41, 51-52, 66-67, 77-78, 88-89, 107-112, 134-134, 139-139, 149-149

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9f7e40d and f604158.

📒 Files selected for processing (2)
  • tests/main/conftest.py (9 hunks)
  • tests/main/test_exec_validation.py (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint
tests/main/test_exec_validation.py

[error] 30-30: PLR6301 Method test_openapi_api_exec_current_version could be a function, class method, or static method


[error] 30-30: ANN001 Missing type annotation for function argument output_file


[error] 41-41: PLR6301 Method test_openapi_with_refs_exec_current_version could be a function, class method, or static method


[error] 41-41: ANN001 Missing type annotation for function argument output_file


[error] 52-52: PLR6301 Method test_openapi_allof_exec_current_version could be a function, class method, or static method


[error] 52-52: ANN001 Missing type annotation for function argument output_file


[error] 67-67: PLR6301 Method test_jsonschema_person_exec_current_version could be a function, class method, or static method


[error] 67-67: ANN001 Missing type annotation for function argument output_file


[error] 78-78: PLR6301 Method test_jsonschema_nested_array_exec_current_version could be a function, class method, or static method


[error] 78-78: ANN001 Missing type annotation for function argument output_file


[error] 89-89: PLR6301 Method test_jsonschema_circular_reference_exec_current_version could be a function, class method, or static method


[error] 89-89: ANN001 Missing type annotation for function argument output_file


[error] 112-112: PLR6301 Method test_force_exec_with_different_target_version could be a function, class method, or static method


[error] 112-112: ANN001 Missing type annotation for function argument output_file


[error] 134-134: PLR6301 Method test_get_current_version_args_basic could be a function, class method, or static method


[error] 134-134: ANN001 Missing type annotation for function argument output_file


[error] 139-139: PLR6301 Method test_get_current_version_args_with_extra could be a function, class method, or static method


[error] 139-139: ANN001 Missing type annotation for function argument output_file


[error] 149-149: PLR6301 Method test_current_python_version_format could be a function, class method, or static method


[error] 149-149: ANN001 Missing type annotation for function argument output_file

⏰ 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). (8)
  • GitHub Check: 3.11 on Windows
  • GitHub Check: 3.10 on Ubuntu
  • GitHub Check: 3.10 on Windows
  • GitHub Check: 3.12 on Windows
  • GitHub Check: py312-black23 on Ubuntu
  • GitHub Check: 3.14 on Windows
  • GitHub Check: Analyze (python)
  • GitHub Check: benchmarks
🔇 Additional comments (8)
tests/main/conftest.py (7)

56-57: LGTM!

Clean implementation of the version constant using sys.version_info. The docstring provides a helpful example.


108-116: LGTM!

The fixture correctly exposes CURRENT_PYTHON_VERSION and includes helpful documentation about its purpose for enabling exec() validation.


118-131: LGTM!

The helper function is well-designed with clear documentation and a practical example. Using *extra_args with unpacking provides a clean API.


419-438: LGTM!

The logic correctly handles the force_exec parameter:

  • Returns early if pydantic version mismatches model type
  • Returns early if no target version specified
  • When force_exec=False, skips if target ≠ runtime (original behavior)
  • When force_exec=True, allows exec to proceed (target ≤ runtime is ensured by _should_skip_compile)

The docstring clearly explains the interaction with target/runtime version checks.


441-459: LGTM!

The updated _validate_output_files function properly threads the force_exec_validation parameter through to _should_skip_exec. The docstring accurately describes when the parameter has no effect (target > runtime).


254-254: LGTM!

The force_exec_validation parameter is properly integrated into run_main_and_assert:

  • Added to function signature with sensible default (False)
  • Well-documented in the docstring explaining its behavior and limitations
  • Correctly passed to _validate_output_files

Also applies to: 291-297, 388-388


531-531: LGTM!

Consistent propagation of force_exec_validation through run_main_url_and_assert, matching the pattern established in run_main_and_assert.

Also applies to: 543-544, 551-551

tests/main/test_exec_validation.py (1)

1-7: LGTM!

Clear module-level docstring explaining the purpose of these tests and referencing the relevant skip logic in conftest.py.

Comment thread tests/main/test_exec_validation.py Outdated
Comment thread tests/main/test_exec_validation.py Outdated
Comment thread tests/main/test_exec_validation.py Outdated
@koxudaxi koxudaxi force-pushed the add-force-exec-validation-option branch from 64d29f4 to 7d56e30 Compare December 21, 2025 02:19
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.35%. Comparing base (f4b074f) to head (2407f23).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2719   +/-   ##
=======================================
  Coverage   99.34%   99.35%           
=======================================
  Files          82       83    +1     
  Lines       11615    11708   +93     
  Branches     1406     1410    +4     
=======================================
+ Hits        11539    11632   +93     
  Misses         45       45           
  Partials       31       31           
Flag Coverage Δ
unittests 99.35% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Dec 21, 2025

CodSpeed Performance Report

Merging #2719 will not alter performance

Comparing add-force-exec-validation-option (2407f23) with main (3aeab90)

Summary

✅ 59 untouched
⏩ 10 skipped1

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@koxudaxi koxudaxi force-pushed the add-force-exec-validation-option branch from 7d56e30 to 2407f23 Compare December 21, 2025 02:32
@koxudaxi koxudaxi merged commit e1fe42e into main Dec 21, 2025
37 checks passed
@koxudaxi koxudaxi deleted the add-force-exec-validation-option branch December 21, 2025 02:54
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