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

Initial commit of rebased branch coverage. #228

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

EmbeddedDevops1
Copy link
Collaborator

@EmbeddedDevops1 EmbeddedDevops1 commented Nov 19, 2024

PR Type

enhancement, tests


Description

  • Added support for branch coverage calculation in coverage report parsing methods across different formats (Cobertura, LCOV, JaCoCo).
  • Updated CoverageProcessor to return branch coverage percentages alongside line coverage.
  • Enhanced UnitTestValidator to incorporate branch coverage in coverage validation and reporting.
  • Modified shell scripts to enable branch coverage in LCOV report generation for C and C++ CLI projects.
  • Updated test cases to validate the new branch coverage functionality, ensuring comprehensive test coverage.

Changes walkthrough 📝

Relevant files
Enhancement
CoverageProcessor.py
Add branch coverage support to coverage parsing                   

cover_agent/CoverageProcessor.py

  • Added branch coverage calculation to coverage report parsing methods.
  • Updated return types to include branch coverage percentage.
  • Enhanced parsing logic for different coverage report formats.
  • +102/-37
    UnitTestValidator.py
    Integrate branch coverage into validation logic                   

    cover_agent/UnitTestValidator.py

  • Integrated branch coverage into coverage validation logic.
  • Updated methods to handle branch coverage data.
  • +10/-7   
    build_and_test_with_coverage.sh
    Enable branch coverage in LCOV for C CLI                                 

    templated_tests/c_cli/build_and_test_with_coverage.sh

    • Enabled branch coverage in LCOV report generation.
    +3/-3     
    build_and_test_with_coverage.sh
    Enable branch coverage in LCOV for C++ CLI                             

    templated_tests/cpp_cli/build_and_test_with_coverage.sh

    • Enabled branch coverage in LCOV report generation.
    +3/-3     
    test_all.sh
    Add branch coverage flag to integration tests                       

    tests_integration/test_all.sh

    • Added branch coverage flag to pytest command.
    +1/-1     
    Tests
    test_CoverageProcessor.py
    Add tests for branch coverage parsing                                       

    tests/test_CoverageProcessor.py

  • Added tests for branch coverage parsing.
  • Updated existing tests to include branch coverage assertions.
  • +155/-25
    test_UnitTestValidator.py
    Update tests for branch coverage validation                           

    tests/test_UnitTestValidator.py

  • Updated tests to validate branch coverage integration.
  • Added assertions for branch coverage in test cases.
  • +14/-8   

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

    Copy link
    Contributor

    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

    Code Complexity
    The parse_coverage_data_for_class method has become more complex with the addition of branch coverage calculation. Consider splitting the method into smaller, more focused methods for better maintainability.

    Error Handling
    The branch coverage parsing in parse_coverage_report_lcov may fail if the BRDA line format is unexpected. Consider adding more robust error handling for malformed branch coverage data.

    Code Duplication
    Similar branch coverage handling code is repeated across multiple test methods. Consider extracting common branch coverage validation logic into a helper method.

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Initialize class attributes before first use to prevent runtime errors

    Add initialization of self.current_branch_coverage in the class to prevent potential
    AttributeError if accessed before being set.

    cover_agent/UnitTestValidator.py [300]

    +if not hasattr(self, 'current_branch_coverage'):
    +    self.current_branch_coverage = 0.0
     self.current_branch_coverage = branch_coverage
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This is an important defensive programming practice that prevents potential AttributeError exceptions if the branch_coverage attribute is accessed before being set, improving code robustness.

    8
    Test case should validate handling of malformed branch coverage data to ensure robustness

    The test_parse_coverage_data_for_class_with_branch_coverage test case is missing
    validation for the case where branch coverage data is malformed. Add assertions to
    verify the method handles invalid branch coverage data gracefully.

    tests/test_CoverageProcessor.py [523-540]

     def test_parse_coverage_data_for_class_with_branch_coverage(self):
         """
         Test parse_coverage_data_for_class with branch coverage data.
         """
         xml_str = """<class>
                         <lines>
                             <line number="1" hits="1" branch="true" condition-coverage="50%(1/2)"/>
                             <line number="2" hits="0"/>
    +                        <line number="3" hits="1" branch="true" condition-coverage="invalid"/>
                         </lines>
                         </class>"""
         cls = ET.fromstring(xml_str)
         processor = CoverageProcessor("fake_path", "app.py", "lcov", use_report_coverage_feature_flag=False)
         lines_covered, lines_missed, line_coverage_percentage, branch_coverage_percentage = processor.parse_coverage_data_for_class(cls)
    -    assert lines_covered == [1], "Expected line 1 to be covered"
    +    assert lines_covered == [1, 3], "Expected lines 1 and 3 to be covered"
         assert lines_missed == [2], "Expected line 2 to be missed"
    -    assert line_coverage_percentage == 0.5, "Expected 50% line coverage"
    +    assert line_coverage_percentage == 2/3, "Expected 66.7% line coverage"
         assert branch_coverage_percentage == 0.5, "Expected 50% branch coverage"
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion improves test coverage by adding validation for malformed branch coverage data, which is important for ensuring the code handles invalid input gracefully. This helps prevent potential runtime errors.

    7
    General
    Include branch coverage metrics in the coverage report output for completeness

    Add branch coverage to the code coverage report string since it's now being tracked.
    This ensures consistent reporting of both line and branch coverage metrics.

    cover_agent/UnitTestValidator.py [731]

    -self.code_coverage_report = f"Lines covered: {lines_covered}\nLines missed: {lines_missed}\nPercentage covered: {round(percentage_covered * 100, 2)}%"
    +self.code_coverage_report = f"Lines covered: {lines_covered}\nLines missed: {lines_missed}\nLine coverage: {round(percentage_covered * 100, 2)}%\nBranch coverage: {round(branch_covered * 100, 2)}%"
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion improves consistency and completeness of reporting by including branch coverage metrics alongside line coverage, making the coverage report more informative and aligned with the PR's branch coverage integration.

    7

    💡 Need additional feedback ? start a PR chat

    @EmbeddedDevops1 EmbeddedDevops1 force-pushed the 161-branch-coverage-gets-ignored-request-support-for-branch-coverage branch from 950610d to 82cd311 Compare November 19, 2024 18:21
    @draialexis
    Copy link
    Contributor

    Is support for branch coverage currently part of this product's roadmap?

    @EmbeddedDevops1
    Copy link
    Collaborator Author

    Is support for branch coverage currently part of this product's roadmap?

    It is but we're currently undergoing some refactoring right now to make that a bit easier. Stay tuned!

    ...Or feel free to put up a PR 😄

    @draialexis
    Copy link
    Contributor

    draialexis commented Jan 7, 2025 via email

    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.

    Branch coverage gets ignored (Request: Support for Branch Coverage)
    2 participants