fix: suppress Error Prone CheckReturnValue in instrumented tests and fix stale pom dependency

codeflash/languages/java/build_tools.py

@@ -8,6 +8,7 @@
 
 import logging
 import os
+import re
 import shutil
 import subprocess
 import xml.etree.ElementTree as ET
@@ -645,6 +646,31 @@ def add_codeflash_dependency_to_pom(pom_path: Path) -> bool:
 
         # Check if already present
         if "codeflash-runtime" in content:
+            # If a previous run left a system-scope dependency, replace it with test scope.
+            # System-scope dependencies cause Maven warnings and are rejected by some projects.
+            if "<scope>system</scope>" in content:
+                # Replace ONLY the codeflash-runtime dependency block that has system scope.
+                # We find each <dependency>...</dependency> block individually and only replace
+                # the one containing both "codeflash-runtime" and "<scope>system</scope>".
+                # The previous regex used [\s\S]*? lookaheads that could match across blocks,
+                # accidentally replacing every dependency in the file.
+                def replace_system_dep(match: re.Match) -> str:
+                    block = match.group(0)
+                    if "codeflash-runtime" in block and "<scope>system</scope>" in block:
+                        return (
+                            "<dependency>\n"
+                            "            <groupId>com.codeflash</groupId>\n"
+                            "            <artifactId>codeflash-runtime</artifactId>\n"
+                            "            <version>1.0.0</version>\n"
+                            "            <scope>test</scope>\n"
+                            "        </dependency>"
+                        )
+                    return block
+
+                content = re.sub(r"<dependency>[\s\S]*?</dependency>", replace_system_dep, content)
+                pom_path.write_text(content, encoding="utf-8")
+                logger.info("Replaced system-scope codeflash-runtime dependency with test scope")
+                return True
             logger.info("codeflash-runtime dependency already present in pom.xml")
             return True
 

codeflash/languages/java/instrumentation.py

@@ -638,6 +638,12 @@ def instrument_existing_test(
     # replacing substrings of other identifiers.
     modified_source = re.sub(rf"\b{re.escape(original_class_name)}\b", new_class_name, source)
 
+    # Add @SuppressWarnings("CheckReturnValue") to the class declaration.
+    # Projects using Error Prone (e.g. Guava) enforce CheckReturnValue as a compiler error.
+    # Applied in both modes: performance mode strips assertions (creating discarded return values),
+    # and behavior mode adds wrapper calls that may also discard return values.
+    modified_source = _add_suppress_warnings_annotation(modified_source, new_class_name)
+
     # Add timing instrumentation to test methods
     # Use original class name (without suffix) in timing markers for consistency with Python
     if mode == "performance":
@@ -828,6 +834,23 @@ def _add_behavior_instrumentation(source: str, class_name: str, func_name: str)
     return "\n".join(result)
 
 
+def _add_suppress_warnings_annotation(source: str, class_name: str) -> str:
+    """Add @SuppressWarnings("CheckReturnValue") before the class declaration.
+
+    Projects using Error Prone (e.g. Guava) enforce CheckReturnValue as a compiler error.
+    Our instrumented tests intentionally discard return values after assertion stripping,
+    which would fail compilation without this suppression.
+    """
+    class_decl_pattern = re.compile(
+        rf"^((?:(?:public|protected|final|abstract)\s+)*class\s+{re.escape(class_name)}\b)", re.MULTILINE
+    )
+    match = class_decl_pattern.search(source)
+    if not match:
+        return source
+    insert_pos = match.start()
+    return source[:insert_pos] + '@SuppressWarnings("CheckReturnValue")\n' + source[insert_pos:]
+
+
 def _add_timing_instrumentation(source: str, class_name: str, func_name: str) -> str:
     """Add timing instrumentation to test methods with inner loop for JIT warmup.
 
@@ -1307,6 +1330,9 @@ def instrument_generated_java_test(
         # This includes the class declaration, return types, constructor calls, etc.
         modified_code = re.sub(rf"\b{re.escape(original_class_name)}\b", new_class_name, test_code)
 
+        # Suppress Error Prone's CheckReturnValue for generated performance tests
+        modified_code = _add_suppress_warnings_annotation(modified_code, new_class_name)
+
         modified_code = _add_timing_instrumentation(
             modified_code,
             original_class_name,  # Use original name in markers, not the renamed class

codeflash/languages/java/test_runner.py

@@ -43,6 +43,10 @@
 # so we avoid calling `mvn dependency:build-classpath` (~2-3s) repeatedly.
 _classpath_cache: dict[tuple[Path, str | None], str] = {}
 
+# Cache for multi-module dependency installs — keyed on (maven_root, test_module).
+# After pre-installing deps to .m2 once, subsequent Maven invocations can skip -am.
+_multimodule_deps_installed: set[tuple[Path, str]] = set()
+
 # Regex pattern for valid Java class names (package.ClassName format)
 # Allows: letters, digits, underscores, dots, and dollar signs (inner classes)
 _VALID_JAVA_CLASS_NAME = re.compile(r"^[a-zA-Z_$][a-zA-Z0-9_$.]*$")
@@ -251,6 +255,68 @@ def _ensure_codeflash_runtime(maven_root: Path, test_module: str | None) -> bool
     return True
 
 
+def ensure_multi_module_deps_installed(maven_root: Path, test_module: str | None, env: dict[str, str]) -> bool:
+    """Pre-install multi-module dependencies to the local Maven repository.
+
+    In multi-module Maven projects (like Guava), Maven compiler plugin 3.15.0's
+    JDK-8318913 workaround patches module-info.class timestamps after compilation.
+    When a subsequent Maven invocation uses -am (also-make), the compiler detects
+    "changed source code" and recompiles dependency modules — which fails because
+    module-path resolution doesn't work in a partial reactor rebuild.
+
+    This function runs `mvn install -DskipTests -pl <module> -am` once to put all
+    dependency JARs into ~/.m2.  After that, test-running commands can use
+    `-pl <module>` without `-am`, resolving deps from .m2 instead.
+
+    Skipped for single-module projects (test_module is None) and cached so it only
+    runs once per (maven_root, test_module) pair within a session.
+    """
+    if not test_module:
+        return True
+
+    cache_key = (maven_root, test_module)
+    if cache_key in _multimodule_deps_installed:
+        logger.debug("Multi-module deps already installed for %s:%s, skipping", maven_root, test_module)
+        return True
+
+    mvn = find_maven_executable()
+    if not mvn:
+        logger.error("Maven not found — cannot pre-install multi-module dependencies")
+        return False
+
+    cmd = [
+        mvn,
+        "install",
+        "-DskipTests",
+        "-B",
+        "-pl",
+        test_module,
+        "-am",
+    ]
+    cmd.extend(_MAVEN_VALIDATION_SKIP_FLAGS)
+
+    logger.info("Pre-installing multi-module dependencies: %s (module: %s)", maven_root, test_module)
+    logger.debug("Running: %s", " ".join(cmd))
+
+    try:
+        result = _run_cmd_kill_pg_on_timeout(cmd, cwd=maven_root, env=env, timeout=300)
+        if result.returncode != 0:
+            logger.error(
+                "Failed to pre-install multi-module deps (exit %d).\nstdout: %s\nstderr: %s",
+                result.returncode,
+                result.stdout[-2000:] if result.stdout else "",
+                result.stderr[-2000:] if result.stderr else "",
+            )
+            return False
+    except Exception:
+        logger.exception("Exception during multi-module dependency install")
+        return False
+
+    _multimodule_deps_installed.add(cache_key)
+    logger.info("Multi-module dependencies installed successfully for %s:%s", maven_root, test_module)
+    return True
+
+
 def _extract_modules_from_pom_content(content: str) -> list[str]:
     """Extract module names from Maven POM XML content using proper XML parsing.
 
@@ -485,6 +551,11 @@ def run_behavioral_tests(
     # Ensure codeflash-runtime is installed and added as dependency before compilation
     _ensure_codeflash_runtime(maven_root, test_module)
 
+    # Pre-install multi-module deps to .m2 so subsequent Maven runs don't need -am
+    base_env = os.environ.copy()
+    base_env.update(test_env)
+    ensure_multi_module_deps_installed(maven_root, test_module, base_env)
+
     # Create SQLite database path for behavior capture - use standard path that parse_test_results expects
     sqlite_db_path = get_run_tmp_file(Path(f"test_return_values_{candidate_index}.sqlite"))
 
@@ -604,7 +675,7 @@ def _compile_tests(
     cmd.extend(_MAVEN_VALIDATION_SKIP_FLAGS)
 
     if test_module:
-        cmd.extend(["-pl", test_module, "-am"])
+        cmd.extend(["-pl", test_module])
 
     logger.debug("Compiling tests: %s in %s", " ".join(cmd), project_root)
 
@@ -1186,6 +1257,11 @@ def run_benchmarking_tests(
     # Ensure codeflash-runtime is installed and added as dependency before compilation
     _ensure_codeflash_runtime(maven_root, test_module)
 
+    # Pre-install multi-module deps to .m2 so subsequent Maven runs don't need -am
+    base_env = os.environ.copy()
+    base_env.update(test_env)
+    ensure_multi_module_deps_installed(maven_root, test_module, base_env)
+
     # Get test class names
     test_classes = _get_test_class_names(test_paths, mode="performance")
     if not test_classes:
@@ -1569,16 +1645,15 @@ def _run_maven_tests(
     if enable_coverage:
         cmd.append("-Dmaven.test.failure.ignore=true")
 
-    # For multi-module projects, specify which module to test
+    # For multi-module projects, specify which module to test.
+    # Dependencies are pre-installed to .m2 by ensure_multi_module_deps_installed(),
+    # so we use -pl without -am to avoid recompiling dependency modules (which fails
+    # on projects like Guava due to Maven compiler plugin's JDK-8318913 workaround).
     if test_module:
-        # -am = also make dependencies
-        # -DfailIfNoTests=false allows dependency modules without tests to pass
-        # -DskipTests=false overrides any skipTests=true in pom.xml
         cmd.extend(
             [
                 "-pl",
                 test_module,
-                "-am",
                 "-DfailIfNoTests=false",
                 "-Dsurefire.failIfNoSpecifiedTests=false",
                 "-DskipTests=false",
@@ -2019,6 +2094,11 @@ def run_line_profile_tests(
     # Ensure codeflash-runtime is installed and added as dependency before compilation
     _ensure_codeflash_runtime(maven_root, test_module)
 
+    # Pre-install multi-module deps to .m2 so subsequent Maven runs don't need -am
+    base_env = os.environ.copy()
+    base_env.update(test_env)
+    ensure_multi_module_deps_installed(maven_root, test_module, base_env)
+
     # Set up environment with profiling mode
     run_env = os.environ.copy()
     run_env.update(test_env)

tests/test_java_multimodule_deps_install.py

@@ -0,0 +1,102 @@
+"""Tests for ensure_multi_module_deps_installed in Java test runner."""
+
+import subprocess
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from codeflash.languages.java.test_runner import (
+    _multimodule_deps_installed,
+    ensure_multi_module_deps_installed,
+)
+
+
+@pytest.fixture(autouse=True)
+def clear_cache():
+    """Clear the multi-module deps cache before each test."""
+    _multimodule_deps_installed.clear()
+    yield
+    _multimodule_deps_installed.clear()
+
+
+def test_skipped_for_single_module():
+    """Single-module projects (test_module=None) should be a no-op."""
+    result = ensure_multi_module_deps_installed(Path("/fake"), None, {})
+    assert result is True
+    assert len(_multimodule_deps_installed) == 0
+
+
+@patch("codeflash.languages.java.test_runner.find_maven_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):
+    """Should run mvn install -DskipTests -pl <module> -am with validation skip flags."""
+    mock_run.return_value = subprocess.CompletedProcess(args=["mvn"], returncode=0, stdout="", stderr="")
+
+    root = Path("/project")
+    result = ensure_multi_module_deps_installed(root, "guava-tests", {"JAVA_HOME": "/jdk"})
+
+    assert result is True
+    mock_run.assert_called_once()
+    cmd = mock_run.call_args[0][0]
+    assert cmd[0] == "mvn"
+    assert "install" in cmd
+    assert "-DskipTests" in cmd
+    assert "-pl" in cmd
+    assert "guava-tests" in cmd
+    assert "-am" in cmd
+    assert "-B" in cmd
+    # Validation skip flags should be present
+    assert "-Drat.skip=true" in cmd
+    assert "-Dcheckstyle.skip=true" in cmd
+    # cwd should be maven_root
+    assert mock_run.call_args[1]["cwd"] == root
+
+
+@patch("codeflash.languages.java.test_runner.find_maven_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):
+    """Second call with same (root, module) should be cached — no Maven invocation."""
+    mock_run.return_value = subprocess.CompletedProcess(args=["mvn"], returncode=0, stdout="", stderr="")
+
+    root = Path("/project")
+    ensure_multi_module_deps_installed(root, "guava-tests", {})
+    assert mock_run.call_count == 1
+
+    # Second call — should be cached
+    result = ensure_multi_module_deps_installed(root, "guava-tests", {})
+    assert result is True
+    assert mock_run.call_count == 1  # NOT called again
+
+
+@patch("codeflash.languages.java.test_runner.find_maven_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):
+    """Different test modules should each trigger their own install."""
+    mock_run.return_value = subprocess.CompletedProcess(args=["mvn"], returncode=0, stdout="", stderr="")
+
+    root = Path("/project")
+    ensure_multi_module_deps_installed(root, "module-a", {})
+    ensure_multi_module_deps_installed(root, "module-b", {})
+    assert mock_run.call_count == 2
+
+
+@patch("codeflash.languages.java.test_runner.find_maven_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):
+    """Non-zero exit code should return False and NOT cache."""
+    mock_run.return_value = subprocess.CompletedProcess(
+        args=["mvn"], returncode=1, stdout="", stderr="BUILD FAILURE"
+    )
+
+    root = Path("/project")
+    result = ensure_multi_module_deps_installed(root, "guava-tests", {})
+    assert result is False
+    assert len(_multimodule_deps_installed) == 0
+
+
+@patch("codeflash.languages.java.test_runner.find_maven_executable", return_value=None)
+def test_returns_false_when_maven_not_found(mock_mvn):
+    """Should return False if Maven executable is not found."""
+    result = ensure_multi_module_deps_installed(Path("/fake"), "module", {})
+    assert result is False