Skip to content

Conversation

@aseembits93
Copy link
Contributor

@aseembits93 aseembits93 commented May 29, 2025

PR Type

Enhancement, Tests


Description

Introduce context manager to trim pytest addopts
Wrap all test subprocess calls with stripped addopts
Temporarily modify and restore pyproject.toml
Minor echo formatting tweak in init command


Changes walkthrough 📝

Relevant files
Formatting
cmd_init.py
Fix echo prompt formatting                                                             

codeflash/cli_cmds/cmd_init.py

  • Fixed f-string spacing in GitHub secrets prompt
  • Removed ellipsis from the secrets URL message
  • +1/-1     
    Enhancement
    code_utils.py
    Add custom_addopts context manager                                             

    codeflash/code_utils/code_utils.py

  • Added custom_addopts context manager
  • Parses and modifies pyproject.toml via tomlkit
  • Removes blacklisted addopts (-n, -n auto, auto)
  • Backs up and restores original file content
  • +43/-0   
    Tests
    test_runner.py
    Wrap test execution with custom_addopts                                   

    codeflash/verification/test_runner.py

  • Imported custom_addopts from code_utils
  • Wrapped all execute_test_subprocess calls with custom_addopts
  • Ensures blacklisted pytest plugins are bypassed
  • +34/-30 

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @aseembits93 aseembits93 requested review from KRRT7 and misrasaurabh1 and removed request for misrasaurabh1 May 29, 2025 00:38
    @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Addopts Parsing

    Filtering logic on original_addopts treats it as an iterable of characters rather than splitting into individual CLI arguments. Ensure addopts are parsed into a list of strings and re-serialized correctly to avoid dropping or mangling flags.

    data["tool"]["pytest"]["ini_options"]["addopts"] = [
        x for x in original_addopts if x not in ["-n", "-n auto", "auto"]
    ]
    
    # Write modified file
    with Path.open(pyproject_file, "w", encoding="utf-8") as f:
        f.write(tomlkit.dumps(data))
    Prompt Formatting

    The new f-string in click.prompt adds a space and removes the ellipsis; verify it renders correctly across platforms and handles the case when existing_api_key is None without producing awkward spacing.

    f"Press Enter to open your repo's secrets page at {get_github_secrets_page_url(repo)} {LF}"

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Correctly filter addopts entries

    The list comprehension is iterating over characters if original_addopts is a string.
    Split the string into arguments before filtering and then reassemble into a
    space‐separated string to keep the correct format.

    codeflash/code_utils/code_utils.py [40-42]

    -data["tool"]["pytest"]["ini_options"]["addopts"] = [
    -    x for x in original_addopts if x not in ["-n", "-n auto", "auto"]
    -]
    +original_list = original_addopts.split() if isinstance(original_addopts, str) else original_addopts
    +filtered_addopts = [arg for arg in original_list if arg not in ["-n", "-n auto", "auto"]]
    +data["tool"]["pytest"]["ini_options"]["addopts"] = " ".join(filtered_addopts)
    Suggestion importance[1-10]: 7

    __

    Why: The current list comprehension will iterate characters when original_addopts is a string, leading to incorrect filtering; splitting into arguments before filtering ensures full CLI options are handled correctly.

    Medium
    General
    Target the correct pyproject file

    The context manager defaults to modifying pyproject.toml in the current working dir
    instead of the test project. Pass the actual test cwd path so you modify the correct
    file under test.

    codeflash/verification/test_runner.py [101-107]

    -with custom_addopts():
    +with custom_addopts(pyproject_path=(cwd / "pyproject.toml").as_posix()):
         results = execute_test_subprocess(
             coverage_cmd + common_pytest_args + blocklist_args + result_args + test_files,
             cwd=cwd,
             env=pytest_test_env,
             timeout=600,
         )
    Suggestion importance[1-10]: 6

    __

    Why: By passing pyproject_path=(cwd/"pyproject.toml"), custom_addopts will modify the test project's config instead of the default file in the current working directory.

    Low

    @github-actions github-actions bot added the workflow-modified This PR modifies GitHub Actions workflows label May 29, 2025
    @aseembits93
    Copy link
    Contributor Author

    @misrasaurabh1 @KRRT7 let me know if having an e2e test is a good idea, otherwise I can write some unit tests

    @aseembits93 aseembits93 changed the title Edit pyproject temporarlily to remove blacklisted plugin parameters Edit pyproject temporarily to remove blacklisted plugin parameters May 29, 2025
    @aseembits93 aseembits93 requested a review from KRRT7 May 29, 2025 18:20
    @openhands-ai
    Copy link

    openhands-ai bot commented May 29, 2025

    Looks like there are a few issues preventing this PR from being merged!

    • GitHub Actions are failing:
      • Lint
      • end-to-end-test

    If you'd like me to help, just leave a comment, like

    @OpenHands please fix the failing actions on PR #250

    Feel free to include any additional details that might help me get this PR into a better state.

    You can manage your notification settings

    @aseembits93 aseembits93 merged commit 2ed293e into main May 29, 2025
    16 checks passed
    @aseembits93 aseembits93 deleted the bypass-addopts branch May 29, 2025 20:14
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    Review effort 3/5 workflow-modified This PR modifies GitHub Actions workflows

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    4 participants