Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: run integration #265

Closed

Conversation

coderustic
Copy link
Contributor

@coderustic coderustic commented Jan 11, 2025

PR Type

Enhancement, Tests


Description

  • Enhanced coverage processing to merge data for duplicate filenames.

  • Added support for Kotlin in JaCoCo coverage processing.

  • Improved test cases for coverage processing, including edge cases.

  • Updated GitHub Actions workflow to trigger on pull requests.


Changes walkthrough 📝

Relevant files
Enhancement
processor.py
Enhance coverage processing and add Kotlin support             

cover_agent/coverage/processor.py

  • Added logic to merge coverage data for duplicate filenames.
  • Introduced support for Kotlin in JaCoCo coverage processing.
  • Enhanced XML parsing to handle Kotlin and Java files.
  • Improved error handling for unsupported file types.
  • +65/-14 
    Tests
    test_processor.py
    Add and enhance tests for coverage processing                       

    tests/coverage/test_processor.py

  • Updated tests to validate merged coverage data handling.
  • Added tests for Kotlin coverage processing.
  • Improved test coverage for edge cases in XML parsing.
  • Added new test cases for empty and malformed reports.
  • +155/-7 
    Configuration changes
    nightly_regression.yml
    Update GitHub Actions workflow triggers                                   

    .github/workflows/nightly_regression.yml

  • Updated workflow to trigger on pull requests to the main branch.
  • Retained nightly schedule and manual trigger options.
  • +3/-0     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Error Handling

    The _extract_package_and_class_kotlin method should handle cases where package or class name cannot be found in the file, currently it may return empty strings without warning

    def _extract_package_and_class_kotlin(self):
        package_pattern = re.compile(r"^\s*package\s+([\w.]+)\s*(?:;)?\s*(?://.*)?$")
        class_pattern = re.compile(r"^\s*(?:public|internal|abstract|data|sealed|enum|open|final|private|protected)*\s*class\s+(\w+).*")
        package_name = ""
        class_name = ""
        try:
            with open(self.src_file_path, "r") as file:
                for line in file:
                    if not package_name:  # Only match package if not already found
                        package_match = package_pattern.match(line)
                        if package_match:
                            package_name = package_match.group(1)
                    if not class_name:  # Only match class if not already found
                        class_match = class_pattern.match(line)
                        if class_match:
                            class_name = class_match.group(1)
                    if package_name and class_name:  # Exit loop if both are found
                        break
        except (FileNotFoundError, IOError) as e:
            self.logger.error(f"Error reading file {self.src_file_path}: {e}")
            raise
        return package_name, class_name
    Logic Bug

    The parse_coverage_report method has a typo calling extract_package_and_class_java() instead of _extract_package_and_class_java() when file extension is unsupported

    self.logger.warn(f"Unsupported Bytecode Language: {source_file_extension}. Using default Java logic.")
    package_name, class_name = self.extract_package_and_class_java()
    Performance

    The _merge_coverage_data method performs unnecessary list concatenations that could be optimized using sets or counters for better performance with large coverage data

    def _merge_coverage_data(self, existing_coverage: CoverageData, new_coverage: CoverageData) -> CoverageData:
        covered_lines = existing_coverage.covered_lines + new_coverage.covered_lines
        missed_lines = existing_coverage.missed_lines + new_coverage.missed_lines
        covered = existing_coverage.covered + new_coverage.covered
        missed = existing_coverage.missed + new_coverage.missed
        total_lines = covered + missed
        coverage_percentage = (float(covered) / total_lines) if total_lines > 0 else 0.0
        return CoverageData(covered_lines, covered, missed_lines, missed, coverage_percentage)

    Copy link
    Contributor

    Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Fix incorrect method name causing runtime error in fallback case

    Fix the incorrect method name in the fallback case for unsupported file extensions.
    The current code calls 'extract_package_and_class_java()' but the correct method
    name is '_extract_package_and_class_java()'.

    cover_agent/coverage/processor.py [245-247]

     else:
         self.logger.warn(f"Unsupported Bytecode Language: {source_file_extension}. Using default Java logic.")
    -    package_name, class_name = self.extract_package_and_class_java()
    +    package_name, class_name = self._extract_package_and_class_java()
    • Apply this suggestion
    Suggestion importance[1-10]: 10

    Why: This fixes a critical bug where the fallback case would raise an AttributeError due to calling a non-existent method, completely breaking the coverage processing for unsupported file types.

    10
    Add validation for empty package and class names to prevent silent failures

    Add error handling for the case when both package_name and class_name are empty
    strings after extraction. This could happen if the file doesn't contain valid class
    or package declarations.

    cover_agent/coverage/processor.py [241-244]

     if source_file_extension == 'java':
         package_name, class_name = self._extract_package_and_class_java()
     elif source_file_extension == 'kt':
         package_name, class_name = self._extract_package_and_class_kotlin()
    +if not package_name or not class_name:
    +    raise ValueError(f"Could not extract package and class names from {self.src_file_path}")
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses a critical error handling case that could lead to runtime errors or incorrect coverage data if package or class names are not found. This is particularly important for the core functionality of coverage processing.

    8
    Add proper error handling for missing XML attributes to prevent runtime errors

    Add type checking for line attributes in _parse_jacoco_xml to prevent potential
    TypeError when 'nr' or 'mi' attributes are missing.

    cover_agent/coverage/processor.py [333-337]

     for line in sourcefile.findall('line'):
    -    if line.attrib.get('mi') == '0':
    -        covered += [int(line.attrib.get('nr', 0))]
    -    else :
    -        missed += [int(line.attrib.get('nr', 0))]
    +    line_number = line.attrib.get('nr')
    +    if not line_number:
    +        continue
    +    if line.attrib.get('mi', '1') == '0':
    +        covered.append(int(line_number))
    +    else:
    +        missed.append(int(line_number))
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion improves robustness by properly handling missing XML attributes, preventing potential TypeErrors and making the code more resilient to malformed XML input.

    7

    @coderustic coderustic closed this Jan 11, 2025
    @coderustic coderustic deleted the usr/coderustic/run-integration branch January 11, 2025 20:51
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant