Skip to content

Commit 3c38a80

Browse files
authored
Merge pull request #1931 from codeflash-ai/fix/js-setup-early-exit
Fix: early exit on JS setup failures
2 parents 5baccd4 + 5b84811 commit 3c38a80

7 files changed

Lines changed: 125 additions & 33 deletions

File tree

codeflash/languages/base.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ class IndexResult:
5050
error: bool
5151

5252

53+
@dataclass(frozen=True)
54+
class SetupError:
55+
message: str
56+
should_abort: bool
57+
58+
5359
@dataclass
5460
class HelperFunction:
5561
"""A helper function that is a dependency of the target function.
@@ -725,8 +731,12 @@ def prepare_module(
725731
"""Parse/validate a module before optimization."""
726732
...
727733

728-
def setup_test_config(self, test_cfg: TestConfig, file_path: Path, current_worktree: Path | None = None) -> None:
729-
"""One-time project setup after language detection. Default: no-op."""
734+
def setup_test_config(self, test_cfg: TestConfig, file_path: Path, current_worktree: Path | None = None) -> bool:
735+
"""One-time project setup after language detection. Default: no-op.
736+
737+
Returns True if the project is valid for optimization, False otherwise.
738+
"""
739+
return True
730740

731741
def adjust_test_config_for_discovery(self, test_cfg: TestConfig) -> None:
732742
"""Adjust test config before test discovery. Default: no-op."""

codeflash/languages/java/support.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,11 +403,12 @@ def load_coverage(
403403
) -> None:
404404
return None
405405

406-
def setup_test_config(self, test_cfg: Any, file_path: Path, current_worktree: Path | None = None) -> None:
406+
def setup_test_config(self, test_cfg: Any, file_path: Path, current_worktree: Path | None = None) -> bool:
407407
"""Detect test framework from project build config (pom.xml / build.gradle)."""
408408
config = detect_java_project(test_cfg.project_root_path)
409409
if config is not None:
410410
self._test_framework = config.test_framework
411+
return True
411412

412413
def adjust_test_config_for_discovery(self, test_cfg: Any) -> None:
413414
"""Adjust test config before test discovery for Java.

codeflash/languages/javascript/optimizer.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from typing import TYPE_CHECKING
44

55
from codeflash.cli_cmds.console import logger
6+
from codeflash.languages.base import SetupError
67
from codeflash.models.models import ValidCode
78

89
if TYPE_CHECKING:
@@ -25,19 +26,21 @@ def prepare_javascript_module(
2526
return validated_original_code, None
2627

2728

28-
def verify_js_requirements(test_cfg: TestConfig) -> None:
29+
def verify_js_requirements(test_cfg: TestConfig) -> list[SetupError]:
2930
"""Verify JavaScript/TypeScript requirements before optimization.
3031
3132
Checks that Node.js, npm, and the test framework are available.
3233
Logs warnings if requirements are not met but does not abort.
34+
35+
Returns: List of setup errors if requirements are not met, empty list otherwise.
3336
"""
3437
from codeflash.languages import get_language_support
3538
from codeflash.languages.base import Language
3639
from codeflash.languages.test_framework import get_js_test_framework_or_default
3740

3841
js_project_root = test_cfg.js_project_root
3942
if not js_project_root:
40-
return
43+
return [SetupError("JavaScript project root not found", should_abort=True)]
4144

4245
try:
4346
js_support = get_language_support(Language.JAVASCRIPT)
@@ -47,6 +50,9 @@ def verify_js_requirements(test_cfg: TestConfig) -> None:
4750
if not success:
4851
logger.warning("JavaScript requirements check found issues:")
4952
for error in errors:
50-
logger.warning(f" - {error}")
53+
logger.warning(f" - {error.message}")
54+
return errors
55+
return []
5156
except Exception as e:
5257
logger.debug(f"Failed to verify JS requirements: {e}")
58+
return [SetupError(str(e), should_abort=True)]

codeflash/languages/javascript/support.py

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,15 @@
1414

1515
from codeflash.code_utils.git_utils import git_root_dir, mirror_path
1616
from codeflash.discovery.functions_to_optimize import FunctionToOptimize
17-
from codeflash.languages.base import CodeContext, FunctionFilterCriteria, HelperFunction, Language, TestInfo, TestResult
17+
from codeflash.languages.base import (
18+
CodeContext,
19+
FunctionFilterCriteria,
20+
HelperFunction,
21+
Language,
22+
SetupError,
23+
TestInfo,
24+
TestResult,
25+
)
1826
from codeflash.languages.javascript.treesitter import TreeSitterAnalyzer, TreeSitterLanguage, get_analyzer_for_file
1927
from codeflash.languages.registry import register_language
2028
from codeflash.models.models import FunctionParent
@@ -1950,11 +1958,13 @@ def prepare_module(
19501958

19511959
return prepare_javascript_module(module_code, module_path)
19521960

1953-
def setup_test_config(self, test_cfg: TestConfig, file_path: Path, current_worktree: Path | None) -> None:
1961+
def setup_test_config(self, test_cfg: TestConfig, file_path: Path, current_worktree: Path | None) -> bool:
19541962
from codeflash.languages.javascript.optimizer import verify_js_requirements
19551963
from codeflash.languages.javascript.test_runner import find_node_project_root
19561964

19571965
test_cfg.js_project_root = find_node_project_root(file_path)
1966+
if test_cfg.js_project_root is None:
1967+
return False
19581968
if current_worktree is not None:
19591969
original_js_root = git_root_dir()
19601970
worktree_node_modules = test_cfg.js_project_root / "node_modules"
@@ -1970,7 +1980,11 @@ def setup_test_config(self, test_cfg: TestConfig, file_path: Path, current_workt
19701980
original_root_node_modules = original_js_root / "node_modules"
19711981
if original_root_node_modules.exists() and not worktree_root_node_modules.exists():
19721982
worktree_root_node_modules.symlink_to(original_root_node_modules)
1973-
verify_js_requirements(test_cfg)
1983+
setup_errors = verify_js_requirements(test_cfg)
1984+
if any(e.should_abort for e in setup_errors):
1985+
return False
1986+
1987+
return True
19741988

19751989
def adjust_test_config_for_discovery(self, test_cfg: TestConfig) -> None:
19761990
test_cfg.tests_project_rootdir = test_cfg.tests_root
@@ -2243,7 +2257,7 @@ def get_module_path(self, source_file: Path, project_root: Path, tests_root: Pat
22432257
return path_without_ext + ".js"
22442258
return path_without_ext
22452259

2246-
def verify_requirements(self, project_root: Path, test_framework: str = "jest") -> tuple[bool, list[str]]:
2260+
def verify_requirements(self, project_root: Path, test_framework: str = "jest") -> tuple[bool, list[SetupError]]:
22472261
"""Verify that all JavaScript requirements are met.
22482262
22492263
Checks for:
@@ -2263,27 +2277,40 @@ def verify_requirements(self, project_root: Path, test_framework: str = "jest")
22632277
Tuple of (success, list of error messages).
22642278
22652279
"""
2266-
errors: list[str] = []
2280+
errors: list[SetupError] = []
22672281

22682282
# Check Node.js
22692283
try:
22702284
result = subprocess.run(["node", "--version"], check=False, capture_output=True, text=True, timeout=10)
22712285
if result.returncode != 0:
2272-
errors.append("Node.js is not installed. Please install Node.js 18+ from https://nodejs.org/")
2286+
errors.append(
2287+
SetupError(
2288+
"Node.js is not installed. Please install Node.js 18+ from https://nodejs.org/",
2289+
should_abort=True,
2290+
)
2291+
)
22732292
except FileNotFoundError:
2274-
errors.append("Node.js is not installed. Please install Node.js 18+ from https://nodejs.org/")
2293+
errors.append(
2294+
SetupError(
2295+
"Node.js is not installed. Please install Node.js 18+ from https://nodejs.org/", should_abort=True
2296+
)
2297+
)
22752298
except Exception as e:
2276-
errors.append(f"Failed to check Node.js: {e}")
2299+
errors.append(SetupError(f"Failed to check Node.js: {e}", should_abort=True))
22772300

22782301
# Check npm
22792302
try:
22802303
result = subprocess.run(["npm", "--version"], check=False, capture_output=True, text=True, timeout=10)
22812304
if result.returncode != 0:
2282-
errors.append("npm is not available. Please ensure npm is installed with Node.js.")
2305+
errors.append(
2306+
SetupError("npm is not available. Please ensure npm is installed with Node.js.", should_abort=True)
2307+
)
22832308
except FileNotFoundError:
2284-
errors.append("npm is not available. Please ensure npm is installed with Node.js.")
2309+
errors.append(
2310+
SetupError("npm is not available. Please ensure npm is installed with Node.js.", should_abort=True)
2311+
)
22852312
except Exception as e:
2286-
errors.append(f"Failed to check npm: {e}")
2313+
errors.append(SetupError(f"Failed to check npm: {e}", should_abort=True))
22872314

22882315
# Check test framework is installed (with monorepo support)
22892316
# Uses find_node_modules_with_package which searches up the directory tree
@@ -2297,12 +2324,17 @@ def verify_requirements(self, project_root: Path, test_framework: str = "jest")
22972324
local_node_modules = project_root / "node_modules"
22982325
if not local_node_modules.exists():
22992326
errors.append(
2300-
f"node_modules not found in {project_root}. Please run 'npm install' to install dependencies."
2327+
SetupError(
2328+
f"node_modules not found in {project_root}. Please run 'npm install' to install dependencies.",
2329+
should_abort=True,
2330+
)
23012331
)
23022332
else:
23032333
errors.append(
2304-
f"{test_framework} is not installed. "
2305-
f"Please run 'npm install --save-dev {test_framework}' to install it."
2334+
SetupError(
2335+
f"{test_framework} is not installed. Please run 'npm install --save-dev {test_framework}' to install it.",
2336+
should_abort=True,
2337+
)
23062338
)
23072339

23082340
return len(errors) == 0, errors

codeflash/languages/python/support.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1044,8 +1044,9 @@ def prepare_module(
10441044

10451045
pytest_cmd: str = "pytest"
10461046

1047-
def setup_test_config(self, test_cfg: TestConfig, file_path: Path, current_worktree: Path | None = None) -> None:
1047+
def setup_test_config(self, test_cfg: TestConfig, file_path: Path, current_worktree: Path | None = None) -> bool:
10481048
self.pytest_cmd = test_cfg.pytest_cmd or "pytest"
1049+
return True
10491050

10501051
def pytest_cmd_tokens(self, is_posix: bool) -> list[str]:
10511052
import shlex

codeflash/optimization/optimizer.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -527,7 +527,11 @@ def run(self) -> None:
527527
if funcs and funcs[0].language:
528528
set_current_language(funcs[0].language)
529529
self.test_cfg.set_language(funcs[0].language)
530-
current_language_support().setup_test_config(self.test_cfg, file_path, self.current_worktree)
530+
if not current_language_support().setup_test_config(
531+
self.test_cfg, file_path, self.current_worktree
532+
):
533+
logger.error("Project setup failed — aborting optimization. Check warnings above for details.")
534+
return
531535
break
532536

533537
if self.args.all:

tests/test_languages/test_javascript_requirements.py

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ def test_verify_requirements_fails_without_node(self, js_support, project_with_j
9191

9292
assert success is False
9393
assert len(errors) >= 1
94-
node_error_found = any("Node.js" in error for error in errors)
94+
node_error_found = any("Node.js" in error.message for error in errors)
9595
assert node_error_found is True
9696

9797
def test_verify_requirements_fails_without_npm(self, js_support, project_with_jest):
@@ -108,7 +108,7 @@ def mock_run_side_effect(cmd, **kwargs):
108108
success, errors = js_support.verify_requirements(project_with_jest, "jest")
109109

110110
assert success is False
111-
npm_error_found = any("npm" in error for error in errors)
111+
npm_error_found = any("npm" in error.message for error in errors)
112112
assert npm_error_found is True
113113

114114
def test_verify_requirements_fails_without_node_modules(self, js_support, project_without_node_modules):
@@ -120,11 +120,11 @@ def test_verify_requirements_fails_without_node_modules(self, js_support, projec
120120

121121
assert success is False
122122
assert len(errors) == 1
123-
expected_error = (
123+
expected_message = (
124124
f"node_modules not found in {project_without_node_modules}. "
125125
f"Please run 'npm install' to install dependencies."
126126
)
127-
assert errors[0] == expected_error
127+
assert errors[0].message == expected_message
128128

129129
def test_verify_requirements_fails_without_test_framework(self, js_support, project_without_jest):
130130
"""Test verification fails when test framework is not installed."""
@@ -135,8 +135,8 @@ def test_verify_requirements_fails_without_test_framework(self, js_support, proj
135135

136136
assert success is False
137137
assert len(errors) == 1
138-
expected_error = "jest is not installed. Please run 'npm install --save-dev jest' to install it."
139-
assert errors[0] == expected_error
138+
expected_message = "jest is not installed. Please run 'npm install --save-dev jest' to install it."
139+
assert errors[0].message == expected_message
140140

141141
def test_verify_requirements_returns_multiple_errors(self, js_support, project_without_node_modules):
142142
"""Test that multiple errors can be returned."""
@@ -148,7 +148,7 @@ def test_verify_requirements_returns_multiple_errors(self, js_support, project_w
148148
assert success is False
149149
assert len(errors) >= 2
150150
# Should have errors for Node.js, npm, and node_modules
151-
error_text = " ".join(errors)
151+
error_text = " ".join(e.message for e in errors)
152152
assert "Node.js" in error_text
153153
assert "npm" in error_text
154154

@@ -161,8 +161,8 @@ def test_verify_requirements_vitest_not_installed(self, js_support, project_with
161161

162162
assert success is False
163163
assert len(errors) == 1
164-
expected_error = "vitest is not installed. Please run 'npm install --save-dev vitest' to install it."
165-
assert errors[0] == expected_error
164+
expected_message = "vitest is not installed. Please run 'npm install --save-dev vitest' to install it."
165+
assert errors[0].message == expected_message
166166

167167
def test_verify_requirements_jest_not_installed(self, js_support, project_with_vitest):
168168
"""Test verification fails when Jest is requested but only Vitest is installed."""
@@ -173,8 +173,46 @@ def test_verify_requirements_jest_not_installed(self, js_support, project_with_v
173173

174174
assert success is False
175175
assert len(errors) == 1
176-
expected_error = "jest is not installed. Please run 'npm install --save-dev jest' to install it."
177-
assert errors[0] == expected_error
176+
expected_message = "jest is not installed. Please run 'npm install --save-dev jest' to install it."
177+
assert errors[0].message == expected_message
178+
179+
180+
class TestSetupTestConfig:
181+
"""Tests for JavaScriptSupport.setup_test_config() early-exit behavior."""
182+
183+
@pytest.fixture
184+
def js_support(self):
185+
return JavaScriptSupport()
186+
187+
def test_setup_test_config_returns_false_on_abort_error(self, js_support, tmp_path):
188+
"""setup_test_config returns False when verify_js_requirements reports a should_abort error."""
189+
from codeflash.languages.base import SetupError
190+
191+
abort_error = SetupError("Node.js is not installed", should_abort=True)
192+
with (
193+
patch("codeflash.languages.javascript.test_runner.find_node_project_root", return_value=tmp_path.resolve()),
194+
patch("codeflash.languages.javascript.optimizer.verify_js_requirements", return_value=[abort_error]),
195+
):
196+
test_cfg = MagicMock()
197+
result = js_support.setup_test_config(test_cfg, tmp_path.resolve(), current_worktree=None)
198+
assert result is False
199+
200+
def test_setup_test_config_returns_true_on_no_errors(self, js_support, tmp_path):
201+
"""setup_test_config returns True when verify_js_requirements reports no errors."""
202+
with (
203+
patch("codeflash.languages.javascript.test_runner.find_node_project_root", return_value=tmp_path.resolve()),
204+
patch("codeflash.languages.javascript.optimizer.verify_js_requirements", return_value=[]),
205+
):
206+
test_cfg = MagicMock()
207+
result = js_support.setup_test_config(test_cfg, tmp_path.resolve(), current_worktree=None)
208+
assert result is True
209+
210+
def test_setup_test_config_returns_false_when_project_root_is_none(self, js_support, tmp_path):
211+
"""setup_test_config returns False when find_node_project_root returns None."""
212+
with patch("codeflash.languages.javascript.test_runner.find_node_project_root", return_value=None):
213+
test_cfg = MagicMock()
214+
result = js_support.setup_test_config(test_cfg, tmp_path.resolve(), current_worktree=None)
215+
assert result is False
178216

179217

180218
class TestVerifyRequirementsIntegration:

0 commit comments

Comments
 (0)