Skip to content
Merged
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
15 changes: 9 additions & 6 deletions autobuild/build_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,17 @@ def should_skip(file: Path, no_run_list: List[str]) -> bool:
"""
Return True if the file matches any entry in no_run_list.

Entries with a '/' are treated as path-specific patterns: the file is
skipped only if that pattern appears in the file's path (without extension).
Entries with a '/' are treated as path-specific patterns and are
substring-matched against the file's full path **including extension** —
so a pattern may include ``.py`` to anchor against the script form (e.g.
``imaging/visualization.py`` matches ``scripts/imaging/visualization.py``
but not ``scripts/imaging/visualization_jax.py``).
Entries without a '/' match any file whose stem equals the entry.
"""
file_path_no_ext = str(file.with_suffix(""))
file_str = str(file)
for pattern in no_run_list:
if "/" in pattern:
if pattern in file_path_no_ext:
if pattern in file_str:
return True
else:
if file.stem == pattern:
Expand All @@ -72,10 +75,10 @@ def should_skip(file: Path, no_run_list: List[str]) -> bool:

def _find_skip_reason(file: Path, no_run_list: List[str], skip_reasons: dict) -> str:
"""Find the reason a file is being skipped from the skip_reasons dict."""
file_path_no_ext = str(file.with_suffix(""))
file_str = str(file)
for pattern in no_run_list:
if "/" in pattern:
if pattern in file_path_no_ext:
if pattern in file_str:
return skip_reasons.get(pattern, "No reason documented")
else:
if file.stem == pattern:
Expand Down
16 changes: 9 additions & 7 deletions autobuild/env_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,10 @@ def build_env_for_script(
env[key] = str(value)

file = Path(file)
file_path_no_ext = str(file.with_suffix(""))

for override in env_config.get("overrides", []):
pattern = override["pattern"]
if _pattern_matches(file, file_path_no_ext, pattern):
if _pattern_matches(file, pattern):
for var_name in override.get("unset", []):
env.pop(var_name, None)
for key, value in override.get("set", {}).items():
Expand All @@ -57,14 +56,17 @@ def build_env_for_script(
return env


def _pattern_matches(file: Path, file_path_no_ext: str, pattern: str) -> bool:
def _pattern_matches(file: Path, pattern: str) -> bool:
"""Match a pattern against a file path.

Patterns containing '/' are substring-matched against the full path
(without extension). Patterns without '/' match the file stem exactly.
Same convention as build_util.should_skip().
Patterns containing '/' are substring-matched against the file's full
path **including extension** — so a pattern may include ``.py`` to anchor
against the script form (e.g. ``imaging/visualization.py`` matches
``scripts/imaging/visualization.py`` but not
``scripts/imaging/visualization_jax.py``). Patterns without '/' match the
file stem exactly. Same convention as build_util.should_skip().
"""
if "/" in pattern:
return pattern in file_path_no_ext
return pattern in str(file)
else:
return file.stem == pattern
3 changes: 1 addition & 2 deletions autobuild/repro_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,9 @@ def canonical_env_for_script(file: Path, env_config: Optional[dict]) -> Dict[str
for key, value in env_config.get("defaults", {}).items():
env[key] = str(value)

file_path_no_ext = str(file.with_suffix(""))
for override in env_config.get("overrides", []):
pattern = override["pattern"]
if _pattern_matches(file, file_path_no_ext, pattern):
if _pattern_matches(file, pattern):
for var_name in override.get("unset", []):
env.pop(var_name, None)
for key, value in override.get("set", {}).items():
Expand Down
78 changes: 78 additions & 0 deletions tests/test_pattern_matches.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
"""Regression tests for the path-pattern matchers in autobuild.

Both ``env_config._pattern_matches`` and ``build_util.should_skip`` substring-
match against the file's full path including extension. Patterns can therefore
end in ``.py`` to anchor against the script form and avoid clobbering sibling
``_jax`` / ``_jit`` variants. These tests lock in that convention — without
them, the previous bug (substring match against the path with ``.py`` stripped)
silently disabled overrides ending in ``.py``.
"""

from pathlib import Path

from autobuild.build_util import should_skip
from autobuild.env_config import _pattern_matches


def test_dot_py_pattern_matches_only_the_py_script():
"""Pattern ending in `.py` must NOT also match sibling files whose stem
starts with the same prefix (e.g. `_jax`, `_jit`)."""
py_file = Path("scripts/imaging/visualization.py")
jax_file = Path("scripts/imaging/visualization_jax.py")
jit_file = Path("scripts/imaging/visualization_jit.py")

assert _pattern_matches(py_file, "imaging/visualization.py") is True
assert _pattern_matches(jax_file, "imaging/visualization.py") is False
assert _pattern_matches(jit_file, "imaging/visualization.py") is False


def test_prefix_pattern_substring_matches_siblings():
"""A pattern without `.py` still substring-matches both the `.py` script
and `_jax` / `_jit` siblings, because the prefix is shared. Authors who
want to target only one must anchor with `.py`."""
py_file = Path("scripts/imaging/visualization.py")
jax_file = Path("scripts/imaging/visualization_jax.py")

assert _pattern_matches(py_file, "imaging/visualization") is True
assert _pattern_matches(jax_file, "imaging/visualization") is True
# _jax-specific pattern only matches the _jax file.
assert _pattern_matches(py_file, "imaging/visualization_jax") is False
assert _pattern_matches(jax_file, "imaging/visualization_jax") is True


def test_stem_pattern_matches_exactly():
"""Patterns without `/` must match the file stem exactly — not as a
substring."""
assert _pattern_matches(Path("scripts/a/foo.py"), "foo") is True
assert _pattern_matches(Path("scripts/a/foobar.py"), "foo") is False
assert _pattern_matches(Path("scripts/a/foo.py"), "foo.py") is False


def test_directory_pattern_matches_anything_in_directory():
"""A pattern ending in `/` matches every script under that directory."""
assert _pattern_matches(Path("scripts/aggregator/test_x.py"), "aggregator/") is True
assert _pattern_matches(Path("scripts/foo/bar.py"), "aggregator/") is False


def test_should_skip_uses_same_convention():
"""`should_skip` shares the convention with `_pattern_matches`; the same
`.py` anchoring works there."""
no_run = ["imaging/visualization.py"]
assert should_skip(Path("scripts/imaging/visualization.py"), no_run) is True
assert should_skip(Path("scripts/imaging/visualization_jax.py"), no_run) is False


def test_should_skip_stem_pattern():
no_run = ["foo"]
assert should_skip(Path("scripts/a/foo.py"), no_run) is True
assert should_skip(Path("scripts/a/foobar.py"), no_run) is False


def test_should_skip_directory_pattern():
no_run = ["aggregator/"]
assert should_skip(Path("scripts/aggregator/test_x.py"), no_run) is True
assert should_skip(Path("scripts/foo/bar.py"), no_run) is False


def test_should_skip_empty_list():
assert should_skip(Path("scripts/a/foo.py"), []) is False