Skip to content

Conversation

@mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Oct 9, 2025

PR Type

Tests, Enhancement


Description

  • Add E2E test using git worktree

  • Extend TestConfig with worktree flag

  • Pass --worktree to CLI when enabled

  • Update CI to run worktree E2E


Diagram Walkthrough

flowchart LR A["TestConfig (use_worktree)"] -- "propagates" --> B["build_command adds --worktree"] C["New E2E script (worktree)"] -- "uses TestConfig" --> A D["GitHub Workflow"] -- "runs worktree E2E" --> C 
Loading

File Walkthrough

Relevant files
Tests
end_to_end_test_topological_sort.py
Remove legacy topological sort E2E                                             

tests/scripts/end_to_end_test_topological_sort.py

  • Remove non-worktree E2E script
+0/-29   
end_to_end_test_topological_sort_worktree.py
Add worktree-enabled E2E test script                                         

tests/scripts/end_to_end_test_topological_sort_worktree.py

  • Add E2E test enabling worktree
  • Configure coverage expectations
  • Invoke runner with expected improvement
+28/-0   
Enhancement
end_to_end_test_utilities.py
Support worktree flag in test utilities                                   

tests/scripts/end_to_end_test_utilities.py

  • Add use_worktree to TestConfig
  • Append --worktree CLI flag when set
+3/-0     
Configuration changes
e2e-topological-sort.yaml
Update CI to run worktree E2E                                                       

.github/workflows/e2e-topological-sort.yaml

  • Rename workflow and job for worktree
  • Execute new worktree E2E script
+3/-3     

@github-actions github-actions bot added workflow-modified This PR modifies GitHub Actions workflows Review effort 2/5 labels Oct 9, 2025
@github-actions
Copy link

github-actions bot commented Oct 9, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

CLI Flag Consistency

Ensure the CLI actually supports the new '--worktree' flag and that its behavior is covered; otherwise runs may silently ignore or fail on unknown flag. Validate downstream handling and docs.

if config.use_worktree: base_command.append("--worktree")
Hardcoded Paths

The test uses hardcoded function and file paths; confirm they are stable in CI. Consider deriving from a shared fixture or constant to avoid breakage if the sample code moves.

config = TestConfig( file_path="topological_sort.py", function_name="Graph.topologicalSort", test_framework="pytest", min_improvement_x=0.05, use_worktree=True, coverage_expectations=[ CoverageExpectation( function_name="Graph.topologicalSort", expected_coverage=100.0, expected_lines=[25, 26, 27, 28, 29, 30, 31], ) ], ) cwd = (pathlib.Path(__file__).parent.parent.parent / "code_to_optimize").resolve() return_var = run_codeflash_command(cwd, config, expected_improvement_pct)
@github-actions
Copy link

github-actions bot commented Oct 9, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Make path resolution robust

Use file via pathlib.Path(file) only when the file is run as a script to
avoid issues in environments where file may not be set (e.g., some test
runners). Fall back to Path.cwd() to ensure a valid path. This prevents crashes when
resolving cwd.

tests/scripts/end_to_end_test_topological_sort_worktree.py [22]

-cwd = (pathlib.Path(__file__).parent.parent.parent / "code_to_optimize").resolve() +try: + base_dir = pathlib.Path(__file__).parent.parent.parent +except NameError: + base_dir = pathlib.Path.cwd() +cwd = (base_dir / "code_to_optimize").resolve()
Suggestion importance[1-10]: 6

__

Why: The fallback to Path.cwd() can prevent failures where __file__ is unset, improving robustness with minimal risk; it's accurate for the shown cwd line but not critical.

Low
General
Gate new CLI flag via env

Only append the --worktree flag when the underlying codeflash command supports it to
avoid failures on older versions. Guard with an environment toggle so CI can disable
it when unsupported. This reduces hard failures due to unknown CLI flags.

tests/scripts/end_to_end_test_utilities.py [141-142]

-if config.use_worktree: +if config.use_worktree and os.environ.get("CF_ENABLE_WORKTREE", "1") == "1": base_command.append("--worktree")
Suggestion importance[1-10]: 5

__

Why: Adding an env guard reduces failures on older CLIs; however, it introduces a behavior toggle not present in the PR and requires importing os, making it a moderate-impact, optional safeguard.

Low
Set env flag in workflow

Ensure the workflow sets the environment toggle expected by tests when using
worktree mode to avoid mismatches. Export an env var before running to explicitly
enable worktree behavior. This prevents silent divergence across environments.

.github/workflows/e2e-topological-sort.yaml [91-93]

 - name: Run Codeflash to optimize code id: optimize_code run: | + export CF_ENABLE_WORKTREE=1 uv run python tests/scripts/end_to_end_test_topological_sort_worktree.py
Suggestion importance[1-10]: 5

__

Why: Ensuring an env var is set in the workflow can coordinate with the proposed guard, but it's contingent on adopting that guard and is not necessary for current PR behavior.

Low
@mohammedahmed18 mohammedahmed18 requested a review from KRRT7 October 9, 2025 12:27
@mohammedahmed18 mohammedahmed18 merged commit 925075a into main Oct 9, 2025
20 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

4 participants