Skip to content

Commit 2f36044

Browse files
authored
Merge branch 'main' into aseembits93-patch-1
2 parents 5a5b16c + 4a978b1 commit 2f36044

2 files changed

Lines changed: 145 additions & 12 deletions

File tree

codeflash/discovery/functions_to_optimize.py

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,23 @@ def __init__(self, file_path: str) -> None:
7878
self.file_path: str = file_path
7979
self.functions: list[FunctionToOptimize] = []
8080

81+
@staticmethod
82+
def is_pytest_fixture(node: cst.FunctionDef) -> bool:
83+
for decorator in node.decorators:
84+
dec = decorator.decorator
85+
if isinstance(dec, cst.Call):
86+
dec = dec.func
87+
if isinstance(dec, cst.Attribute) and dec.attr.value == "fixture":
88+
if isinstance(dec.value, cst.Name) and dec.value.value == "pytest":
89+
return True
90+
if isinstance(dec, cst.Name) and dec.value == "fixture":
91+
return True
92+
return False
93+
8194
def visit_FunctionDef(self, node: cst.FunctionDef) -> None:
8295
return_visitor: ReturnStatementVisitor = ReturnStatementVisitor()
8396
node.visit(return_visitor)
84-
if return_visitor.has_return_statement:
97+
if return_visitor.has_return_statement and not self.is_pytest_fixture(node):
8598
pos: CodeRange = self.get_metadata(cst.metadata.PositionProvider, node)
8699
parents: CSTNode | None = self.get_metadata(cst.metadata.ParentNodeProvider, node)
87100
ast_parents: list[FunctionParent] = []
@@ -108,14 +121,12 @@ def __init__(self, file_path: Path) -> None:
108121
self.file_path: Path = file_path
109122

110123
def visit_FunctionDef(self, node: FunctionDef) -> None:
111-
# Check if the function has a return statement and add it to the list
112124
if function_has_return_statement(node) and not function_is_a_property(node):
113125
self.functions.append(
114126
FunctionToOptimize(function_name=node.name, file_path=self.file_path, parents=self.ast_path[:])
115127
)
116128

117129
def visit_AsyncFunctionDef(self, node: AsyncFunctionDef) -> None:
118-
# Check if the async function has a return statement and add it to the list
119130
if function_has_return_statement(node) and not function_is_a_property(node):
120131
self.functions.append(
121132
FunctionToOptimize(
@@ -831,22 +842,17 @@ def filter_functions(
831842
test_dir_patterns = (os.sep + "test" + os.sep, os.sep + "tests" + os.sep, os.sep + "__tests__" + os.sep)
832843

833844
def is_test_file(file_path_normalized: str) -> bool:
834-
"""Check if a file is a test file based on patterns."""
835845
if tests_root_overlaps_source:
836-
# Use file pattern matching when tests_root overlaps with source
837846
file_lower = file_path_normalized.lower()
838-
# Check filename patterns (e.g., .test.ts, .spec.ts)
847+
basename = Path(file_lower).name
848+
if basename.startswith("test_") or basename == "conftest.py":
849+
return True
839850
if any(pattern in file_lower for pattern in test_file_name_patterns):
840851
return True
841-
# Check directory patterns, but only within the project root
842-
# to avoid false positives from parent directories (e.g., project at /home/user/tests/myproject)
843852
if project_root_str and file_lower.startswith(project_root_str.lower()):
844853
relative_path = file_lower[len(project_root_str) :]
845854
return any(pattern in relative_path for pattern in test_dir_patterns)
846-
# If we can't compute relative path from project root, don't check directory patterns
847-
# This avoids false positives when project is inside a folder named "tests"
848855
return False
849-
# Use directory-based filtering when tests are in a separate directory
850856
return file_path_normalized.startswith(tests_root_str + os.sep)
851857

852858
# We desperately need Python 3.10+ only support to make this code readable with structural pattern matching

tests/test_function_discovery.py

Lines changed: 128 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1149,4 +1149,131 @@ def test_is_object_empty():
11491149
)
11501150

11511151
# Strict check: exactly 2 functions
1152-
assert count == 2, f"Expected exactly 2 functions, got {count}"
1152+
assert count == 2, f"Expected exactly 2 functions, got {count}"
1153+
1154+
1155+
def test_filter_functions_python_test_prefix_convention():
1156+
"""Test that files following Python's test_*.py naming convention are filtered.
1157+
1158+
Python's standard test file naming uses the test_ prefix (e.g., test_utils.py),
1159+
which was previously not caught by the pattern matching in overlapping mode.
1160+
"""
1161+
with tempfile.TemporaryDirectory() as temp_dir_str:
1162+
temp_dir = Path(temp_dir_str)
1163+
1164+
# Source file that should NOT be filtered
1165+
source_file = temp_dir / "utils.py"
1166+
with source_file.open("w") as f:
1167+
f.write("def process(): return 1")
1168+
1169+
# Python test file with test_ prefix - SHOULD be filtered
1170+
test_prefix_file = temp_dir / "test_utils.py"
1171+
with test_prefix_file.open("w") as f:
1172+
f.write("def test_process(): return 1")
1173+
1174+
# conftest.py - SHOULD be filtered
1175+
conftest_file = temp_dir / "conftest.py"
1176+
with conftest_file.open("w") as f:
1177+
f.write("""
1178+
import pytest
1179+
1180+
@pytest.fixture
1181+
def sample_data():
1182+
return [1, 2, 3]
1183+
""")
1184+
1185+
# File in a test_ prefixed directory - should NOT be filtered by file patterns
1186+
# (directory patterns don't cover test_ prefix dirs, which is fine)
1187+
test_subdir = temp_dir / "test_integration"
1188+
test_subdir.mkdir()
1189+
file_in_test_dir = test_subdir / "helpers.py"
1190+
with file_in_test_dir.open("w") as f:
1191+
f.write("def helper(): return 1")
1192+
1193+
# test_ prefix file inside a subdirectory - SHOULD be filtered
1194+
test_in_subdir = test_subdir / "test_helpers.py"
1195+
with test_in_subdir.open("w") as f:
1196+
f.write("def test_helper(): return 1")
1197+
1198+
all_functions = {}
1199+
for file_path in [source_file, test_prefix_file, conftest_file, file_in_test_dir, test_in_subdir]:
1200+
discovered = find_all_functions_in_file(file_path)
1201+
all_functions.update(discovered)
1202+
1203+
with unittest.mock.patch(
1204+
"codeflash.discovery.functions_to_optimize.get_blocklisted_functions", return_value={}
1205+
):
1206+
filtered, count = filter_functions(
1207+
all_functions,
1208+
tests_root=temp_dir, # Overlapping case
1209+
ignore_paths=[],
1210+
project_root=temp_dir,
1211+
module_root=temp_dir,
1212+
)
1213+
1214+
# source_file and file_in_test_dir should remain
1215+
# test_prefix_file, conftest_file, and test_in_subdir should be filtered
1216+
expected_files = {source_file, file_in_test_dir}
1217+
assert set(filtered.keys()) == expected_files, (
1218+
f"Expected {expected_files}, got {set(filtered.keys())}"
1219+
)
1220+
assert count == 2, f"Expected exactly 2 functions, got {count}"
1221+
1222+
1223+
def test_pytest_fixture_not_discovered():
1224+
"""Test that @pytest.fixture decorated functions are not discovered via libcst path."""
1225+
from codeflash.languages.python.support import PythonSupport
1226+
1227+
with tempfile.TemporaryDirectory() as temp_dir_str:
1228+
temp_dir = Path(temp_dir_str)
1229+
1230+
fixture_file = temp_dir / "conftest.py"
1231+
with fixture_file.open("w") as f:
1232+
f.write("""
1233+
import pytest
1234+
from pytest import fixture
1235+
1236+
def regular_function():
1237+
return 42
1238+
1239+
@pytest.fixture
1240+
def sample_data():
1241+
return [1, 2, 3]
1242+
1243+
@pytest.fixture()
1244+
def sample_config():
1245+
return {"key": "value"}
1246+
1247+
@fixture
1248+
def direct_import_fixture():
1249+
return "data"
1250+
1251+
@fixture()
1252+
def direct_import_fixture_with_parens():
1253+
return "data"
1254+
1255+
@pytest.fixture(scope="session")
1256+
def session_fixture():
1257+
return "session"
1258+
1259+
class TestHelpers:
1260+
@pytest.fixture
1261+
def class_fixture(self):
1262+
return "class_data"
1263+
1264+
def helper_method(self):
1265+
return "helper"
1266+
""")
1267+
1268+
support = PythonSupport()
1269+
functions = support.discover_functions(fixture_file)
1270+
function_names = [fn.function_name for fn in functions]
1271+
1272+
assert "regular_function" in function_names
1273+
assert "helper_method" in function_names
1274+
assert "sample_data" not in function_names
1275+
assert "sample_config" not in function_names
1276+
assert "direct_import_fixture" not in function_names
1277+
assert "direct_import_fixture_with_parens" not in function_names
1278+
assert "session_fixture" not in function_names
1279+
assert "class_fixture" not in function_names

0 commit comments

Comments
 (0)