Skip to content

Commit 9ac3d14

Browse files
HeshamHM28claude
andcommitted
fix: harden settings.gradle module detection and use exact test assertions
- Replace fragile multi-regex settings.gradle parser with a single broad pattern that extracts all quoted module identifiers, handling any DSL style - Add _scan_filesystem_for_modules() as fallback when settings parsing fails - Fix all test assertions to use full string equality (== []) not substring (in) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent dd107d5 commit 9ac3d14

3 files changed

Lines changed: 63 additions & 53 deletions

File tree

codeflash/languages/java/test_runner.py

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -202,29 +202,43 @@ def _validate_test_filter(test_filter: str) -> str:
202202
def _extract_modules_from_settings_gradle(content: str) -> list[str]:
203203
"""Extract module names from settings.gradle(.kts) content.
204204
205-
Handles several patterns:
206-
include("module-a", "module-b") // Kotlin DSL (single/multi-line)
207-
include 'module-a', 'module-b' // Groovy DSL
208-
val projects = listOf("module-a", "module-b") // Kotlin variable lists
209-
Module names may be prefixed with ':' which is stripped.
205+
Collects every quoted string that looks like a Gradle module identifier
206+
(word chars, hyphens, colons, dots). This is intentionally broad so it
207+
works regardless of DSL style — ``include("a")``, ``listOf("a")``,
208+
variable-based indirection, etc. False positives are harmless because
209+
callers match module names against actual file paths.
210210
"""
211+
seen: set[str] = set()
211212
modules: list[str] = []
212-
# Match include(...) calls, including multi-line ones
213-
for match in re.findall(r"""include\s*\(([^)]*)\)""", content, re.DOTALL):
214-
for name in re.findall(r"""['"]([^'"]+)['"]""", match):
215-
modules.append(name.lstrip(":"))
216-
# Match Groovy-style: include 'mod-a', 'mod-b' (no parens, single line)
217-
for match in re.findall(r"""include\s+(?=['"])([^\n]+)""", content):
218-
for name in re.findall(r"""['"]([^'"]+)['"]""", match):
219-
clean = name.lstrip(":")
220-
if clean not in modules:
221-
modules.append(clean)
222-
# Match Kotlin-style variable lists: val x = listOf("mod-a", "mod-b")
223-
for match in re.findall(r"""listOf\s*\(([^)]*)\)""", content, re.DOTALL):
224-
for name in re.findall(r"""['"]([^'"]+)['"]""", match):
225-
clean = name.lstrip(":")
226-
if clean not in modules:
227-
modules.append(clean)
213+
for name in re.findall(r"""['"]([:\w][\w.:-]*)['"]""", content):
214+
clean = name.lstrip(":")
215+
if clean and clean not in seen:
216+
seen.add(clean)
217+
modules.append(clean)
218+
return modules
219+
220+
221+
def _scan_filesystem_for_modules(directory: Path) -> list[str]:
222+
"""Detect Gradle/Maven sub-modules by scanning for build files on disk.
223+
224+
Checks up to two directory levels (covers nested modules like
225+
``connect/runtime``). This is a reliable fallback when the settings
226+
file uses complex DSL that text-based parsing cannot handle.
227+
"""
228+
modules: list[str] = []
229+
_BUILD_FILES = ("build.gradle", "build.gradle.kts", "pom.xml")
230+
for child in directory.iterdir():
231+
if not child.is_dir() or child.name.startswith("."):
232+
continue
233+
if any((child / bf).exists() for bf in _BUILD_FILES):
234+
modules.append(child.name)
235+
# One level deeper for nested modules (connect/runtime)
236+
for grandchild in child.iterdir():
237+
if not grandchild.is_dir() or grandchild.name.startswith("."):
238+
continue
239+
if any((grandchild / bf).exists() for bf in _BUILD_FILES):
240+
rel = grandchild.relative_to(directory)
241+
modules.append(str(rel).replace(os.sep, ":"))
228242
return modules
229243

230244

@@ -251,7 +265,8 @@ def _detect_modules(directory: Path) -> list[str]:
251265
except Exception:
252266
pass
253267

254-
return []
268+
# Fallback: scan filesystem for directories with build files
269+
return _scan_filesystem_for_modules(directory)
255270

256271

257272
def _is_build_root(directory: Path) -> bool:

tests/test_gradle_resolve_project_classpath.py

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -77,25 +77,24 @@ def test_replaces_missing_jars_with_class_dirs(strategy, build_root, tmp_path):
7777

7878
entries = result.split(os.pathsep)
7979

80-
# Module A: JAR replaced with java/main classes + resources/main
80+
ext_jar = str(tmp_path / "gradle-cache" / "some-lib-1.0.jar")
8181
mod_a_java = str(build_root / "module-a" / "build" / "classes" / "java" / "main")
8282
mod_a_resources = str(build_root / "module-a" / "build" / "resources" / "main")
83-
assert mod_a_java in entries
84-
assert mod_a_resources in entries
85-
86-
# Module B: JAR replaced with both kotlin/main and java/main classes
8783
mod_b_kotlin = str(build_root / "module-b" / "build" / "classes" / "kotlin" / "main")
8884
mod_b_java = str(build_root / "module-b" / "build" / "classes" / "java" / "main")
89-
assert mod_b_kotlin in entries
90-
assert mod_b_java in entries
91-
92-
# External JAR preserved as-is
93-
ext_jar = str(tmp_path / "gradle-cache" / "some-lib-1.0.jar")
94-
assert ext_jar in entries
95-
96-
# Original JAR paths should NOT be present
97-
assert str(build_root / "module-a" / "build" / "libs" / "module-a-1.0.jar") not in entries
98-
assert str(build_root / "module-b" / "build" / "libs" / "module-b-1.0.jar") not in entries
85+
mod_c_java = str(build_root / "module-c" / "build" / "classes" / "java" / "main")
86+
87+
# Full equality: module-a JAR → java/main + resources/main,
88+
# external JAR preserved, module-b JAR → kotlin/main + java/main,
89+
# module-c JAR → java/main (compiled by mock)
90+
assert entries == [
91+
mod_a_java,
92+
mod_a_resources,
93+
ext_jar,
94+
mod_b_kotlin,
95+
mod_b_java,
96+
mod_c_java,
97+
]
9998

10099

101100
def test_compiles_uncompiled_modules(strategy, build_root, tmp_path):

tests/test_languages/test_java/test_java_test_paths.py

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -489,46 +489,42 @@ class TestExtractModulesFromSettingsGradle:
489489
def test_simple_top_level_modules(self):
490490
content = """include("streams", "clients", "tools")"""
491491
modules = _extract_modules_from_settings_gradle(content)
492-
assert "streams" in modules
493-
assert "clients" in modules
494-
assert "tools" in modules
492+
assert modules == ["streams", "clients", "tools"]
495493

496494
def test_nested_gradle_modules(self):
497495
"""Nested modules like connect:runtime should be extracted."""
498496
content = """include("connect:runtime", "connect:api", "streams")"""
499497
modules = _extract_modules_from_settings_gradle(content)
500-
assert "connect:runtime" in modules
501-
assert "connect:api" in modules
502-
assert "streams" in modules
498+
assert modules == ["connect:runtime", "connect:api", "streams"]
503499

504500
def test_leading_colon_stripped(self):
505501
content = """include(":streams", ":clients")"""
506502
modules = _extract_modules_from_settings_gradle(content)
507-
assert "streams" in modules
508-
assert "clients" in modules
503+
assert modules == ["streams", "clients"]
509504

510505
def test_multiline_include(self):
511506
"""Multi-line include() calls should be parsed correctly."""
512507
content = 'include(\n "rewrite-core",\n "rewrite-java",\n "rewrite-test"\n)'
513508
modules = _extract_modules_from_settings_gradle(content)
514-
assert "rewrite-core" in modules
515-
assert "rewrite-java" in modules
516-
assert "rewrite-test" in modules
509+
assert modules == ["rewrite-core", "rewrite-java", "rewrite-test"]
517510

518511
def test_kotlin_listof_variable(self):
519512
"""Kotlin-style val x = listOf(...) should be parsed for module names."""
520513
content = 'val allProjects = listOf(\n "rewrite-core",\n "rewrite-java",\n "rewrite-test"\n)\ninclude(*(allProjects).toTypedArray())'
521514
modules = _extract_modules_from_settings_gradle(content)
522-
assert "rewrite-core" in modules
523-
assert "rewrite-java" in modules
524-
assert "rewrite-test" in modules
515+
assert modules == ["rewrite-core", "rewrite-java", "rewrite-test"]
525516

526517
def test_groovy_include_without_parens(self):
527518
"""Groovy-style include without parentheses."""
528519
content = "include 'streams', 'clients'"
529520
modules = _extract_modules_from_settings_gradle(content)
530-
assert "streams" in modules
531-
assert "clients" in modules
521+
assert modules == ["streams", "clients"]
522+
523+
def test_deduplicates_modules(self):
524+
"""Same module referenced in listOf and include should appear once."""
525+
content = 'val mods = listOf("core")\ninclude("core", "web")'
526+
modules = _extract_modules_from_settings_gradle(content)
527+
assert modules == ["core", "web"]
532528

533529

534530
class TestFindMultiModuleRoot:

0 commit comments

Comments
 (0)