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

PR Diff that adds tests #195

Conversation

EmbeddedDevops1
Copy link
Collaborator

@EmbeddedDevops1 EmbeddedDevops1 commented Oct 28, 2024

PR Type

Enhancement, Tests


Description

  • Added support for generating tests based on diff coverage, focusing on changes between branches.
  • Implemented parsing and processing of diff coverage reports.
  • Enhanced the PromptBuilder to include diff coverage instructions.
  • Updated the UnitTestGenerator to handle diff coverage during test validation and generation.
  • Added command-line options to enable diff coverage and specify the comparison branch.
  • Updated and added tests to cover new diff coverage functionality.
  • Added diff-cover as a new dependency in pyproject.toml.

Changes walkthrough 📝

Relevant files
Enhancement
6 files
CoverAgent.py
Add diff coverage support to CoverAgent class                       

cover_agent/CoverAgent.py

  • Added support for diff coverage in the CoverAgent class.
  • Modified logging to differentiate between diff coverage and overall
    coverage.
  • Updated logic to handle diff coverage during test generation.
  • +19/-6   
    CoverageProcessor.py
    Implement parsing for diff coverage reports                           

    cover_agent/CoverageProcessor.py

  • Added method to parse diff coverage reports.
  • Extracted total, missing, and processed lines from diff coverage
    reports.
  • +17/-0   
    PromptBuilder.py
    Enhance PromptBuilder with diff coverage capabilities       

    cover_agent/PromptBuilder.py

  • Introduced diff coverage text in prompt building.
  • Added methods to extract changed lines from git diff.
  • +77/-1   
    UnitTestGenerator.py
    Integrate diff coverage in UnitTestGenerator                         

    cover_agent/UnitTestGenerator.py

  • Integrated diff coverage logic in test generation.
  • Added methods to run and validate diff coverage.
  • +69/-1   
    main.py
    Add CLI options for diff coverage                                               

    cover_agent/main.py

    • Added command-line arguments for diff coverage.
    +13/-0   
    test_generation_prompt.toml
    Update prompt template for diff coverage                                 

    cover_agent/settings/test_generation_prompt.toml

    • Added diff coverage text section to the prompt template.
    +5/-0     
    Tests
    2 files
    test_CoverAgent.py
    Update CoverAgent tests for diff coverage                               

    tests/test_CoverAgent.py

    • Updated tests to include diff coverage parameters.
    +9/-3     
    test_UnitTestGenerator.py
    Add tests for diff coverage in UnitTestGenerator                 

    tests/test_UnitTestGenerator.py

    • Added tests for diff coverage functionality.
    +44/-0   
    Formatting
    1 files
    test_PromptBuilder.py
    Format test_PromptBuilder.py                                                         

    tests/test_PromptBuilder.py

    • Minor formatting changes.
    +2/-1     
    Dependencies
    1 files
    pyproject.toml
    Add diff-cover dependency                                                               

    pyproject.toml

    • Added diff-cover as a dependency.
    +1/-0     

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

    @EmbeddedDevops1 EmbeddedDevops1 linked an issue Oct 28, 2024 that may be closed by this pull request
    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

    Possible Bug
    The diff coverage logic might not be correctly integrated with the existing coverage logic. The code switches between diff coverage and regular coverage without clear conditions.

    Code Duplication
    The diff coverage command is duplicated in both run_diff_coverage and validate_test methods. This could be refactored into a separate method.

    Performance Concern
    The _get_diff method runs two separate git commands for unstaged and staged changes. This could potentially be optimized into a single git command.

    Copy link
    Contributor

    qodo-merge-pro bot commented Oct 28, 2024

    CI Failure Feedback 🧐

    (Checks updated until commit e90ddd8)

    Action: test

    Failed stage: Run tests and generate reports [❌]

    Failed test name: tests/test_CoverAgent.py

    Failure summary:

    The action failed due to multiple errors encountered during test collection:

  • A NameError occurred because the name List is not defined in the CoverageProcessor class in the
    cover_agent/CoverageProcessor.py file. This error suggests that the List type from the typing module
    is not imported.
  • This error was repeated across multiple test files (tests/test_CoverAgent.py,
    tests/test_CoverageProcessor.py, tests/test_UnitTestGenerator.py, and tests/test_main.py) as they
    all attempt to import and use the CoverageProcessor class.
  • Additionally, the required test coverage of 65% was not reached, with the total coverage being only
    15.24%.

  • Relevant error logs:
    1:  ##[group]Operating System
    2:  Ubuntu
    ...
    
    724:  ============================= test session starts ==============================
    725:  platform linux -- Python 3.12.7, pytest-8.3.3, pluggy-1.5.0
    726:  rootdir: /home/runner/work/cover-agent/cover-agent
    727:  configfile: pyproject.toml
    728:  plugins: anyio-4.6.2.post1, timeout-2.3.1, asyncio-0.23.8, cov-5.0.0, mock-3.14.0
    729:  asyncio: mode=Mode.STRICT
    730:  ----------------------------- live log collection ------------------------------
    731:  INFO     httpx:_client.py:1038 HTTP Request: GET https://raw.githubusercontent.com/BerriAI/litellm/main/model_prices_and_context_window.json "HTTP/1.1 200 OK"
    732:  collected 47 items / 4 errors
    733:  ==================================== ERRORS ====================================
    734:  __________________ ERROR collecting tests/test_CoverAgent.py ___________________
    ...
    
    737:  cover_agent/CoverAgent.py:9: in <module>
    738:  from cover_agent.UnitTestGenerator import UnitTestGenerator
    739:  cover_agent/UnitTestGenerator.py:9: in <module>
    740:  from cover_agent.CoverageProcessor import CoverageProcessor
    741:  cover_agent/CoverageProcessor.py:10: in <module>
    742:  class CoverageProcessor:
    743:  cover_agent/CoverageProcessor.py:270: in CoverageProcessor
    744:  def parse_json_diff_coverage_report(self) -> Tuple[List[int], List[int], float]:
    745:  E   NameError: name 'List' is not defined
    746:  _______________ ERROR collecting tests/test_CoverageProcessor.py _______________
    747:  tests/test_CoverageProcessor.py:3: in <module>
    748:  from cover_agent.CoverageProcessor import CoverageProcessor
    749:  cover_agent/CoverageProcessor.py:10: in <module>
    750:  class CoverageProcessor:
    751:  cover_agent/CoverageProcessor.py:270: in CoverageProcessor
    752:  def parse_json_diff_coverage_report(self) -> Tuple[List[int], List[int], float]:
    753:  E   NameError: name 'List' is not defined
    754:  _______________ ERROR collecting tests/test_UnitTestGenerator.py _______________
    755:  tests/test_UnitTestGenerator.py:1: in <module>
    756:  from cover_agent.CoverageProcessor import CoverageProcessor
    757:  cover_agent/CoverageProcessor.py:10: in <module>
    758:  class CoverageProcessor:
    759:  cover_agent/CoverageProcessor.py:270: in CoverageProcessor
    760:  def parse_json_diff_coverage_report(self) -> Tuple[List[int], List[int], float]:
    761:  E   NameError: name 'List' is not defined
    762:  _____________________ ERROR collecting tests/test_main.py ______________________
    ...
    
    767:  cover_agent/CoverAgent.py:9: in <module>
    768:  from cover_agent.UnitTestGenerator import UnitTestGenerator
    769:  cover_agent/UnitTestGenerator.py:9: in <module>
    770:  from cover_agent.CoverageProcessor import CoverageProcessor
    771:  cover_agent/CoverageProcessor.py:10: in <module>
    772:  class CoverageProcessor:
    773:  cover_agent/CoverageProcessor.py:270: in CoverageProcessor
    774:  def parse_json_diff_coverage_report(self) -> Tuple[List[int], List[int], float]:
    775:  E   NameError: name 'List' is not defined
    776:  =============================== warnings summary ===============================
    777:  ../../../.cache/pypoetry/virtualenvs/cover-agent-a0pACUfe-py3.12/lib/python3.12/site-packages/starlette/formparsers.py:12
    778:  /home/runner/.cache/pypoetry/virtualenvs/cover-agent-a0pACUfe-py3.12/lib/python3.12/site-packages/starlette/formparsers.py:12: PendingDeprecationWarning: Please use `import python_multipart` instead.
    779:  import multipart
    780:  ../../../.cache/pypoetry/virtualenvs/cover-agent-a0pACUfe-py3.12/lib/python3.12/site-packages/pydantic/_internal/_config.py:291
    781:  /home/runner/.cache/pypoetry/virtualenvs/cover-agent-a0pACUfe-py3.12/lib/python3.12/site-packages/pydantic/_internal/_config.py:291: PydanticDeprecatedSince20: Support for class-based `config` is deprecated, use ConfigDict instead. Deprecated in Pydantic V2.0 to be removed in V3.0. See Pydantic V2 Migration Guide at https://errors.pydantic.dev/2.9/migration/
    ...
    
    809:  cover_agent/settings/config_loader.py      20     11    45%
    810:  cover_agent/utils.py                       71     65     8%
    811:  cover_agent/version.py                     11      1    91%
    812:  -----------------------------------------------------------
    813:  TOTAL                                     945    801    15%
    814:  Coverage XML written to file cobertura.xml
    815:  FAIL Required test coverage of 65% not reached. Total coverage: 15.24%
    816:  =========================== short test summary info ============================
    817:  ERROR tests/test_CoverAgent.py - NameError: name 'List' is not defined
    818:  ERROR tests/test_CoverageProcessor.py - NameError: name 'List' is not defined
    819:  ERROR tests/test_UnitTestGenerator.py - NameError: name 'List' is not defined
    820:  ERROR tests/test_main.py - NameError: name 'List' is not defined
    821:  !!!!!!!!!!!!!!!!!!! Interrupted: 4 errors during collection !!!!!!!!!!!!!!!!!!!!
    822:  ======================== 5 warnings, 4 errors in 9.42s =========================
    823:  make: *** [Makefile:8: test] Error 2
    824:  ##[error]Process completed with exit code 2.
    

    ✨ CI feedback usage guide:

    The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
    The tool analyzes the failed checks and provides several feedbacks:

    • Failed stage
    • Failed test name
    • Failure summary
    • Relevant error logs

    In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

    /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"
    

    where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

    Configuration options

    • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
    • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
    • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
    • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
    • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

    See more information about the checks tool in the docs.

    Copy link
    Contributor

    qodo-merge-pro bot commented Oct 28, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Simplify the Git diff retrieval process by using a single command for both staged and unstaged changes

    The _get_diff method could be improved by using a single Git command to get both
    staged and unstaged changes, reducing the number of subprocess calls and simplifying
    the code.

    cover_agent/PromptBuilder.py [147-169]

     def _get_diff(self, source_file_path):
         try:
    -        # Get unstaged changes
    -        command_unstaged = ["git", "diff", self.diff_branch, "--", source_file_path]
    -        result_unstaged = subprocess.run(
    -            command_unstaged,
    +        # Get both staged and unstaged changes
    +        command = ["git", "diff", "HEAD", self.diff_branch, "--", source_file_path]
    +        result = subprocess.run(
    +            command,
                 capture_output=True,
                 text=True,
                 check=True
             )
    +        return result.stdout
     
    -        # Get staged changes
    -        command_staged = ["git", "diff", "--staged", self.diff_branch, "--", source_file_path]
    -        result_staged = subprocess.run(
    -            command_staged,
    -            capture_output=True,
    -            text=True,
    -            check=True
    -        )
    -
    -        # Combine both diffs
    -        combined_diff = result_unstaged.stdout + result_staged.stdout
    -        return combined_diff
    -
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This suggestion reduces the number of subprocess calls and simplifies the code, which can improve performance and maintainability. Using a single command to retrieve both staged and unstaged changes is more efficient and reduces complexity.

    8
    Use a more robust regular expression pattern to extract numeric values from the diff coverage report

    Consider using a more robust method to extract numeric values from the diff coverage
    report. The current implementation assumes the presence of specific patterns and
    might fail if the report format changes. A more flexible approach using regular
    expressions with named capture groups could be more resilient to format changes.

    cover_agent/CoverageProcessor.py [270-278]

    -total_lines_match = re.search(r'Total:\s+(\d+)\s+lines', report_text)
    -total_lines = int(total_lines_match.group(1)) if total_lines_match else 0
    +pattern = r'Total:\s+(?P<total>\d+)\s+lines.*Missing:\s+(?P<missing>\d+)\s+lines.*Coverage:\s+(?P<coverage>\d+)%'
    +match = re.search(pattern, report_text, re.DOTALL)
    +if match:
    +    total_lines = int(match.group('total'))
    +    missing_lines = int(match.group('missing'))
    +    coverage_percentage = float(match.group('coverage')) / 100
    +else:
    +    total_lines = missing_lines = 0
    +    coverage_percentage = 0.0
     
    -# Extract missing lines
    -missing_lines_match = re.search(r'Missing:\s+(\d+)\s+lines', report_text)
    -missing_lines = int(missing_lines_match.group(1)) if missing_lines_match else 0
    -
    -coverage_match = re.search(r'Coverage:\s+(\d+)%', report_text)
    -coverage_percentage = float(coverage_match.group(1)) / 100 if coverage_match else 0.0
    -
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion improves the robustness of the code by using a single regular expression with named capture groups. This makes the code more resilient to changes in the report format and reduces the risk of errors when extracting numeric values.

    7
    ✅ Extract the diff coverage command execution logic into a separate method to reduce code duplication and improve maintainability
    Suggestion Impact:The commit refactored the code to remove the duplicated logic for running the diff coverage command, integrating it into the run_coverage method. This aligns with the suggestion to reduce code duplication and improve maintainability.

    code diff:

    +        # Step 2: Determine if diff coverage or full coverage processing is needed
    +        if self.diff_coverage:
    +            # Run the diff-cover command to generate a JSON diff coverage report
    +            coverage_filename = os.path.basename(self.code_coverage_report_path)
    +            coverage_command = f"diff-cover --json-report diff-cover-report.json --compare-branch={self.comparasion_branch} {coverage_filename}"
    +            report_file_path = "diff-cover-report.json"
    +            
    +            # Log and execute the diff coverage command
    +            self.logger.info(f'Running diff coverage command: "{coverage_command}"')
    +            stdout, stderr, exit_code, _ = Runner.run_command(
    +                command=coverage_command, cwd=self.test_command_dir
    +            )
    +
    +            # Ensure the diff command executed successfully
    +            assert exit_code == 0, (
    +                f'Fatal: Error running diff coverage command. Are you sure the command is correct? "{coverage_command}"'
    +                f"\nExit code {exit_code}. \nStdout: \n{stdout} \nStderr: \n{stderr}"
    +            )
    +        else:
    +            # Use the standard coverage report path
    +            report_file_path = self.code_coverage_report_path

    The validate_test method contains duplicated code for running the diff coverage
    command. Consider extracting this logic into a separate method to improve code
    maintainability and reduce duplication.

    cover_agent/UnitTestGenerator.py [603-614]

     if self.diff_coverage:
    -    report_path = self.code_coverage_report_path.replace(self.test_command_dir, "")
    -    report_path = report_path.lstrip("/")
    -    test_command = f"diff-cover --compare-branch={self.comparasion_branch} {report_path}"
    -    self.logger.info(
    -        f'Running diff coverage command to generate diff coverage report: "{test_command}"'
    -    )
    -    stdout, stderr, exit_code, time_of_test_command = Runner.run_command(
    -        command=test_command, cwd=self.test_command_dir
    -    )
    +    exit_code = self._run_diff_coverage_command()
         if exit_code != 0:
             break
     
    +# Add this method to the class:
    +def _run_diff_coverage_command(self):
    +    report_path = self.code_coverage_report_path.replace(self.test_command_dir, "").lstrip("/")
    +    test_command = f"diff-cover --compare-branch={self.comparasion_branch} {report_path}"
    +    self.logger.info(f'Running diff coverage command to generate diff coverage report: "{test_command}"')
    +    stdout, stderr, exit_code, _ = Runner.run_command(command=test_command, cwd=self.test_command_dir)
    +    return exit_code
    +
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: The suggestion enhances code maintainability by reducing duplication. Extracting the diff coverage command execution into a separate method makes the code cleaner and easier to manage, although the impact on functionality is minimal.

    6

    💡 Need additional feedback ? start a PR chat

    @liyuyun-lyy
    Copy link

    really need this feature👍

    @EmbeddedDevops1 EmbeddedDevops1 force-pushed the 183-generate-new-tests-that-are-focused-on-the-pr-changeset branch from d4d3509 to b966bc6 Compare November 6, 2024 16:42
    @EmbeddedDevops1 EmbeddedDevops1 deleted the 183-generate-new-tests-that-are-focused-on-the-pr-changeset branch November 13, 2024 05:10
    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.

    Generate new tests that are focused on the PR changeset
    2 participants