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

Removing bad reference to self.last_coverage_percentages #262

Conversation

kelper-hub
Copy link
Contributor

@kelper-hub kelper-hub commented Jan 8, 2025

User description

This is causing logs that look like that are causing errors when there really aren't any errors. self.last_coverage_percentages is never updated, so there is no key to be referenced.

Before:

2025-01-07 22:14:08,946 - cover_agent.UnitTestValidator - INFO - Running test with the following command: "pytest --cov=. --cov-report=xml --cov-report=term"
2025-01-07 22:14:10,013 - cover_agent.UnitTestValidator - INFO - coverage: Percentage 71.93%
2025-01-07 22:14:10,013 - cover_agent.UnitTestValidator - ERROR - Error validating test: 'app.py'

After:

2025-01-07 22:14:55,079 - cover_agent.UnitTestValidator - INFO - Running test with the following command: "pytest --cov=. --cov-report=xml --cov-report=term"
2025-01-07 22:14:56,156 - cover_agent.UnitTestValidator - INFO - coverage: Percentage 82.61%
2025-01-07 22:14:56,156 - cover_agent.UnitTestValidator - INFO - Coverage for provided source file: app.py increased to 72.73
2025-01-07 22:14:56,156 - cover_agent.UnitTestValidator - INFO - Test passed and coverage increased. Current coverage: 82.61%

PR Type

Bug fix


Description

  • Removed invalid reference to self.last_coverage_percentages.

  • Fixed misleading error logs during test validation.

  • Improved logging for coverage changes in source and non-source files.

  • Ensured accurate reporting of test coverage increases.


Changes walkthrough 📝

Relevant files
Bug fix
UnitTestValidator.py
Fix invalid reference and improve coverage logging             

cover_agent/UnitTestValidator.py

  • Removed references to self.last_coverage_percentages to fix errors.
  • Updated logging messages for coverage increases.
  • Improved clarity and accuracy of coverage-related logs.
  • +3/-4     

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

    Copy link
    Contributor

    qodo-merge-pro bot commented Jan 8, 2025

    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: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Log Format

    Consider adding a consistent format for coverage percentage logging - some messages use % symbol while others don't. This could lead to confusion when parsing logs.

            f"Coverage for provided source file: {key} increased to {round(new_v.coverage * 100, 2)}"
        )
    elif new_v.coverage > old_v.coverage:
        self.logger.info(
            f"Coverage for non-source file: {key} increased to {round(new_v.coverage * 100, 2)}"

    Copy link
    Contributor

    qodo-merge-pro bot commented Jan 8, 2025

    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
    General
    Include both old and new values when logging metric changes to improve tracking and debugging capabilities

    Include the previous coverage value in the log message to better track coverage
    changes. Use old_v.coverage which is already available.

    cover_agent/UnitTestValidator.py [572-574]

     self.logger.info(
    -    f"Coverage for provided source file: {key} increased to {round(new_v.coverage * 100, 2)}"
    +    f"Coverage for provided source file: {key} increased from {round(old_v.coverage * 100, 2)} to {round(new_v.coverage * 100, 2)}"
     )
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion improves observability by showing the coverage change delta, using the already available old_v.coverage value. This makes it easier to track and analyze coverage improvements.

    7
    Maintain consistent logging format across similar operations for better readability and analysis

    Apply the same improvement to the non-source file coverage logging for consistency
    in reporting.

    cover_agent/UnitTestValidator.py [576-578]

     self.logger.info(
    -    f"Coverage for non-source file: {key} increased to {round(new_v.coverage * 100, 2)}"
    +    f"Coverage for non-source file: {key} increased from {round(old_v.coverage * 100, 2)} to {round(new_v.coverage * 100, 2)}"
     )
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion maintains consistency in logging format with the previous suggestion, making the logs more uniform and easier to analyze. It uses the same pattern of showing coverage changes from old to new values.

    7

    @EmbeddedDevops1 EmbeddedDevops1 self-assigned this Jan 9, 2025
    @EmbeddedDevops1 EmbeddedDevops1 self-requested a review January 9, 2025 04:26
    @EmbeddedDevops1 EmbeddedDevops1 merged commit 1b1a9ad into qodo-ai:main Jan 9, 2025
    7 checks passed
    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.

    2 participants