Cover input error messages#3537
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new pytest module with parametrized cases covering CLI stderr diagnostics and exit codes for malformed input, missing input paths, and one tolerated malformed case. ChangesError message regression tests
Estimated code review effort: 1 (Trivial) | ~5 minutes Related PRs: None found. Suggested labels: tests Suggested reviewers: koxudaxi 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Merging this PR will improve performance by 21.59%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | test_perf_all_options_enabled |
5.3 s | 4.3 s | +23.85% |
| ⚡ | WallTime | test_perf_deep_nested |
5.1 s | 4.1 s | +23.18% |
| ⚡ | WallTime | test_perf_openapi_large |
2.9 s | 2.4 s | +23.18% |
| ⚡ | WallTime | test_perf_complex_refs |
1.9 s | 1.6 s | +22.45% |
| ⚡ | WallTime | test_perf_large_models_pydantic_v2 |
3.5 s | 2.9 s | +21.94% |
| ⚡ | WallTime | test_perf_duplicate_names |
1,022.1 ms | 841.1 ms | +21.53% |
| ⚡ | WallTime | test_perf_multiple_files_input |
3.4 s | 2.8 s | +21.2% |
| ⚡ | WallTime | test_perf_stripe_style_pydantic_v2 |
1.9 s | 1.6 s | +20.46% |
| ⚡ | WallTime | test_perf_kubernetes_style_pydantic_v2 |
2.5 s | 2.1 s | +20.39% |
| ⚡ | WallTime | test_perf_graphql_style_pydantic_v2 |
770.3 ms | 641.8 ms | +20.01% |
| ⚡ | WallTime | test_perf_aws_style_openapi_pydantic_v2 |
1.8 s | 1.5 s | +19.42% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing cover-input-error-messages (fd2ec04) with main (1af3514)
Footnotes
-
98 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. ↩
|
📚 Docs Preview: https://pr-3537.datamodel-code-generator.pages.dev |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/main/test_error_messages.py (1)
45-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify
_input_file_type_id.A
match/casewith a bind-and-return capture is more machinery than needed for a plain None-check.♻️ Simpler alternative
-def _input_file_type_id(input_file_type: InputFileTypeLiteral | None) -> str: - match input_file_type: - case None: - return "auto" - case input_type: - return input_type +def _input_file_type_id(input_file_type: InputFileTypeLiteral | None) -> str: + return "auto" if input_file_type is None else input_file_type🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/main/test_error_messages.py` around lines 45 - 50, Simplify _input_file_type_id by replacing the match/case in tests/main/test_error_messages.py with a direct None check, since the function only maps None to "auto" and otherwise returns the provided InputFileTypeLiteral. Keep the behavior identical, but make the logic in _input_file_type_id more direct and readable by removing the bind-and-return pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/main/test_error_messages.py`:
- Around line 37-40: The missing-path handling in the input-file validation path
is inconsistent: `jsonschema` still falls through to a traceback instead of the
same clean “File not found” message used for `None`. Update the logic around the
input-type selection and `FileNotFoundError` handling so the early missing-file
check applies to explicit `jsonschema` input too, and keep the expected
diagnostic aligned in `MISSING_INPUT_CASES` and the related error-reporting
flow.
---
Nitpick comments:
In `@tests/main/test_error_messages.py`:
- Around line 45-50: Simplify _input_file_type_id by replacing the match/case in
tests/main/test_error_messages.py with a direct None check, since the function
only maps None to "auto" and otherwise returns the provided
InputFileTypeLiteral. Keep the behavior identical, but make the logic in
_input_file_type_id more direct and readable by removing the bind-and-return
pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1d8655de-a3d9-4b15-88a5-22906ce28744
⛔ Files ignored due to path filters (12)
tests/data/malformed/bad.graphqlis excluded by!tests/data/**/*.graphqland included by nonetests/data/malformed/bad_openapi.yamlis excluded by!tests/data/**/*.yamland included by nonetests/data/malformed/dangling_local_ref.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/malformed/empty_jsonschema.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/malformed/enum_as_dict.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/malformed/missing_external_ref.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/malformed/non_dict_root.yamlis excluded by!tests/data/**/*.yamland included by nonetests/data/malformed/pointer_through_scalar.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/malformed/pointer_through_scalar_openapi.yamlis excluded by!tests/data/**/*.yamland included by nonetests/data/malformed/required_as_string.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/malformed/truncated_jsonschema.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/malformed/wrong_type_properties.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (1)
tests/main/test_error_messages.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3537 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 160 161 +1
Lines 34295 34363 +68
Branches 4003 4011 +8
=========================================
+ Hits 34295 34363 +68
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Breaking Change AnalysisResult: No breaking changes detected Reasoning: PR #3537 is test-only. It adds malformed input fixtures under tests/data/malformed/ and a new regression test file tests/main/test_error_messages.py that pins the current stderr signals and exit codes for malformed/missing inputs. No source code, CLI options, Python API, Jinja2 templates, generated code output, default values, or error-handling logic were changed. The deleted-lines file is empty, so nothing was removed. All changes are additive test coverage, which cannot break existing users. This analysis was performed by Claude Code Action |
|
🎉 Released in 0.67.0 This PR is now available in the latest release. See the release notes for details. |
2 participants
Summary by CodeRabbit