Refactor generate tests to use assertion helper#3047
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded a test helper run_generate_file_and_assert to tests/main/conftest.py (duplicate definition present), exported Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 not alter performance
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/main/conftest.py (1)
273-283: Extract expected-file inference into one private helper.This block now duplicates the same caller-name inference already in
run_main_and_assert()at Lines 438-449. Pulling it into a shared helper will keep thetest_main_/test_convention from drifting between the CLI andgenerate()test paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/main/conftest.py` around lines 273 - 283, Extract the caller-name inference logic into a single private helper (e.g. _infer_expected_file_from_caller) and replace the duplicated blocks in tests/main/conftest.py: the current inline block and the similar code in run_main_and_assert() (lines around 438-449). The helper should use inspect.currentframe() to get the caller frame, assert frame and frame.f_back are not None, read func_name = frame.f_back.f_code.co_name, del frame, strip the prefixes ("test_main_", "test_") exactly as the current loop does, and return f"{func_name}.py" so both call sites simply call _infer_expected_file_from_caller() when expected_file is None to preserve identical behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/main/conftest.py`:
- Around line 273-283: Extract the caller-name inference logic into a single
private helper (e.g. _infer_expected_file_from_caller) and replace the
duplicated blocks in tests/main/conftest.py: the current inline block and the
similar code in run_main_and_assert() (lines around 438-449). The helper should
use inspect.currentframe() to get the caller frame, assert frame and
frame.f_back are not None, read func_name = frame.f_back.f_code.co_name, del
frame, strip the prefixes ("test_main_", "test_") exactly as the current loop
does, and return f"{func_name}.py" so both call sites simply call
_infer_expected_file_from_caller() when expected_file is None to preserve
identical behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 89747c0e-07fb-4639-a82e-397505753a56
📒 Files selected for processing (2)
tests/main/conftest.pytests/main/jsonschema/test_main_jsonschema.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/main/jsonschema/test_main_jsonschema.py`:
- Around line 1654-1662: The test test_main_generate_relative_input_path fails
when pytest cwd is not an ancestor of JSON_SCHEMA_DATA_PATH because it uses
.relative_to(Path.cwd()); replace that with a cwd-independent relative path
computed via os.path.relpath, e.g. build the input_path with
Path(os.path.relpath(JSON_SCHEMA_DATA_PATH / "person.json")), and pass that into
run_generate_file_and_assert (references:
test_main_generate_relative_input_path, JSON_SCHEMA_DATA_PATH,
run_generate_file_and_assert).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 83927274-25fa-46f8-81a3-4c97a8845f8b
📒 Files selected for processing (1)
tests/main/jsonschema/test_main_jsonschema.py
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/main/conftest.py (1)
273-281: Extract expected-file inference into a shared helper.
run_generate_file_and_assert()now duplicates the caller-name inspection already used inrun_main_and_assert(), and the prefix stripping has already drifted slightly between the two implementations. Pull this into one private helper so both code paths keep the same default expected-file convention.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/main/conftest.py` around lines 273 - 281, Extract the caller-name inspection and prefix-stripping logic that builds expected_file into a single private helper (e.g., _infer_expected_file_from_caller) and have both run_generate_file_and_assert and run_main_and_assert call it; move the code that uses inspect.currentframe(), checks f_back, reads f_code.co_name, deletes the frame, and removes prefixes ("test_main_", "test_") into that helper so both functions share identical behavior and avoid duplication or drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/main/conftest.py`:
- Around line 266-270: The test currently forwards input_file_type
unconditionally into generate(), which overrides GenerateConfig.input_file_type
default (InputFileType.Auto) with None and breaks enum checks (e.g.,
InputFileType.Auto / InputFileType.GraphQL); change the call site in
tests/main/conftest.py so you build generate kwargs conditionally and only
include input_file_type when input_file_type is not None (keep passing input_
and output_path as before) so generate() can use its default when the test
omitted input_file_type.
In `@tests/main/jsonschema/test_main_jsonschema.py`:
- Around line 1666-1677: The test
test_main_generate_external_absolute_input_path must ensure the test file is
truly outside the repository root: instead of using the pytest tmp_path, create
an absolute temporary directory under the system temp (e.g. via
tempfile.mkdtemp() or Path(tempfile.gettempdir()) / unique name), write the
schema file there (using JSON_SCHEMA_DATA_PATH to copy contents), set input_path
to that external path, and add an explicit check that input_path.resolve() is
not inside Path.cwd() (e.g. assert not
input_path.resolve().is_relative_to(Path.cwd()) or compare parents) before
calling run_generate_file_and_assert; update references to input_path, tmp_path,
JSON_SCHEMA_DATA_PATH, and run_generate_file_and_assert accordingly.
---
Nitpick comments:
In `@tests/main/conftest.py`:
- Around line 273-281: Extract the caller-name inspection and prefix-stripping
logic that builds expected_file into a single private helper (e.g.,
_infer_expected_file_from_caller) and have both run_generate_file_and_assert and
run_main_and_assert call it; move the code that uses inspect.currentframe(),
checks f_back, reads f_code.co_name, deletes the frame, and removes prefixes
("test_main_", "test_") into that helper so both functions share identical
behavior and avoid duplication or drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 08154532-3ab0-4fd0-826c-dab76e2dedd7
📒 Files selected for processing (2)
tests/main/conftest.pytests/main/jsonschema/test_main_jsonschema.py
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR only refactors internal test code by adding a helper function This analysis was performed by Claude Code Action |
|
🎉 Released in 0.55.0 This PR is now available in the latest release. See the release notes for details. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3047 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 86 86
Lines 18091 18031 -60
Branches 2108 2112 +4
=========================================
- Hits 18091 18031 -60
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Testing
Summary by CodeRabbit