feat: add tests_with_reruns count as suite attribute#12
Conversation
📝 WalkthroughWalkthroughSuite-level attributes are precomputed before suite start, aggregating rerun counts and conditionally adding Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
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 `@src/reportportal/writer.py`:
- Around line 222-224: The suite attribute "tests_with_reruns" is only added
when rerun_test_count > 0, but it must be emitted for every suite; update the
logic in writer.py where suite_attrs is built (the block referencing suite_attrs
and rerun_test_count) to always append {"key": "tests_with_reruns", "value":
str(rerun_test_count)} regardless of its value so zero counts are recorded as
"0".
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 810920b3-c2df-4366-8f8c-b92535441cbf
📒 Files selected for processing (1)
src/reportportal/writer.py
zkraus
left a comment
There was a problem hiding this comment.
Recommended to prefix a code block with a header comment. Otherwise LGTM.
Consider it pre-approved.
Signed-off-by: Silvia Tarabova <starabov@redhat.com>
cb1d087 to
eb6c4a4
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/reportportal/writer.py (2)
299-307: Add an explicit type forcase_resultin the private method contract.Typing this parameter as
CasePropertyResultimproves readability and static checking for this refactor.🧩 Suggested typing improvement
-from .property_processor import PropertyFilter, LaunchPropertyBuilder +from .property_processor import PropertyFilter, LaunchPropertyBuilder, CasePropertyResult ... - def _process_test_case(self, case_data: dict, case_result, + def _process_test_case(self, case_data: dict, case_result: CasePropertyResult, suite_id: str, start_time: int) -> int:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/reportportal/writer.py` around lines 299 - 307, The private method _process_test_case currently types case_result loosely; update its signature to annotate case_result as CasePropertyResult (def _process_test_case(self, case_data: dict, case_result: CasePropertyResult, suite_id: str, start_time: int) -> int) and add or update the import for CasePropertyResult at the top of the module if missing, then run and adjust any call sites to satisfy the new type (or add typing.cast where necessary) to ensure static checkers recognize the strong type.
236-239: Consider guarding against silentziptruncation.
zip(...)will silently drop trailing items if the lists diverge in a future edit. A small defensive length check here would make failures explicit.🔧 Suggested defensive check
+ if len(case_results) != len(suite_data['test_cases']): + raise RuntimeError("Internal mismatch: case_results and test_cases lengths differ") for case_data, case_result in zip(suite_data['test_cases'], case_results): case_runtime = self._process_test_case( case_data, case_result, suite_id, suite_timestamp + suite_runtime )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/reportportal/writer.py` around lines 236 - 239, The loop using zip(suite_data['test_cases'], case_results) can silently drop items if the two lists differ in length; before the for-loop that calls self._process_test_case(case_data, case_result, suite_id, suite_timestamp + suite_runtime) add a defensive check that compares len(suite_data['test_cases']) to len(case_results) and, if they differ, emit a clear error (e.g., process/log an error) and raise an exception (ValueError) so the mismatch is explicit rather than silently truncating test cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/reportportal/writer.py`:
- Around line 299-307: The private method _process_test_case currently types
case_result loosely; update its signature to annotate case_result as
CasePropertyResult (def _process_test_case(self, case_data: dict, case_result:
CasePropertyResult, suite_id: str, start_time: int) -> int) and add or update
the import for CasePropertyResult at the top of the module if missing, then run
and adjust any call sites to satisfy the new type (or add typing.cast where
necessary) to ensure static checkers recognize the strong type.
- Around line 236-239: The loop using zip(suite_data['test_cases'],
case_results) can silently drop items if the two lists differ in length; before
the for-loop that calls self._process_test_case(case_data, case_result,
suite_id, suite_timestamp + suite_runtime) add a defensive check that compares
len(suite_data['test_cases']) to len(case_results) and, if they differ, emit a
clear error (e.g., process/log an error) and raise an exception (ValueError) so
the mismatch is explicit rather than silently truncating test cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1eea9141-10c5-49d8-897c-a75f671b8848
📒 Files selected for processing (1)
src/reportportal/writer.py
Summary
tests_with_rerunsattribute to each test suite, reporting the number of test cases that had rerun attemptsSummary by CodeRabbit