Skip to content

Commit f67057d

Browse files
misrasaurabh1claude
andcommitted
fix: improve Java test file handling and JaCoCo coverage for multi-module projects
- Fix duplicate test file issue: when multiple tests have the same class name, append unique index suffix (e.g., CryptoTest_2) to avoid file overwrites - Fix multi-module JaCoCo support: add JaCoCo plugin to test module's pom.xml instead of source module, ensuring coverage data is collected where tests run - Fix timeout: use minimum 60s (120s with coverage) for Java builds since Maven takes longer than the default 15s INDIVIDUAL_TESTCASE_TIMEOUT - Fix Maven phase: use 'verify' instead of 'test' when coverage is enabled, with maven.test.failure.ignore=true to generate report even if tests fail - Update JaCoCo report phase from 'test' to 'verify' to run after tests complete Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 1858044 commit f67057d

3 files changed

Lines changed: 85 additions & 18 deletions

File tree

codeflash/languages/java/build_tools.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,7 @@ def add_jacoco_plugin_to_pom(pom_path: Path) -> bool:
720720
</execution>
721721
<execution>
722722
<id>report</id>
723-
<phase>test</phase>
723+
<phase>verify</phase>
724724
<goals>
725725
<goal>report</goal>
726726
</goals>

codeflash/languages/java/test_runner.py

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -183,22 +183,36 @@ def run_behavioral_tests(
183183
run_env["CODEFLASH_OUTPUT_FILE"] = str(sqlite_db_path) # SQLite output path
184184

185185
# If coverage is enabled, ensure JaCoCo is configured
186-
# For multi-module projects, add JaCoCo to the source module (project_root), not the test module
186+
# For multi-module projects, add JaCoCo to the test module's pom.xml (where tests run)
187187
coverage_xml_path: Path | None = None
188188
if enable_coverage:
189-
pom_path = project_root / "pom.xml"
190-
if pom_path.exists():
191-
if not is_jacoco_configured(pom_path):
192-
logger.info("Adding JaCoCo plugin to pom.xml for coverage collection")
193-
add_jacoco_plugin_to_pom(pom_path)
194-
coverage_xml_path = get_jacoco_xml_path(project_root)
189+
# Determine which pom.xml to configure JaCoCo in
190+
if test_module:
191+
# Multi-module project: add JaCoCo to test module
192+
test_module_pom = maven_root / test_module / "pom.xml"
193+
if test_module_pom.exists():
194+
if not is_jacoco_configured(test_module_pom):
195+
logger.info(f"Adding JaCoCo plugin to test module pom.xml: {test_module_pom}")
196+
add_jacoco_plugin_to_pom(test_module_pom)
197+
coverage_xml_path = get_jacoco_xml_path(maven_root / test_module)
198+
else:
199+
# Single module project
200+
pom_path = project_root / "pom.xml"
201+
if pom_path.exists():
202+
if not is_jacoco_configured(pom_path):
203+
logger.info("Adding JaCoCo plugin to pom.xml for coverage collection")
204+
add_jacoco_plugin_to_pom(pom_path)
205+
coverage_xml_path = get_jacoco_xml_path(project_root)
195206

196207
# Run Maven tests from the appropriate root
208+
# Use a minimum timeout of 60s for Java builds (120s when coverage is enabled due to verify phase)
209+
min_timeout = 120 if enable_coverage else 60
210+
effective_timeout = max(timeout or 300, min_timeout)
197211
result = _run_maven_tests(
198212
maven_root,
199213
test_paths,
200214
run_env,
201-
timeout=timeout or 300,
215+
timeout=effective_timeout,
202216
mode="behavior",
203217
enable_coverage=enable_coverage,
204218
test_module=test_module,
@@ -464,9 +478,14 @@ def _run_maven_tests(
464478
test_filter = _build_test_filter(test_paths, mode=mode)
465479

466480
# Build Maven command
467-
# Note: JaCoCo report is generated automatically during test phase via plugin execution binding
468-
# We don't need to call jacoco:report explicitly since the plugin config binds it to test phase
469-
cmd = [mvn, "test", "-fae"] # Fail at end to run all tests
481+
# When coverage is enabled, use 'verify' phase to ensure JaCoCo report runs after tests
482+
# JaCoCo's report goal is bound to the verify phase to get post-test execution data
483+
maven_goal = "verify" if enable_coverage else "test"
484+
cmd = [mvn, maven_goal, "-fae"] # Fail at end to run all tests
485+
486+
# When coverage is enabled, continue build even if tests fail so JaCoCo report is generated
487+
if enable_coverage:
488+
cmd.append("-Dmaven.test.failure.ignore=true")
470489

471490
# For multi-module projects, specify which module to test
472491
if test_module:

codeflash/optimization/function_optimizer.py

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -576,18 +576,23 @@ def generate_and_instrument_tests(
576576
generated_tests = normalize_generated_tests_imports(generated_tests)
577577

578578
logger.debug(f"[PIPELINE] Processing {count_tests} generated tests")
579+
used_behavior_paths: set[Path] = set()
579580
for i, generated_test in enumerate(generated_tests.generated_tests):
580581
behavior_path = generated_test.behavior_file_path
581582
perf_path = generated_test.perf_file_path
582583

583584
# For Java, fix paths to match package structure
584585
if is_java():
585-
behavior_path, perf_path = self._fix_java_test_paths(
586+
behavior_path, perf_path, modified_behavior_source, modified_perf_source = self._fix_java_test_paths(
586587
generated_test.instrumented_behavior_test_source,
587588
generated_test.instrumented_perf_test_source,
589+
used_behavior_paths,
588590
)
589591
generated_test.behavior_file_path = behavior_path
590592
generated_test.perf_file_path = perf_path
593+
generated_test.instrumented_behavior_test_source = modified_behavior_source
594+
generated_test.instrumented_perf_test_source = modified_perf_source
595+
used_behavior_paths.add(behavior_path)
591596

592597
logger.debug(
593598
f"[PIPELINE] Test {i + 1}: behavior_path={behavior_path}, perf_path={perf_path}"
@@ -653,20 +658,23 @@ def generate_and_instrument_tests(
653658
)
654659

655660
def _fix_java_test_paths(
656-
self, behavior_source: str, perf_source: str
657-
) -> tuple[Path, Path]:
661+
self, behavior_source: str, perf_source: str, used_paths: set[Path]
662+
) -> tuple[Path, Path, str, str]:
658663
"""Fix Java test file paths to match package structure.
659664
660665
Java requires test files to be in directories matching their package.
661666
This method extracts the package and class from the generated tests
662-
and returns correct paths.
667+
and returns correct paths. If the path would conflict with an already
668+
used path, it renames the class by adding an index suffix.
663669
664670
Args:
665671
behavior_source: Source code of the behavior test.
666672
perf_source: Source code of the performance test.
673+
used_paths: Set of already used behavior file paths.
667674
668675
Returns:
669-
Tuple of (behavior_path, perf_path) with correct package structure.
676+
Tuple of (behavior_path, perf_path, modified_behavior_source, modified_perf_source)
677+
with correct package structure and unique class names.
670678
671679
"""
672680
import re
@@ -692,15 +700,55 @@ def _fix_java_test_paths(
692700
behavior_path = test_dir / package_path / f"{behavior_class}.java"
693701
perf_path = test_dir / package_path / f"{perf_class}.java"
694702
else:
703+
package_path = ""
695704
behavior_path = test_dir / f"{behavior_class}.java"
696705
perf_path = test_dir / f"{perf_class}.java"
697706

707+
# If path already used, rename class by adding index suffix
708+
modified_behavior_source = behavior_source
709+
modified_perf_source = perf_source
710+
if behavior_path in used_paths:
711+
# Find a unique index
712+
index = 2
713+
while True:
714+
new_behavior_class = f"{behavior_class}_{index}"
715+
new_perf_class = f"{perf_class}_{index}"
716+
if package_path:
717+
new_behavior_path = test_dir / package_path / f"{new_behavior_class}.java"
718+
new_perf_path = test_dir / package_path / f"{new_perf_class}.java"
719+
else:
720+
new_behavior_path = test_dir / f"{new_behavior_class}.java"
721+
new_perf_path = test_dir / f"{new_perf_class}.java"
722+
if new_behavior_path not in used_paths:
723+
behavior_path = new_behavior_path
724+
perf_path = new_perf_path
725+
# Rename class in source code - replace the class declaration
726+
modified_behavior_source = re.sub(
727+
rf'^((?:public\s+)?class\s+){re.escape(behavior_class)}(\b)',
728+
rf'\g<1>{new_behavior_class}\g<2>',
729+
behavior_source,
730+
count=1,
731+
flags=re.MULTILINE,
732+
)
733+
modified_perf_source = re.sub(
734+
rf'^((?:public\s+)?class\s+){re.escape(perf_class)}(\b)',
735+
rf'\g<1>{new_perf_class}\g<2>',
736+
perf_source,
737+
count=1,
738+
flags=re.MULTILINE,
739+
)
740+
logger.debug(
741+
f"[JAVA] Renamed duplicate test class from {behavior_class} to {new_behavior_class}"
742+
)
743+
break
744+
index += 1
745+
698746
# Create directories if needed
699747
behavior_path.parent.mkdir(parents=True, exist_ok=True)
700748
perf_path.parent.mkdir(parents=True, exist_ok=True)
701749

702750
logger.debug(f"[JAVA] Fixed paths: behavior={behavior_path}, perf={perf_path}")
703-
return behavior_path, perf_path
751+
return behavior_path, perf_path, modified_behavior_source, modified_perf_source
704752

705753
# note: this isn't called by the lsp, only called by cli
706754
def optimize_function(self) -> Result[BestOptimization, str]:

0 commit comments

Comments
 (0)