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

Support for Branch Coverage #196

Conversation

EmbeddedDevops1
Copy link
Collaborator

@EmbeddedDevops1 EmbeddedDevops1 commented Oct 28, 2024

PR Type

enhancement, tests


Description

  • Enhanced the coverage processing to support branch coverage alongside line coverage.
  • Updated logging and reporting to include branch coverage metrics.
  • Modified coverage parsing methods to return branch coverage percentages.
  • Added and updated tests to validate branch coverage parsing and reporting.
  • Enabled branch coverage in lcov commands for C and C++ CLI tests.

Changes walkthrough 📝

Relevant files
Enhancement
CoverAgent.py
Add branch coverage logging and reporting                               

cover_agent/CoverAgent.py

  • Enhanced logging to include branch coverage.
  • Updated failure messages to reflect branch coverage.
  • +4/-4     
    CoverageProcessor.py
    Extend coverage parsing to include branch coverage             

    cover_agent/CoverageProcessor.py

  • Updated parsing methods to include branch coverage.
  • Modified return types to include branch coverage percentages.
  • Enhanced logging for coverage data.
  • +80/-43 
    UnitTestGenerator.py
    Track and report branch coverage in tests                               

    cover_agent/UnitTestGenerator.py

  • Added branch coverage tracking.
  • Updated coverage report to include branch coverage.
  • +8/-5     
    build_and_test_with_coverage.sh
    Enable branch coverage in C CLI tests                                       

    templated_tests/c_cli/build_and_test_with_coverage.sh

    • Enabled branch coverage in lcov commands.
    +3/-6     
    build_and_test_with_coverage.sh
    Enable branch coverage in C++ CLI tests                                   

    templated_tests/cpp_cli/build_and_test_with_coverage.sh

    • Enabled branch coverage in lcov commands.
    +3/-6     
    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.
  • +25/-11 
    test_UnitTestGenerator.py
    Validate branch coverage in unit tests                                     

    tests/test_UnitTestGenerator.py

    • Updated test to include branch coverage validation.
    +4/-3     

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

    Copy link
    Contributor

    qodo-merge-pro bot commented Oct 28, 2024

    PR Reviewer Guide 🔍

    (Review updated until commit 74a2e44)

    Here are some key observations to aid the review process:

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

    Error Handling
    The branch coverage parsing in condition-coverage could fail if the format is unexpected. Consider adding error handling for malformed coverage data.

    Code Duplication
    The branch coverage percentage calculation is duplicated across multiple methods. Consider extracting this logic into a helper method.

    Default Values
    Branch coverage is initialized to 0 when not available. Consider making this behavior configurable or more explicit.

    Copy link
    Contributor

    qodo-merge-pro bot commented Oct 28, 2024

    PR Code Suggestions ✨

    Latest suggestions up to 74a2e44
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add defensive parsing of XML coverage data to prevent runtime errors from malformed input

    Add error handling for malformed condition-coverage data in the XML to prevent
    potential runtime errors if the format is unexpected.

    cover_agent/CoverageProcessor.py [166-167]

     condition_coverage = line.get('condition-coverage')
    -covered, total = map(int, condition_coverage.split('(')[1].split(')')[0].split('/'))
    +if condition_coverage and '(' in condition_coverage and ')' in condition_coverage:
    +    try:
    +        covered, total = map(int, condition_coverage.split('(')[1].split(')')[0].split('/'))
    +    except (ValueError, IndexError):
    +        covered, total = 0, 0
    +else:
    +    covered, total = 0, 0
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses a potential runtime error that could crash the application when parsing malformed XML data. Adding proper error handling for condition-coverage parsing is crucial for application stability.

    8
    Initialize all coverage-related attributes in constructor to prevent attribute access errors

    Initialize branches_coverage in the constructor to prevent potential AttributeError
    when accessing uninitialized attribute.

    cover_agent/UnitTestGenerator.py [70]

     self.current_coverage = 0
    +self.branches_coverage = 0
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion prevents potential AttributeError by properly initializing the branches_coverage attribute. This is important as the attribute is used throughout the code and could cause runtime errors if accessed before initialization.

    7

    💡 Need additional feedback ? start a PR chat


    Previous suggestions

    Suggestions up to commit 74a2e44
    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Use f-string formatting with percentage representation for better readability in logging statements

    Consider using f-string formatting for better readability and performance when
    logging the coverage information.

    cover_agent/UnitTestGenerator.py [729]

     self.logger.info(
    -    f"Test passed and coverage increased. Current line coverage: {round(new_percentage_covered * 100, 2)}%, Current Branch coverage: {round(new_branches_covered * 100 , 2)}%"
    +    f"Test passed and coverage increased. Current line coverage: {new_percentage_covered:.2%}, Current Branch coverage: {new_branches_covered:.2%}"
     )
    Suggestion importance[1-10]: 6

    Why: The suggestion to use f-string formatting with percentage representation improves readability and consistency in logging statements. This is a beneficial change for maintaining clear and professional logging output.

    6
    Use more descriptive variable names to improve code readability

    Consider using a more descriptive variable name instead of branch_pct to clearly
    indicate that it represents branch coverage percentage.

    tests/test_CoverageProcessor.py [44]

    -covered_lines, missed_lines, coverage_pct, branch_pct = processor.parse_coverage_report()
    +covered_lines, missed_lines, coverage_pct, branch_coverage_pct = processor.parse_coverage_report()
    Suggestion importance[1-10]: 5

    Why: The suggestion to rename branch_pct to branch_coverage_pct improves code readability by making the variable's purpose clearer. However, the impact is minor as it does not affect functionality.

    5
    Use multi-line f-strings for complex logging statements to improve readability

    Consider using a multi-line f-string for better readability when logging the final
    coverage information.

    cover_agent/CoverAgent.py [157]

     self.logger.info(
    -    f"Reached above target coverage of {self.test_gen.desired_coverage}% (Current Line Coverage: {round(self.test_gen.current_coverage * 100, 2)}%, Current Branch Coverage: {round(self.test_gen.branches_coverage * 100, 2)}%) in {iteration_count} iterations."
    +    f"""Reached above target coverage of {self.test_gen.desired_coverage}%
    +    (Current Line Coverage: {self.test_gen.current_coverage:.2%},
    +    Current Branch Coverage: {self.test_gen.branches_coverage:.2%})
    +    in {iteration_count} iterations."""
     )
    Suggestion importance[1-10]: 5

    Why: The suggestion to use multi-line f-strings for logging statements enhances readability, especially for complex messages. While it improves code style, the functional impact is limited.

    5
    Use tuple unpacking for multiple assignments to improve code conciseness

    Consider using a tuple unpacking assignment instead of individual assignments for
    better readability and conciseness.

    cover_agent/CoverageProcessor.py [278-281]

    -missed += int(counter.attrib.get('missed', 0))
    -covered += int(counter.attrib.get('covered', 0))
    -missed_branches += int(counter.attrib.get('missed', 0))
    -covered_branches += int(counter.attrib.get('covered', 0))
    +missed, covered = map(int, (counter.attrib.get('missed', 0), counter.attrib.get('covered', 0)))
    +missed_branches, covered_branches = map(int, (counter.attrib.get('missed', 0), counter.attrib.get('covered', 0)))
    Suggestion importance[1-10]: 4

    Why: The suggestion to use tuple unpacking for assignments is a stylistic improvement that enhances code conciseness. However, it does not significantly impact the functionality or performance of the code.

    4

    @draialexis
    Copy link
    Contributor

    draialexis commented Oct 30, 2024

    @EmbeddedDevops1 Thanks again for working on this.

    I got it to run locally and I tested it: cover-agent increased line coverage to 100% and stopped there.

    Is there a new argument I should pass, like --desired-branch-coverage i and --desired-line-coverage j?


    I'm going to try to make a small PR, to add some info to the README, for other devs. I'll ping you in it

    UPDATE: The README PR was reviewed and merged

    @EmbeddedDevops1 EmbeddedDevops1 deleted the 161-branch-coverage-gets-ignored-request-support-for-branch-coverage branch November 13, 2024 05:11
    @EmbeddedDevops1 EmbeddedDevops1 restored the 161-branch-coverage-gets-ignored-request-support-for-branch-coverage branch November 13, 2024 05:11
    Copy link
    Contributor

    Persistent review updated to latest commit 74a2e44

    @EmbeddedDevops1
    Copy link
    Collaborator Author

    @EmbeddedDevops1 Thanks again for working on this.

    I got it to run locally and I tested it: cover-agent increased line coverage to 100% and stopped there.

    Is there a new argument I should pass, like --desired-branch-coverage i and --desired-line-coverage j?

    I'm going to try to make a small PR, to add some info to the README, for other devs. I'll ping you in it

    UPDATE: The README PR was reviewed and merged

    @draialexis Currently there isn't. The previous implementation made it difficult to update and add new data points to the LLM without the potential of introducing a regression. Now that cover_agent is being refactored, it should be easier for cover_agent to ingest new data and we can include a --desired-line-coverage flag to be passed and considered by the LLM. We're working through a few other issues in parallel but we plan to get back to this ASAP.

    @draialexis
    Copy link
    Contributor

    Sweet! Thanks for the update.

    Refactoring is healthy, I congratulate yall.

    I re-read my previous comment and realized some of it was unclear -- rephrasing now in case that's useful:

    When I ran this branch of Cover Agent on a source file that already had around 70% line coverage, and around 50% branch coverage, Cover Agent maxed out line coverage and stopped early, leaving branch coverage well below the desired number.

    @EmbeddedDevops1 EmbeddedDevops1 deleted the 161-branch-coverage-gets-ignored-request-support-for-branch-coverage branch November 19, 2024 17:55
    @EmbeddedDevops1
    Copy link
    Collaborator Author

    Heads up. I rebased and moved to a new PR: #228

    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