Skip to content

Conversation

@misrasaurabh1
Copy link
Contributor

@misrasaurabh1 misrasaurabh1 commented Apr 29, 2025

User description

"no:cov-s"
also added profiling to blocked plugins


PR Type

Enhancement


Description

  • Replace no:cov-s flag with no:cov

  • Exclude profiling plugin from benchmarks

  • Add -s option to show stdout output

  • Simplify -o addopts formatting


Changes walkthrough 📝

Relevant files
Enhancement
pytest_new_process_trace_benchmarks.py
Refine pytest argument flags                                                         

codeflash/benchmarking/pytest_new_process_trace_benchmarks.py

  • Updated pytest arg list formatting
  • Swapped out no:cov-s for no:cov
  • Added no:profiling exclusion
  • Included -s for stdout capture
  • +16/-2   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Missing argument

    The "-o" flag is passed without a value, which will cause pytest to error. It needs an option value (e.g., "addopts=") or should be removed if unnecessary.

    "-o",

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Add missing -o argument

    Provide a value for the -o flag to avoid pytest parsing errors due to a missing
    argument. For example, include "addopts=" after "-o" to override any default
    addopts.

    codeflash/benchmarking/pytest_new_process_trace_benchmarks.py [19-32]

     [
         benchmarks_root,
         "--codeflash-trace",
         "-p",
         "no:benchmark",
         "-p",
         "no:codspeed",
         "-p",
         "no:cov",
         "-p",
         "no:profiling",
         "-s",
         "-o",
    +    "addopts=",
     ]
    Suggestion importance[1-10]: 9

    __

    Why: The -o flag in the pytest invocation requires an argument, and without adding "addopts=", pytest will fail to parse the options, making this fix essential for test execution.

    High

    @misrasaurabh1 misrasaurabh1 requested a review from KRRT7 April 29, 2025 07:26
    KRRT7
    KRRT7 previously approved these changes Apr 29, 2025
    @misrasaurabh1 misrasaurabh1 enabled auto-merge April 30, 2025 01:31
    @misrasaurabh1 misrasaurabh1 disabled auto-merge April 30, 2025 02:10
    @misrasaurabh1 misrasaurabh1 merged commit 4e6bdc6 into main Apr 30, 2025
    15 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.

    4 participants