Fix command recommend fail #904
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdded a guard in aibolit/main.py to skip non-dict pattern items when processing results. Introduced test/xml/init.py. Added two unit tests verifying XML report generation for nested and flat result structures via create_xml_tree. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI Runner
participant Main as __main__._process_file_result
participant Patterns as patterns iterable
CLI->>Main: process file result
loop iterate patterns
alt item is dict
Main->>Main: read pattern.get("pattern_code")
Main->>CLI: record pattern_code
else not a dict
Note over Main: guard skips item
end
end
CLI-->>CLI: continue aggregation
sequenceDiagram
autonumber
participant Test as unit test
participant XML as create_xml_tree
participant Output as XML string
Test->>XML: create_xml_tree(results, exit_code=0, cmd)
XML-->>Output: return XML containing patterns (e.g., "P1","P2")
Test->>Test: assert "P1" and "P2" in XML
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/xml/test_xml.py (1)
1-33: Tests cannot run in current formThese new tests never execute successfully:
- The functions are module-level but still take
self, so pytest treatsselfas a fixture and errors out (see pipeline failure).create_xml_treeis never imported, producing F821.- The call omits the required
full_reportargument, so even with an import you’d get aTypeError.create_xml_treereturns an lxmlElement, soself.assertIn('P1', result)will fail—you need to serialize (e.g.,etree.tostring(..., encoding='unicode')) or navigate the tree.
Please restructure the tests (e.g., move them back inside the appropriate test class or make them pure functions), importcreate_xml_tree, passfull_report, and assert on the rendered XML before we can merge.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
aibolit/__main__.py(1 hunks)test/xml/__init__.py(1 hunks)test/xml/test_xml.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/xml/test_xml.py (1)
aibolit/__main__.py (1)
create_xml_tree(573-601)
🪛 GitHub Actions: ty
test/xml/test_xml.py
[error] 14-14: unresolved-reference: Name create_xml_tree used when not defined.
[error] 30-30: unresolved-reference: Name create_xml_tree used when not defined.
🪛 GitHub Actions: ruff
test/xml/test_xml.py
[error] 14-14: Undefined name 'create_xml_tree'.
[error] 30-30: Undefined name 'create_xml_tree'.
aibolit/__main__.py
[warning] 449-449: Unexpected # ruff: noqa directive at aibolit/main.py:449. File-level suppression comments must appear on their own line. For line-level suppression, omit the ruff: prefix.
🪛 GitHub Actions: pylint
test/xml/test_xml.py
[warning] 12-12: Trailing whitespace detected.
[warning] 13-13: Access to a protected member '__create_mock_cmd' of a client class (protected-access).
[error] 14-14: Undefined variable 'create_xml_tree'.
[warning] 29-29: Access to a protected member '__create_mock_cmd' of a client class (protected-access).
[error] 30-30: Undefined variable 'create_xml_tree'.
test/xml/__init__.py
[error] 1-1: Trailing whitespace detected.
aibolit/__main__.py
[error] 537-537: Trailing whitespace detected.
🪛 GitHub Actions: flake8
test/xml/test_xml.py
[error] 12-12: W293 blank line contains whitespace
[error] 14-14: F821 undefined name 'create_xml_tree'
[error] 18-18: E302 expected 2 blank lines, found 1
[error] 22-22: W291 trailing whitespace
[error] 30-30: F821 undefined name 'create_xml_tree'
test/xml/__init__.py
[error] 1-1: W291 trailing whitespace
aibolit/__main__.py
[error] 537-537: W293 blank line contains whitespace
🪛 GitHub Actions: make
test/xml/test_xml.py
[error] 1-1: pytest error: fixture 'self' not found in test_xml_report_with_nested_results.
[error] 1-1: pytest error: fixture 'self' not found in test_xml_report_with_flat_results.
⏰ 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). (1)
- GitHub Check: latexmk
| patterns_number = len(results) | ||
| importance_for_class = [] | ||
|
|
||
There was a problem hiding this comment.
Clean up whitespace-only line
Line 537 is a blank line filled with spaces, producing W293/W trailing-whitespace failures. Please make it an actual empty line (no spaces) or remove it.
Apply this diff:
-
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| (empty line here—ensure no spaces or tabs) |
🧰 Tools
🪛 GitHub Actions: pylint
[error] 537-537: Trailing whitespace detected.
🪛 GitHub Actions: flake8
[error] 537-537: W293 blank line contains whitespace
🤖 Prompt for AI Agents
In aibolit/__main__.py around line 537, the line contains trailing spaces
causing W293/W trailing-whitespace failures; fix by removing all whitespace
characters on that line so it is an actual empty line (or delete the line
entirely) and ensure the file contains no trailing spaces on that line before
committing.
test/xml/__init__.py
Outdated
| @@ -0,0 +1 @@ | ||
| # |
There was a problem hiding this comment.
Remove trailing whitespace to satisfy linters
This file currently consists of a lone comment with a trailing space, which is tripping pylint/flake8 (W291). Please drop the trailing whitespace so the linters succeed.
Apply this diff:
-#
+#📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # | |
| # |
🧰 Tools
🪛 GitHub Actions: pylint
[error] 1-1: Trailing whitespace detected.
🪛 GitHub Actions: flake8
[error] 1-1: W291 trailing whitespace
🤖 Prompt for AI Agents
In test/xml/__init__.py around line 1, the file contains a lone comment with
trailing whitespace causing linter W291; remove the trailing space at the end of
the comment line so there is no whitespace after the '#' character and save the
file.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/xml/__init__.py(1 hunks)test/xml/test_xml.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/xml/init.py
🧰 Additional context used
🧬 Code graph analysis (1)
test/xml/test_xml.py (1)
aibolit/__main__.py (1)
create_xml_tree(573-601)
🪛 GitHub Actions: pylint
test/xml/test_xml.py
[warning] 15-15: C0303: Trailing whitespace (trailing-whitespace)
[warning] 16-16: W0212: Access to a protected member __create_mock_cmd of a client class (protected-access)
[error] 17-17: E0602: Undefined variable 'create_xml_tree' (undefined-variable)
[warning] 32-32: W0212: Access to a protected member __create_mock_cmd of a client class (protected-access)
[error] 33-33: E0602: Undefined variable 'create_xml_tree' (undefined-variable)
🪛 GitHub Actions: ty
test/xml/test_xml.py
[error] 17-17: uvx ty==0.0.1-alpha.8 check: unresolved-reference - Name 'create_xml_tree' used when not defined at test_xml.py:17.127
[error] 33-33: uvx ty==0.0.1-alpha.8 check: unresolved-reference - Name 'create_xml_tree' used when not defined at test_xml.py:33.127
🪛 GitHub Actions: ruff
test/xml/test_xml.py
[error] 17-17: F821 Undefined name create_xml_tree.
[error] 33-33: F821 Undefined name create_xml_tree.
🪛 GitHub Actions: make
test/xml/test_xml.py
[error] 4-4: fixture 'self' not found. This test defines a method with a 'self' parameter which pytest does not provide as a fixture.
[error] 21-21: fixture 'self' not found. This test defines a method with a 'self' parameter which pytest does not provide as a fixture.
🪛 GitHub Actions: flake8
test/xml/test_xml.py
[warning] 15-15: W293 blank line contains whitespace
[error] 17-17: F821 undefined name 'create_xml_tree'
[error] 21-21: E302 expected 2 blank lines, found 1
[warning] 25-25: W291 trailing whitespace
[error] 33-33: F821 undefined name 'create_xml_tree'
⏰ 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). (1)
- GitHub Check: latexmk
| def test_xml_report_with_nested_results(self): | ||
| """Test that XML report handles nested results structure correctly""" | ||
| mock_input_with_nested = [ | ||
| { | ||
| 'filename': 'test.java', | ||
| 'results': [ | ||
| [{'pattern_code': 'P1', 'importance': 5}], | ||
| [{'pattern_code': 'P2', 'importance': 3}] | ||
| ] | ||
| } | ||
| ] | ||
|
|
||
| mock_cmd = self.__create_mock_cmd() | ||
| result = create_xml_tree(mock_input_with_nested, cmd=mock_cmd, exit_code=0) | ||
| self.assertIn('P1', result) | ||
| self.assertIn('P2', result) |
There was a problem hiding this comment.
Fix unresolved names and invalid test signature before these can run
Right now the first test fails immediately: self isn’t provided for a module-level pytest test, self.__create_mock_cmd() is undefined in this scope, create_xml_tree isn’t imported, and the call omits the required full_report argument. Please either move this logic into the existing test class that defines __create_mock_cmd or turn it into a plain function without self, add the missing import (e.g., from aibolit.__main__ import create_xml_tree), and call create_xml_tree(..., full_report=<expected value>, cmd=mock_cmd, exit_code=0). Without those fixes the pipeline errors (F821, fixture “self” not found, etc.) will keep the suite red.
🧰 Tools
🪛 GitHub Actions: pylint
[warning] 15-15: C0303: Trailing whitespace (trailing-whitespace)
[warning] 16-16: W0212: Access to a protected member __create_mock_cmd of a client class (protected-access)
[error] 17-17: E0602: Undefined variable 'create_xml_tree' (undefined-variable)
🪛 GitHub Actions: ty
[error] 17-17: uvx ty==0.0.1-alpha.8 check: unresolved-reference - Name 'create_xml_tree' used when not defined at test_xml.py:17.127
🪛 GitHub Actions: ruff
[error] 17-17: F821 Undefined name create_xml_tree.
🪛 GitHub Actions: make
[error] 4-4: fixture 'self' not found. This test defines a method with a 'self' parameter which pytest does not provide as a fixture.
🪛 GitHub Actions: flake8
[warning] 15-15: W293 blank line contains whitespace
[error] 17-17: F821 undefined name 'create_xml_tree'
🤖 Prompt for AI Agents
In test/xml/test_xml.py around lines 4 to 19, the test is written as a method
using self but lives at module level and references undefined helpers and
symbols; fix by either moving this function into the existing test class that
defines __create_mock_cmd (so self and __create_mock_cmd() are valid) or convert
it to a plain pytest function: remove self, call a module-level helper to create
mock_cmd, import create_xml_tree (e.g., from aibolit.__main__ import
create_xml_tree) at top of the file, and call create_xml_tree with the required
full_report argument (e.g., create_xml_tree(mock_input_with_nested,
full_report=<expected value>, cmd=mock_cmd, exit_code=0)); ensure all names used
are defined/imported so the test no longer raises F821 or fixture “self” not
found.
| result = create_xml_tree(mock_input_with_nested, cmd=mock_cmd, exit_code=0) | ||
| self.assertIn('P1', result) | ||
| self.assertIn('P2', result) |
There was a problem hiding this comment.
Convert the XML element to text before asserting on pattern codes
create_xml_tree returns an lxml.etree.Element, so self.assertIn('P1', result) will never match—Python will iterate over child nodes, not the serialized XML, and 'P1' isn’t an element. After fixing the issues above, serialize the tree (e.g., xml = etree.tostring(result, encoding='unicode')) and assert on that string for both tests.
Also applies to: 33-35
🧰 Tools
🪛 GitHub Actions: pylint
[error] 17-17: E0602: Undefined variable 'create_xml_tree' (undefined-variable)
🪛 GitHub Actions: ty
[error] 17-17: uvx ty==0.0.1-alpha.8 check: unresolved-reference - Name 'create_xml_tree' used when not defined at test_xml.py:17.127
🪛 GitHub Actions: ruff
[error] 17-17: F821 Undefined name create_xml_tree.
🪛 GitHub Actions: flake8
[error] 17-17: F821 undefined name 'create_xml_tree'
🤖 Prompt for AI Agents
In test/xml/test_xml.py around lines 17 to 19 (and similarly lines 33 to 35),
the tests call create_xml_tree which returns an lxml.etree.Element and then
assertIn on raw Element objects; instead serialize the Element to a string
(e.g., xml = etree.tostring(result, encoding='unicode')) and then use
self.assertIn('P1', xml) and self.assertIn('P2', xml) (and the corresponding
assertions at 33-35) so the substring checks operate on the serialized XML text.
Summary by CodeRabbit
Bug Fixes
Tests
Chores