Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .cursor/rules/pr-review.mdc
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,13 @@ When adding a new weblog, verify the name is unique across all languages. Search

- Ref: [build.md](mdc:docs/execute/build.md)

## 10. Manifest YAML Syntax
## 10. No Cross-Test-File Imports

A `test_*.py` file must never import from another `test_*.py` file (absolute like `from tests.xxx.test_yyy import ...` or relative like `from .test_yyy import ...`). If logic is shared between test files, it must be moved to a local `utils.py` file in the same folder instead. Flag any PR that adds such an import.

- Ref: [repository-structure.mdc](mdc:.cursor/rules/repository-structure.mdc), enforced by `tests/test_the_test/test_conventions.py::test_no_cross_test_file_imports`

## 11. Manifest YAML Syntax

- `bug` and `flaky` markers must include a JIRA ticket (e.g., `bug (JIRA-123)`)
- Values with special YAML characters (`>`, `<`, `:`, `#`) must be quoted
Expand Down
7 changes: 6 additions & 1 deletion .cursor/rules/repository-structure.mdc
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,9 @@ system-tests/
def test_XYZ(self):
...
```
- Never define a setup method without a matching test method.
- Never define a setup method without a matching test method.

## 6. No Cross-Test-File Imports
- A test file (`test_*.py`) must never import anything from another test file, whether via an absolute (`from tests.xxx.test_yyy import ...`) or relative (`from .test_yyy import ...`) import.
- If logic needs to be shared between test files, move it into a local `utils.py` file in the same folder and have both test files import from there.
- This is enforced by `tests/test_the_test/test_conventions.py::test_no_cross_test_file_imports`.
70 changes: 69 additions & 1 deletion tests/test_the_test/test_conventions.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import ast
import os
from utils import scenarios
from pathlib import Path
from utils import scenarios, logger


@scenarios.test_the_test
Expand All @@ -26,5 +28,71 @@ def test_utils():
raise ValueError(f"File {os.path.join(folder, file)} is not a test file or a utils file {folder}")


def _is_test_file(path: Path) -> bool:
return path.suffix == ".py" and path.name.startswith("test_")


def _resolve_import_target(current_file: Path, module: str | None, level: int) -> Path | None:
"""Resolve an import (relative or absolute) to the .py file path it points at, if any."""
if not module:
# `from . import something` / `from .. import something`: imports a package, not a specific file
return None

if level > 0:
directory = current_file.parent
for _ in range(level - 1):
directory = directory.parent
return directory.joinpath(*module.split(".")).with_suffix(".py")

if not module.startswith("tests."):
# not an import of another test module, nothing to resolve
return None

return Path(*module.split(".")).with_suffix(".py")


@scenarios.test_the_test
def test_no_cross_test_file_imports():
"""A test file must never import from another test file. If logic is shared, it must live in a local utils.py."""

has_error = False

for folder, _, files in os.walk("tests"):
if folder.startswith("tests/fuzzer"):
# do not check these folders, they are particular use cases
continue

for file in files:
if not file.startswith("test_") or not file.endswith(".py"):
continue

path = Path(folder, file)

tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))

for node in ast.walk(tree):
if isinstance(node, ast.ImportFrom):
targets = [_resolve_import_target(path, node.module, node.level)]
elif isinstance(node, ast.Import):
targets = [_resolve_import_target(path, alias.name, 0) for alias in node.names]
else:
continue

for target in targets:
if target is None or target == path:
continue

if _is_test_file(target):
logger.error(f"{path} imports from another test file: {target}")
has_error = True

if has_error:
raise ValueError(
"Some test file imports from another test file. "
"Shared logic must be moved to a local utils.py file instead."
)


if __name__ == "__main__":
test_utils()
test_no_cross_test_file_imports()
Loading