Added support for Jacoco XML parsing

[leandrogianotti]

PR Type

Enhancement, Bug fix

---

Description

• Added support for parsing JaCoCo XML coverage reports by implementing the parse_missed_covered_lines_jacoco_xml method.
• Introduced the get_file_extension method to determine the file extension of the coverage report.
• Refactored the parse_coverage_report_jacoco method to handle both XML and CSV formats, raising an error for unsupported formats.
• Improved error handling and code readability.

---

Changes walkthrough 📝

	Relevant files
Enhancement	CoverageProcessor.py Add support for JaCoCo XML parsing and refactor coverage processing cover_agent/CoverageProcessor.py Added support for parsing JaCoCo XML coverage reports. Introduced a method to determine file extension. Refactored code to handle different file formats (XML, CSV). Improved error handling for unsupported formats. +42/-4

---

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro[bot]]

PR Reviewer Guide 🔍

(Review updated until commit https://github.com/Codium-ai/cover-agent/commit/8495ff4f4dfcd5347e8011e93f773b76463700c4)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Error Handling The method `parse_missed_covered_lines_jacoco_xml` does not handle XML parsing errors. Consider adding exception handling for potential parsing issues, such as malformed XML. Magic String The string 'LINE' used in `parse_missed_covered_lines_jacoco_xml` is a magic string. Consider defining it as a constant to avoid potential errors and improve maintainability.

[pr-agent-pro-staging[bot]]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

Persistent review updated to latest commit https://github.com/Codium-ai/cover-agent/commit/8495ff4f4dfcd5347e8011e93f773b76463700c4

[pr-agent-pro-staging[bot]]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Add error handling for XML parsing to improve robustness ___ **Consider adding error handling for the case where the XML parsing fails due to an invalid file path or malformed XML content. This can prevent the method from failing silently and help in diagnosing issues more effectively.** [cover_agent/CoverageProcessor.py [165-166]](https://github.com/Codium-ai/cover-agent/pull/132/files#diff-610c623a5c322b67c9a12fa75021723cc335d4dc3ce4676eb8576a20f3ffd93eR165-R166) ```diff -tree = ET.parse(self.file_path) -root = tree.getroot() +try: + tree = ET.parse(self.file_path) + root = tree.getroot() +except ET.ParseError as e: + raise ValueError(f"Failed to parse XML: {e}") ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 9 Why: The suggestion correctly identifies a potential issue with XML parsing and provides a robust solution by adding error handling, which improves the reliability of the code.	9
	Add handling for when file extension is None to prevent type errors ___ **When extracting the file extension, the method get_file_extension might return None, which is not handled in the parse_coverage_report_jacoco method. This could lead to a TypeError when comparing NoneType to a string. Consider adding a check for None before the file extension comparison.** [cover_agent/CoverageProcessor.py [142-148]](https://github.com/Codium-ai/cover-agent/pull/132/files#diff-610c623a5c322b67c9a12fa75021723cc335d4dc3ce4676eb8576a20f3ffd93eR142-R148) ```diff file_extension = self.get_file_extension(self.file_path) -if file_extension == 'xml': +if file_extension is None: + raise ValueError("File extension could not be determined.") +elif file_extension == 'xml': missed, covered = self.parse_missed_covered_lines_jacoco_xml( class_name ) ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 8 Why: The suggestion correctly identifies a potential issue where `None` could be returned and provides a solution to handle this case, preventing a possible `TypeError`.	8
Possible bug	Ensure all relevant XML 'counter' elements are processed ___ **The method parse_missed_covered_lines_jacoco_xml breaks the loop after finding the first 'counter' element of type 'LINE'. If the XML structure includes multiple 'counter' elements before the desired one, this might lead to incorrect results. Consider removing the break statement to ensure all relevant 'counter' elements are processed.** [cover_agent/CoverageProcessor.py [173-177]](https://github.com/Codium-ai/cover-agent/pull/132/files#diff-610c623a5c322b67c9a12fa75021723cc335d4dc3ce4676eb8576a20f3ffd93eR173-R177) ```diff for counter in sourcefile.findall('counter'): if counter.attrib.get('type') == 'LINE': missed += int(counter.attrib.get('missed', 0)) covered += int(counter.attrib.get('covered', 0)) - break ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 8 Why: The suggestion addresses a potential bug by ensuring that all relevant 'counter' elements are processed, which could lead to more accurate results.	8
	Add explicit handling for division by zero to enhance code safety ___ **To ensure that the division operation does not lead to a division by zero error, consider adding a check before performing the division. This can make the code more robust by explicitly handling cases where total_lines might be zero.** [cover_agent/CoverageProcessor.py [157]](https://github.com/Codium-ai/cover-agent/pull/132/files#diff-610c623a5c322b67c9a12fa75021723cc335d4dc3ce4676eb8576a20f3ffd93eR157-R157) ```diff -coverage_percentage = (float(covered) / total_lines) if total_lines > 0 else 0 +if total_lines == 0: + coverage_percentage = 0 +else: + coverage_percentage = float(covered) / total_lines ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7 Why: The suggestion improves code safety by explicitly handling the case where `total_lines` might be zero, preventing a potential division by zero error. However, the existing code already handles this with a conditional expression.	7

[codiumai-pr-agent-pro[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add error handling for XML parsing to prevent runtime crashes ___ **Add error handling for potential exceptions when parsing the XML file in parse_missed_covered_lines_jacoco_xml. This could include file not found errors or XML parsing errors, which should be caught and handled gracefully.** [cover_agent/CoverageProcessor.py [165-166]](https://github.com/Codium-ai/cover-agent/pull/132/files#diff-610c623a5c322b67c9a12fa75021723cc335d4dc3ce4676eb8576a20f3ffd93eR165-R166) ```diff -tree = ET.parse(self.file_path) -root = tree.getroot() +try: + tree = ET.parse(self.file_path) + root = tree.getroot() +except (ET.ParseError, FileNotFoundError) as e: + logging.error(f"Error parsing XML file: {e}") + return 0, 0 ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 10 Why: This suggestion is crucial as it adds error handling for potential exceptions during XML parsing, preventing runtime crashes and ensuring the program can handle unexpected issues gracefully.	10
	Add a check for the existence of 'counter' elements to prevent potential errors ___ **To ensure that the parse_missed_covered_lines_jacoco_xml method handles cases where the XML file does not contain the expected structure, add a check for the existence of the 'counter' elements before attempting to access their attributes. This prevents potential AttributeError if the 'counter' elements are missing.** [cover_agent/CoverageProcessor.py [173-176]](https://github.com/Codium-ai/cover-agent/pull/132/files#diff-610c623a5c322b67c9a12fa75021723cc335d4dc3ce4676eb8576a20f3ffd93eR173-R176) ```diff -for counter in sourcefile.findall('counter'): +counters = sourcefile.findall('counter') +if not counters: + return 0, 0 +for counter in counters: if counter.attrib.get('type') == 'LINE': missed += int(counter.attrib.get('missed', 0)) covered += int(counter.attrib.get('covered', 0)) break ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 9 Why: This suggestion addresses a potential bug by ensuring that the code does not attempt to access attributes of non-existent 'counter' elements, which could lead to an `AttributeError`. It is a crucial improvement for robustness.	9
Possible issue	✅ Handle the case where file extension is None to prevent type errors ___ Suggestion Impact: The commit modified the get_file_extension method to use os.path.splitext, which inherently handles the case where there is no file extension, thus preventing None from being returned. code diff: ```diff def get_file_extension(self, filename: str) -> str | None: """Get the file extension from a given filename.""" - match = re.search(r'\.(\w+)$', filename) - if match: - return match.group(1) - else: - return None + return os.path.splitext(filename)[1].lstrip(".") ``` ___ **Consider handling the case where the get_file_extension method returns None. This could lead to a TypeError when comparing NoneType to string in the conditional checks for file extensions. You can either raise an exception or handle this case explicitly.** [cover_agent/CoverageProcessor.py [142-146]](https://github.com/Codium-ai/cover-agent/pull/132/files#diff-610c623a5c322b67c9a12fa75021723cc335d4dc3ce4676eb8576a20f3ffd93eR142-R146) ```diff file_extension = self.get_file_extension(self.file_path) +if file_extension is None: + raise ValueError("File extension could not be determined.") if file_extension == 'xml': missed, covered = self.parse_missed_covered_lines_jacoco_xml(class_name) ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 8 Why: This suggestion prevents a potential `TypeError` by handling cases where the file extension is `None`. It ensures the code behaves correctly even when the file extension cannot be determined.	8
Maintainability	Refactor file format processing into a separate method to improve maintainability ___ **Refactor the conditional logic for processing different file formats into a separate method to improve code readability and maintainability. This method could map file extensions to their respective parsing functions.** [cover_agent/CoverageProcessor.py [144-154]](https://github.com/Codium-ai/cover-agent/pull/132/files#diff-610c623a5c322b67c9a12fa75021723cc335d4dc3ce4676eb8576a20f3ffd93eR144-R154) ```diff -if file_extension == 'xml': - missed, covered = self.parse_missed_covered_lines_jacoco_xml(class_name) -elif file_extension == 'csv': - missed, covered = self.parse_missed_covered_lines_jacoco_csv(package_name, class_name) -else: - raise ValueError(f"Unsupported JaCoCo code coverage report format: {file_extension}") +def process_file_based_on_extension(self, file_extension, package_name, class_name): + format_to_function = { + 'xml': self.parse_missed_covered_lines_jacoco_xml, + 'csv': self.parse_missed_covered_lines_jacoco_csv + } + if file_extension in format_to_function: + return format_to_function[file_extension](package_name, class_name) + else: + raise ValueError(f"Unsupported JaCoCo code coverage report format: {file_extension}") ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7 Why: This suggestion improves code readability and maintainability by refactoring the conditional logic into a separate method. While it does not fix a bug, it enhances the structure and clarity of the code.	7

[EmbeddedDevops1]

Looks like the CI job is failing. Unit tests are not currently passing.

[EmbeddedDevops1]

@leandrogianotti Thanks again for the PR. Are you able to add a unit test that validates the XML piece of the coverage parser?

[EmbeddedDevops1]

@leandrogianotti This is great and thanks for adding the unit tests. Did you run this against any of the templated projects (or your own) to validate XML parsing is working as expected?

[leandrogianotti]

@leandrogianotti This is great and thanks for adding the unit tests. Did you run this against any of the templated projects (or your own) to validate XML parsing is working as expected?

I have run it against my own project which by configuration only generates the Jacoco report in xml and it worked fine. Do you need me to run it against the project templates and attach evidence?

[EmbeddedDevops1]

That would be ideal just so we have the validation.