Skip to content

Commit e7596b5

Browse files
github-actions[bot]mohammed ahmed
andcommitted
style: fix naming, imports, and test conventions in js_project_root fix
- Rename `_get_js_project_root` → `get_js_project_root` (no leading underscores per convention) - Remove redundant `from pathlib import Path` import inside method (already imported at top) - Remove unnecessary docstring from new method - Rewrite tests to use `tmp_path` fixture instead of `tempfile.TemporaryDirectory()` - Add `.resolve()` calls and `encoding="utf-8"` per project conventions - Simplify second test file to focus on the actual caching behavior Co-authored-by: mohammed ahmed <undefined@users.noreply.github.com>
1 parent 46c4991 commit e7596b5

3 files changed

Lines changed: 111 additions & 245 deletions

File tree

codeflash/languages/function_optimizer.py

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3085,29 +3085,15 @@ def run_optimized_candidate(
30853085
)
30863086
)
30873087

3088-
def _get_js_project_root(self) -> Path | None:
3089-
"""Get the JavaScript project root for the current function being optimized.
3090-
3091-
This method calculates the js_project_root for each function instead of
3092-
caching it in test_cfg. This is important in monorepos where different
3093-
functions may belong to different packages/extensions with their own
3094-
package.json files.
3095-
3096-
Returns:
3097-
Path to the JavaScript project root, or None if not a JavaScript project
3098-
or if the project root cannot be determined.
3099-
"""
3088+
def get_js_project_root(self) -> Path | None:
31003089
# Only calculate for JavaScript/TypeScript projects
31013090
if self.function_to_optimize.language not in ("javascript", "typescript"):
31023091
return self.test_cfg.js_project_root # Fall back to cached value for non-JS
31033092

3104-
# For JS/TS, calculate fresh for each function
3105-
from pathlib import Path
3106-
3093+
# For JS/TS, calculate fresh for each function to support monorepos
31073094
from codeflash.languages.javascript.test_runner import find_node_project_root
31083095

3109-
source_file = Path(self.function_to_optimize.file_path)
3110-
return find_node_project_root(source_file)
3096+
return find_node_project_root(Path(self.function_to_optimize.file_path))
31113097

31123098
def run_and_parse_tests(
31133099
self,
@@ -3129,7 +3115,7 @@ def run_and_parse_tests(
31293115
if testing_type == TestingMode.BEHAVIOR:
31303116
# Calculate js_project_root for the current function being optimized
31313117
# instead of using cached value from test_cfg, which may be from a different function
3132-
js_project_root = self._get_js_project_root()
3118+
js_project_root = self.get_js_project_root()
31333119

31343120
result_file_path, run_result, coverage_database_file, coverage_config_file = (
31353121
self.language_support.run_behavioral_tests(
@@ -3143,7 +3129,7 @@ def run_and_parse_tests(
31433129
)
31443130
)
31453131
elif testing_type == TestingMode.LINE_PROFILE:
3146-
js_project_root = self._get_js_project_root()
3132+
js_project_root = self.get_js_project_root()
31473133
result_file_path, run_result = self.language_support.run_line_profile_tests(
31483134
test_paths=test_files,
31493135
test_env=test_env,
@@ -3153,7 +3139,7 @@ def run_and_parse_tests(
31533139
line_profile_output_file=line_profiler_output_file,
31543140
)
31553141
elif testing_type == TestingMode.PERFORMANCE:
3156-
js_project_root = self._get_js_project_root()
3142+
js_project_root = self.get_js_project_root()
31573143
result_file_path, run_result = self.language_support.run_benchmarking_tests(
31583144
test_paths=test_files,
31593145
test_env=test_env,
Lines changed: 58 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -1,95 +1,66 @@
11
"""Test that js_project_root is recalculated per function, not cached."""
22

3-
import tempfile
43
from pathlib import Path
54

6-
import pytest
7-
85
from codeflash.languages.javascript.test_runner import find_node_project_root
96

107

11-
def test_find_node_project_root_returns_different_roots_for_different_files():
8+
def test_find_node_project_root_returns_different_roots_for_different_files(tmp_path: Path) -> None:
129
"""Test that find_node_project_root returns the correct root for each file."""
13-
with tempfile.TemporaryDirectory() as tmpdir:
14-
root = Path(tmpdir)
15-
16-
# Create main project structure
17-
main_project = root / "project"
18-
main_project.mkdir()
19-
(main_project / "package.json").write_text("{}")
20-
(main_project / "src").mkdir()
21-
main_file = main_project / "src" / "main.ts"
22-
main_file.write_text("// main file")
23-
24-
# Create extension subdirectory with its own package.json
25-
extension_dir = main_project / "extensions" / "discord"
26-
extension_dir.mkdir(parents=True)
27-
(extension_dir / "package.json").write_text("{}")
28-
(extension_dir / "src").mkdir()
29-
extension_file = extension_dir / "src" / "accounts.ts"
30-
extension_file.write_text("// extension file")
31-
32-
# Test 1: Extension file should return extension directory
33-
result1 = find_node_project_root(extension_file)
34-
assert result1 == extension_dir, (
35-
f"Expected {extension_dir}, got {result1}"
36-
)
37-
38-
# Test 2: Main file should return main project directory
39-
result2 = find_node_project_root(main_file)
40-
assert result2 == main_project, (
41-
f"Expected {main_project}, got {result2}"
42-
)
43-
44-
# Test 3: Calling again with extension file should still return extension dir
45-
result3 = find_node_project_root(extension_file)
46-
assert result3 == extension_dir, (
47-
f"Expected {extension_dir}, got {result3}"
48-
)
49-
50-
51-
def test_js_project_root_should_be_recalculated_per_function():
52-
"""
53-
Test the actual bug: when optimizing multiple functions from different
54-
directories, each should get its own js_project_root, not inherit from
55-
the first function.
56-
57-
This test simulates the scenario where:
58-
1. Function #1 is in extensions/discord/src/accounts.ts
59-
2. Function #2 is in src/plugins/commands.ts
60-
3. Both should get their correct respective project roots
61-
"""
62-
with tempfile.TemporaryDirectory() as tmpdir:
63-
root = Path(tmpdir)
64-
65-
# Create main project
66-
main_project = root / "project"
67-
main_project.mkdir()
68-
(main_project / "package.json").write_text('{"name": "main"}')
69-
(main_project / "src").mkdir()
70-
(main_project / "test").mkdir()
71-
72-
# Create extension with its own package.json
73-
extension_dir = main_project / "extensions" / "discord"
74-
extension_dir.mkdir(parents=True)
75-
(extension_dir / "package.json").write_text('{"name": "discord-extension"}')
76-
(extension_dir / "src").mkdir()
77-
78-
# Files to optimize
79-
extension_file = extension_dir / "src" / "accounts.ts"
80-
extension_file.write_text("export function foo() {}")
81-
82-
main_file = main_project / "src" / "commands.ts"
83-
main_file.write_text("export function bar() {}")
84-
85-
# Simulate what happens in Codeflash optimizer
86-
# Function 1 (extension file) sets js_project_root
87-
js_project_root_1 = find_node_project_root(extension_file)
88-
assert js_project_root_1 == extension_dir
89-
90-
# Function 2 (main file) should get its own root, not inherit from function 1
91-
js_project_root_2 = find_node_project_root(main_file)
92-
assert js_project_root_2 == main_project, (
93-
f"Bug reproduced: main file got {js_project_root_2} instead of {main_project}. "
94-
f"This happens when test_cfg.js_project_root is not recalculated per function."
95-
)
10+
# Create main project structure
11+
main_project = (tmp_path / "project").resolve()
12+
main_project.mkdir()
13+
(main_project / "package.json").write_text("{}", encoding="utf-8")
14+
(main_project / "src").mkdir()
15+
main_file = (main_project / "src" / "main.ts").resolve()
16+
main_file.write_text("// main file", encoding="utf-8")
17+
18+
# Create extension subdirectory with its own package.json
19+
extension_dir = (main_project / "extensions" / "discord").resolve()
20+
extension_dir.mkdir(parents=True)
21+
(extension_dir / "package.json").write_text("{}", encoding="utf-8")
22+
(extension_dir / "src").mkdir()
23+
extension_file = (extension_dir / "src" / "accounts.ts").resolve()
24+
extension_file.write_text("// extension file", encoding="utf-8")
25+
26+
# Extension file should return extension directory
27+
result1 = find_node_project_root(extension_file)
28+
assert result1 == extension_dir, f"Expected {extension_dir}, got {result1}"
29+
30+
# Main file should return main project directory
31+
result2 = find_node_project_root(main_file)
32+
assert result2 == main_project, f"Expected {main_project}, got {result2}"
33+
34+
# Calling again with extension file should still return extension dir
35+
result3 = find_node_project_root(extension_file)
36+
assert result3 == extension_dir, f"Expected {extension_dir}, got {result3}"
37+
38+
39+
def test_js_project_root_recalculated_per_function(tmp_path: Path) -> None:
40+
"""Each function in a monorepo should resolve to its own nearest package.json root."""
41+
# Create main project
42+
main_project = (tmp_path / "project").resolve()
43+
main_project.mkdir()
44+
(main_project / "package.json").write_text('{"name": "main"}', encoding="utf-8")
45+
(main_project / "src").mkdir()
46+
47+
# Create extension with its own package.json
48+
extension_dir = (main_project / "extensions" / "discord").resolve()
49+
extension_dir.mkdir(parents=True)
50+
(extension_dir / "package.json").write_text('{"name": "discord-extension"}', encoding="utf-8")
51+
(extension_dir / "src").mkdir()
52+
53+
extension_file = (extension_dir / "src" / "accounts.ts").resolve()
54+
extension_file.write_text("export function foo() {}", encoding="utf-8")
55+
56+
main_file = (main_project / "src" / "commands.ts").resolve()
57+
main_file.write_text("export function bar() {}", encoding="utf-8")
58+
59+
js_project_root_1 = find_node_project_root(extension_file)
60+
assert js_project_root_1 == extension_dir
61+
62+
js_project_root_2 = find_node_project_root(main_file)
63+
assert js_project_root_2 == main_project, (
64+
f"Expected {main_project}, got {js_project_root_2}. "
65+
f"Happens when js_project_root is not recalculated per function."
66+
)

0 commit comments

Comments
 (0)