From 1ec829dafff280cfcec34dfc073aea1918d82c37 Mon Sep 17 00:00:00 2001 From: HeshamHM28 Date: Tue, 10 Mar 2026 08:27:10 +0200 Subject: [PATCH] refactor: replace find_gradle_executable and find_maven_executable with find_executable method in Maven and Gradle strategies --- codeflash/languages/java/__init__.py | 4 -- .../languages/java/build_tool_strategy.py | 5 ++ codeflash/languages/java/build_tools.py | 69 +------------------ codeflash/languages/java/gradle_strategy.py | 29 ++++++-- codeflash/languages/java/maven_strategy.py | 32 ++++++--- codeflash/languages/java/test_runner.py | 4 +- tests/test_java_multimodule_deps_install.py | 10 +-- tests/test_java_test_filter_validation.py | 7 +- .../test_java/test_build_tools.py | 32 ++++----- .../test_java/test_instrumentation.py | 4 +- .../test_java/test_run_and_parse.py | 5 +- 11 files changed, 81 insertions(+), 120 deletions(-) diff --git a/codeflash/languages/java/__init__.py b/codeflash/languages/java/__init__.py index 715c9e1ea..5095a4847 100644 --- a/codeflash/languages/java/__init__.py +++ b/codeflash/languages/java/__init__.py @@ -11,8 +11,6 @@ from codeflash.languages.java.build_tools import ( JavaProjectInfo, MavenTestResult, detect_build_tool, - find_gradle_executable, - find_maven_executable, find_source_root, find_test_root, get_project_info, @@ -145,10 +143,8 @@ __all__ = [ "extract_code_context", "extract_function_source", "extract_read_only_context", - "find_gradle_executable", "find_helper_files", "find_helper_functions", - "find_maven_executable", "find_source_root", "find_test_root", "find_tests_for_function", diff --git a/codeflash/languages/java/build_tool_strategy.py b/codeflash/languages/java/build_tool_strategy.py index be2b23814..a121ff06f 100644 --- a/codeflash/languages/java/build_tool_strategy.py +++ b/codeflash/languages/java/build_tool_strategy.py @@ -55,6 +55,11 @@ class BuildToolStrategy(ABC): return None + @abstractmethod + def find_executable(self, build_root: Path) -> str | None: + """Find the build tool executable, searching up parent directories if needed.""" + ... + @abstractmethod def ensure_runtime(self, build_root: Path, test_module: str | None) -> bool: """Install codeflash-runtime JAR and register it as a project dependency.""" diff --git a/codeflash/languages/java/build_tools.py b/codeflash/languages/java/build_tools.py index dd7646e40..8bd91391b 100644 --- a/codeflash/languages/java/build_tools.py +++ b/codeflash/languages/java/build_tools.py @@ -7,11 +7,10 @@ This module provides functionality to detect and work with Java build tools from __future__ import annotations import logging -import shutil import xml.etree.ElementTree as ET from dataclasses import dataclass from enum import Enum -from pathlib import Path +from pathlib import Path # noqa: TC003 — used at runtime logger = logging.getLogger(__name__) @@ -289,72 +288,6 @@ def _get_gradle_project_info(project_root: Path) -> JavaProjectInfo | None: ) -def find_maven_executable(project_root: Path | None = None) -> str | None: - """Find the Maven executable. - - Returns: - Path to mvn executable, or None if not found. - - """ - # Check for Maven wrapper in project root first - if project_root is not None: - mvnw_path = project_root / "mvnw" - if mvnw_path.exists(): - return str(mvnw_path) - mvnw_cmd_path = project_root / "mvnw.cmd" - if mvnw_cmd_path.exists(): - return str(mvnw_cmd_path) - - # Check for Maven wrapper in current directory - if Path("mvnw").exists(): - return "./mvnw" - if Path("mvnw.cmd").exists(): - return "mvnw.cmd" - - # Check system Maven - mvn_path = shutil.which("mvn") - if mvn_path: - return mvn_path - - return None - - -def find_gradle_executable(project_root: Path | None = None) -> str | None: - """Find the Gradle executable. - - Checks for Gradle wrapper in the project root and current directory, - then falls back to system Gradle. - - Args: - project_root: Optional project root directory to search for Gradle wrapper. - - Returns: - Path to gradle executable, or None if not found. - - """ - # Check for Gradle wrapper in project root first - if project_root is not None: - gradlew_path = project_root / "gradlew" - if gradlew_path.exists(): - return str(gradlew_path) - gradlew_bat_path = project_root / "gradlew.bat" - if gradlew_bat_path.exists(): - return str(gradlew_bat_path) - - # Check for Gradle wrapper in current directory - if Path("gradlew").exists(): - return "./gradlew" - if Path("gradlew.bat").exists(): - return "gradlew.bat" - - # Check system Gradle - gradle_path = shutil.which("gradle") - if gradle_path: - return gradle_path - - return None - - def _parse_surefire_reports(surefire_dir: Path) -> tuple[int, int, int, int]: """Parse Surefire XML reports to get test counts. diff --git a/codeflash/languages/java/gradle_strategy.py b/codeflash/languages/java/gradle_strategy.py index 21d0dcf77..8a471cc60 100644 --- a/codeflash/languages/java/gradle_strategy.py +++ b/codeflash/languages/java/gradle_strategy.py @@ -16,7 +16,6 @@ from pathlib import Path from typing import Any from codeflash.languages.java.build_tool_strategy import BuildToolStrategy -from codeflash.languages.java.build_tools import find_gradle_executable _BUILD = "build" @@ -128,6 +127,24 @@ class GradleStrategy(BuildToolStrategy): def name(self) -> str: return "Gradle" + def find_executable(self, build_root: Path) -> str | None: + # Walk up from build_root to find gradlew — for multi-module projects + # the wrapper lives at the repo root, which may be a parent of build_root. + current = build_root.resolve() + while True: + gradlew_path = current / "gradlew" + if gradlew_path.exists(): + return str(gradlew_path) + gradlew_bat_path = current / "gradlew.bat" + if gradlew_bat_path.exists(): + return str(gradlew_bat_path) + parent = current.parent + if parent == current: + break + current = parent + # Fall back to system Gradle + return shutil.which("gradle") + def ensure_runtime(self, build_root: Path, test_module: str | None) -> bool: runtime_jar = self.find_runtime_jar() if runtime_jar is None: @@ -169,7 +186,7 @@ class GradleStrategy(BuildToolStrategy): logger.debug("Multi-module deps already installed for %s:%s, skipping", build_root, test_module) return True - gradle = find_gradle_executable(build_root) + gradle = self.find_executable(build_root) if not gradle: logger.error("Gradle not found — cannot pre-install multi-module dependencies") return False @@ -202,7 +219,7 @@ class GradleStrategy(BuildToolStrategy): ) -> subprocess.CompletedProcess[str]: from codeflash.languages.java.test_runner import _run_cmd_kill_pg_on_timeout - gradle = find_gradle_executable(build_root) + gradle = self.find_executable(build_root) if not gradle: logger.error("Gradle not found") return subprocess.CompletedProcess(args=["gradle"], returncode=-1, stdout="", stderr="Gradle not found") @@ -238,7 +255,7 @@ class GradleStrategy(BuildToolStrategy): ) -> str | None: from codeflash.languages.java.test_runner import _find_junit_console_standalone, _run_cmd_kill_pg_on_timeout - gradle = find_gradle_executable(build_root) + gradle = self.find_executable(build_root) if not gradle: return None @@ -339,7 +356,7 @@ class GradleStrategy(BuildToolStrategy): ) -> subprocess.CompletedProcess[str]: from codeflash.languages.java.test_runner import _build_test_filter, _run_cmd_kill_pg_on_timeout - gradle = find_gradle_executable(build_root) + gradle = self.find_executable(build_root) if not gradle: logger.error("Gradle not found") return subprocess.CompletedProcess(args=["gradle"], returncode=-1, stdout="", stderr="Gradle not found") @@ -610,7 +627,7 @@ class GradleStrategy(BuildToolStrategy): msg = f"Invalid test class name: '{test_class}'. Test names must follow Java identifier rules." raise ValueError(msg) - gradle = find_gradle_executable(project_root) or "gradle" + gradle = self.find_executable(project_root) or "gradle" cmd = [gradle, "test", "--no-daemon"] if test_classes: for cls in test_classes: diff --git a/codeflash/languages/java/maven_strategy.py b/codeflash/languages/java/maven_strategy.py index 47f35ae44..4c117c640 100644 --- a/codeflash/languages/java/maven_strategy.py +++ b/codeflash/languages/java/maven_strategy.py @@ -9,13 +9,13 @@ from __future__ import annotations import logging import os import re +import shutil import subprocess import xml.etree.ElementTree as ET from pathlib import Path from typing import Any from codeflash.languages.java.build_tool_strategy import BuildToolStrategy -from codeflash.languages.java.build_tools import find_maven_executable _TARGET = "target" @@ -55,8 +55,9 @@ def _safe_parse_xml(file_path: Path) -> ET.ElementTree: return ET.ElementTree(root) -def install_codeflash_runtime(project_root: Path, runtime_jar_path: Path) -> bool: - mvn = find_maven_executable() +def install_codeflash_runtime(project_root: Path, runtime_jar_path: Path, mvn: str | None = None) -> bool: + if not mvn: + mvn = shutil.which("mvn") if not mvn: logger.error("Maven not found") return False @@ -314,6 +315,19 @@ class MavenStrategy(BuildToolStrategy): def name(self) -> str: return "Maven" + def find_executable(self, build_root: Path) -> str | None: + mvnw_path = build_root / "mvnw" + if mvnw_path.exists(): + return str(mvnw_path) + mvnw_cmd_path = build_root / "mvnw.cmd" + if mvnw_cmd_path.exists(): + return str(mvnw_cmd_path) + if Path("mvnw").exists(): + return "./mvnw" + if Path("mvnw.cmd").exists(): + return "mvnw.cmd" + return shutil.which("mvn") + def find_runtime_jar(self) -> Path | None: if self._M2_JAR.exists(): return self._M2_JAR @@ -327,7 +341,7 @@ class MavenStrategy(BuildToolStrategy): if not self._M2_JAR.exists(): logger.info("Installing codeflash-runtime JAR to local Maven repository") - if not install_codeflash_runtime(build_root, runtime_jar): + if not install_codeflash_runtime(build_root, runtime_jar, mvn=self.find_executable(build_root)): logger.error("Failed to install codeflash-runtime to local Maven repository") return False @@ -357,7 +371,7 @@ class MavenStrategy(BuildToolStrategy): logger.debug("Multi-module deps already installed for %s:%s, skipping", build_root, test_module) return True - mvn = find_maven_executable() + mvn = self.find_executable(build_root) if not mvn: logger.error("Maven not found — cannot pre-install multi-module dependencies") return False @@ -391,7 +405,7 @@ class MavenStrategy(BuildToolStrategy): ) -> subprocess.CompletedProcess[str]: from codeflash.languages.java.test_runner import _run_cmd_kill_pg_on_timeout - mvn = find_maven_executable() + mvn = self.find_executable(build_root) if not mvn: logger.error("Maven not found") return subprocess.CompletedProcess(args=["mvn"], returncode=-1, stdout="", stderr="Maven not found") @@ -428,7 +442,7 @@ class MavenStrategy(BuildToolStrategy): ) -> str | None: from codeflash.languages.java.test_runner import _find_junit_console_standalone, _run_cmd_kill_pg_on_timeout - mvn = find_maven_executable() + mvn = self.find_executable(build_root) if not mvn: return None @@ -517,7 +531,7 @@ class MavenStrategy(BuildToolStrategy): _validate_test_filter, ) - mvn = find_maven_executable() + mvn = self.find_executable(build_root) if not mvn: logger.error("Maven not found") return subprocess.CompletedProcess(args=["mvn"], returncode=-1, stdout="", stderr="Maven not found") @@ -749,7 +763,7 @@ class MavenStrategy(BuildToolStrategy): msg = f"Invalid test class name: '{test_class}'. Test names must follow Java identifier rules." raise ValueError(msg) - mvn = find_maven_executable(project_root) or "mvn" + mvn = self.find_executable(project_root) or "mvn" cmd = [mvn, "test", "-B"] if test_classes: cmd.append(f"-Dtest={','.join(test_classes)}") diff --git a/codeflash/languages/java/test_runner.py b/codeflash/languages/java/test_runner.py index 924b04f73..5800f8737 100644 --- a/codeflash/languages/java/test_runner.py +++ b/codeflash/languages/java/test_runner.py @@ -767,11 +767,9 @@ def _find_junit_console_standalone() -> Path | None: This JAR contains ConsoleLauncher which is required for direct JVM test execution with JUnit 5. """ - from codeflash.languages.java.build_tools import find_maven_executable - m2_base = Path.home() / ".m2" / "repository" / "org" / "junit" / "platform" / "junit-platform-console-standalone" if not m2_base.exists(): - mvn = find_maven_executable() + mvn = shutil.which("mvn") if mvn: logger.debug("Console standalone not found in cache, downloading via Maven") with contextlib.suppress(subprocess.TimeoutExpired, Exception): diff --git a/tests/test_java_multimodule_deps_install.py b/tests/test_java_multimodule_deps_install.py index cadac5251..bdf8217be 100644 --- a/tests/test_java_multimodule_deps_install.py +++ b/tests/test_java_multimodule_deps_install.py @@ -29,7 +29,7 @@ def test_skipped_for_single_module(strategy): assert len(_multimodule_deps_installed) == 0 -@patch("codeflash.languages.java.maven_strategy.find_maven_executable", return_value="mvn") +@patch.object(MavenStrategy, "find_executable", return_value="mvn") @patch("codeflash.languages.java.test_runner._run_cmd_kill_pg_on_timeout") def test_runs_install_command_with_correct_args(mock_run, mock_mvn, strategy): """Should run mvn install -DskipTests -pl -am with validation skip flags.""" @@ -53,7 +53,7 @@ def test_runs_install_command_with_correct_args(mock_run, mock_mvn, strategy): assert mock_run.call_args[1]["cwd"] == root -@patch("codeflash.languages.java.maven_strategy.find_maven_executable", return_value="mvn") +@patch.object(MavenStrategy, "find_executable", return_value="mvn") @patch("codeflash.languages.java.test_runner._run_cmd_kill_pg_on_timeout") def test_caches_and_does_not_rerun(mock_run, mock_mvn, strategy): """Second call with same (root, module) should be cached — no Maven invocation.""" @@ -68,7 +68,7 @@ def test_caches_and_does_not_rerun(mock_run, mock_mvn, strategy): assert mock_run.call_count == 1 -@patch("codeflash.languages.java.maven_strategy.find_maven_executable", return_value="mvn") +@patch.object(MavenStrategy, "find_executable", return_value="mvn") @patch("codeflash.languages.java.test_runner._run_cmd_kill_pg_on_timeout") def test_different_modules_not_cached(mock_run, mock_mvn, strategy): """Different test modules should each trigger their own install.""" @@ -80,7 +80,7 @@ def test_different_modules_not_cached(mock_run, mock_mvn, strategy): assert mock_run.call_count == 2 -@patch("codeflash.languages.java.maven_strategy.find_maven_executable", return_value="mvn") +@patch.object(MavenStrategy, "find_executable", return_value="mvn") @patch("codeflash.languages.java.test_runner._run_cmd_kill_pg_on_timeout") def test_returns_false_on_maven_failure(mock_run, mock_mvn, strategy): """Non-zero exit code should return False and NOT cache.""" @@ -92,7 +92,7 @@ def test_returns_false_on_maven_failure(mock_run, mock_mvn, strategy): assert len(_multimodule_deps_installed) == 0 -@patch("codeflash.languages.java.maven_strategy.find_maven_executable", return_value=None) +@patch.object(MavenStrategy, "find_executable", return_value=None) def test_returns_false_when_maven_not_found(mock_mvn, strategy): """Should return False if Maven executable is not found.""" result = strategy.install_multi_module_deps(Path("/fake"), "module", {}) diff --git a/tests/test_java_test_filter_validation.py b/tests/test_java_test_filter_validation.py index d1de92f93..4e0544e87 100644 --- a/tests/test_java_test_filter_validation.py +++ b/tests/test_java_test_filter_validation.py @@ -69,9 +69,7 @@ def test_run_tests_via_build_tool_raises_on_empty_filter(): ] ) - with patch("codeflash.languages.java.maven_strategy.find_maven_executable") as mock_maven: - mock_maven.return_value = "mvn" - + with patch.object(MavenStrategy, "find_executable", return_value="mvn"): with pytest.raises(ValueError, match="Test filter is EMPTY"): strategy.run_tests_via_build_tool( project_root, @@ -101,10 +99,9 @@ def test_run_tests_via_build_tool_succeeds_with_valid_filter(): ) with ( - patch("codeflash.languages.java.maven_strategy.find_maven_executable") as mock_maven, + patch.object(MavenStrategy, "find_executable", return_value="mvn"), patch("codeflash.languages.java.test_runner._run_cmd_kill_pg_on_timeout") as mock_run, ): - mock_maven.return_value = "mvn" mock_run.return_value = subprocess.CompletedProcess( args=[], returncode=0, stdout="Tests run: 1, Failures: 0, Errors: 0, Skipped: 0", stderr="" ) diff --git a/tests/test_languages/test_java/test_build_tools.py b/tests/test_languages/test_java/test_build_tools.py index f00ef6189..257e9eceb 100644 --- a/tests/test_languages/test_java/test_build_tools.py +++ b/tests/test_languages/test_java/test_build_tools.py @@ -6,12 +6,11 @@ from pathlib import Path from codeflash.languages.java.build_tools import ( BuildTool, detect_build_tool, - find_maven_executable, find_source_root, find_test_root, get_project_info, ) -from codeflash.languages.java.maven_strategy import add_codeflash_dependency +from codeflash.languages.java.maven_strategy import MavenStrategy, add_codeflash_dependency from codeflash.languages.java.test_runner import _extract_modules_from_pom_content @@ -175,8 +174,8 @@ class TestMavenExecutable: def test_find_maven_executable_system(self): """Test finding system Maven.""" - # This test may pass or fail depending on whether Maven is installed - mvn = find_maven_executable() + strategy = MavenStrategy() + mvn = strategy.find_executable(Path(".")) # We can't assert it exists, just that the function doesn't crash if mvn: assert "mvn" in mvn.lower() or "maven" in mvn.lower() @@ -188,10 +187,8 @@ class TestMavenExecutable: mvnw_path.write_text("#!/bin/bash\necho 'Maven Wrapper'") mvnw_path.chmod(0o755) - # Change to tmp_path - monkeypatch.chdir(tmp_path) - - mvn = find_maven_executable() + strategy = MavenStrategy() + mvn = strategy.find_executable(tmp_path) # Should find the wrapper assert mvn is not None @@ -377,24 +374,27 @@ class TestMavenProfiles: class TestMavenExecutableWithProjectRoot: - """Tests for find_maven_executable with project_root parameter.""" + """Tests for MavenStrategy.find_executable with project_root parameter.""" def test_find_wrapper_in_project_root(self, tmp_path): mvnw_path = tmp_path / "mvnw" mvnw_path.write_text("#!/bin/bash\necho Maven Wrapper") mvnw_path.chmod(0o755) - result = find_maven_executable(project_root=tmp_path) + strategy = MavenStrategy() + result = strategy.find_executable(tmp_path) assert result is not None assert str(tmp_path / "mvnw") in result - def test_fallback_to_cwd_when_no_project_root(self): - result = find_maven_executable() - # Should not crash even without project_root + def test_fallback_to_cwd(self, tmp_path): + strategy = MavenStrategy() + result = strategy.find_executable(tmp_path) + # Should not crash even with a dir that has no wrapper - def test_project_root_none_uses_cwd(self): - result = find_maven_executable(project_root=None) - # Should not crash + def test_with_nonexistent_wrapper(self, tmp_path): + strategy = MavenStrategy() + result = strategy.find_executable(tmp_path) + # Should not crash, may return system mvn or None class TestCustomSourceDirectoryDetection: diff --git a/tests/test_languages/test_java/test_instrumentation.py b/tests/test_languages/test_java/test_instrumentation.py index 1eceb7545..e82194e17 100644 --- a/tests/test_languages/test_java/test_instrumentation.py +++ b/tests/test_languages/test_java/test_instrumentation.py @@ -22,7 +22,7 @@ os.environ["CODEFLASH_API_KEY"] = "cf-test-key" from codeflash.discovery.functions_to_optimize import FunctionToOptimize from codeflash.languages.base import Language from codeflash.languages.current import set_current_language -from codeflash.languages.java.build_tools import find_maven_executable +from codeflash.languages.java.maven_strategy import MavenStrategy from codeflash.languages.java.discovery import discover_functions_from_source from codeflash.languages.java.instrumentation import ( _add_behavior_instrumentation, @@ -1968,7 +1968,7 @@ public class AccentTest { # Skip all E2E tests if Maven is not available requires_maven = pytest.mark.skipif( - find_maven_executable() is None, reason="Maven not found - skipping execution tests" + MavenStrategy().find_executable(Path(".")) is None, reason="Maven not found - skipping execution tests" ) diff --git a/tests/test_languages/test_java/test_run_and_parse.py b/tests/test_languages/test_java/test_run_and_parse.py index 747b9031a..7d093dbb3 100644 --- a/tests/test_languages/test_java/test_run_and_parse.py +++ b/tests/test_languages/test_java/test_run_and_parse.py @@ -90,9 +90,10 @@ POM_CONTENT = """ def skip_if_maven_not_available(): - from codeflash.languages.java.build_tools import find_maven_executable + from codeflash.languages.java.maven_strategy import MavenStrategy + + if not MavenStrategy().find_executable(Path(".")): - if not find_maven_executable(): pytest.skip("Maven not available")