Skip to content

Commit 761736e

Browse files
authored
Merge pull request #91 from PyAutoLabs/feature/fast-plots-env-coverage
fix(env_config): substring-match patterns against full path
2 parents b1692d2 + 30eab70 commit 761736e

4 files changed

Lines changed: 97 additions & 15 deletions

File tree

autobuild/build_util.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,17 @@ def should_skip(file: Path, no_run_list: List[str]) -> bool:
5555
"""
5656
Return True if the file matches any entry in no_run_list.
5757
58-
Entries with a '/' are treated as path-specific patterns: the file is
59-
skipped only if that pattern appears in the file's path (without extension).
58+
Entries with a '/' are treated as path-specific patterns and are
59+
substring-matched against the file's full path **including extension** —
60+
so a pattern may include ``.py`` to anchor against the script form (e.g.
61+
``imaging/visualization.py`` matches ``scripts/imaging/visualization.py``
62+
but not ``scripts/imaging/visualization_jax.py``).
6063
Entries without a '/' match any file whose stem equals the entry.
6164
"""
62-
file_path_no_ext = str(file.with_suffix(""))
65+
file_str = str(file)
6366
for pattern in no_run_list:
6467
if "/" in pattern:
65-
if pattern in file_path_no_ext:
68+
if pattern in file_str:
6669
return True
6770
else:
6871
if file.stem == pattern:
@@ -72,10 +75,10 @@ def should_skip(file: Path, no_run_list: List[str]) -> bool:
7275

7376
def _find_skip_reason(file: Path, no_run_list: List[str], skip_reasons: dict) -> str:
7477
"""Find the reason a file is being skipped from the skip_reasons dict."""
75-
file_path_no_ext = str(file.with_suffix(""))
78+
file_str = str(file)
7679
for pattern in no_run_list:
7780
if "/" in pattern:
78-
if pattern in file_path_no_ext:
81+
if pattern in file_str:
7982
return skip_reasons.get(pattern, "No reason documented")
8083
else:
8184
if file.stem == pattern:

autobuild/env_config.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,10 @@ def build_env_for_script(
4444
env[key] = str(value)
4545

4646
file = Path(file)
47-
file_path_no_ext = str(file.with_suffix(""))
4847

4948
for override in env_config.get("overrides", []):
5049
pattern = override["pattern"]
51-
if _pattern_matches(file, file_path_no_ext, pattern):
50+
if _pattern_matches(file, pattern):
5251
for var_name in override.get("unset", []):
5352
env.pop(var_name, None)
5453
for key, value in override.get("set", {}).items():
@@ -57,14 +56,17 @@ def build_env_for_script(
5756
return env
5857

5958

60-
def _pattern_matches(file: Path, file_path_no_ext: str, pattern: str) -> bool:
59+
def _pattern_matches(file: Path, pattern: str) -> bool:
6160
"""Match a pattern against a file path.
6261
63-
Patterns containing '/' are substring-matched against the full path
64-
(without extension). Patterns without '/' match the file stem exactly.
65-
Same convention as build_util.should_skip().
62+
Patterns containing '/' are substring-matched against the file's full
63+
path **including extension** — so a pattern may include ``.py`` to anchor
64+
against the script form (e.g. ``imaging/visualization.py`` matches
65+
``scripts/imaging/visualization.py`` but not
66+
``scripts/imaging/visualization_jax.py``). Patterns without '/' match the
67+
file stem exactly. Same convention as build_util.should_skip().
6668
"""
6769
if "/" in pattern:
68-
return pattern in file_path_no_ext
70+
return pattern in str(file)
6971
else:
7072
return file.stem == pattern

autobuild/repro_command.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,9 @@ def canonical_env_for_script(file: Path, env_config: Optional[dict]) -> Dict[str
5555
for key, value in env_config.get("defaults", {}).items():
5656
env[key] = str(value)
5757

58-
file_path_no_ext = str(file.with_suffix(""))
5958
for override in env_config.get("overrides", []):
6059
pattern = override["pattern"]
61-
if _pattern_matches(file, file_path_no_ext, pattern):
60+
if _pattern_matches(file, pattern):
6261
for var_name in override.get("unset", []):
6362
env.pop(var_name, None)
6463
for key, value in override.get("set", {}).items():

tests/test_pattern_matches.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
"""Regression tests for the path-pattern matchers in autobuild.
2+
3+
Both ``env_config._pattern_matches`` and ``build_util.should_skip`` substring-
4+
match against the file's full path including extension. Patterns can therefore
5+
end in ``.py`` to anchor against the script form and avoid clobbering sibling
6+
``_jax`` / ``_jit`` variants. These tests lock in that convention — without
7+
them, the previous bug (substring match against the path with ``.py`` stripped)
8+
silently disabled overrides ending in ``.py``.
9+
"""
10+
11+
from pathlib import Path
12+
13+
from autobuild.build_util import should_skip
14+
from autobuild.env_config import _pattern_matches
15+
16+
17+
def test_dot_py_pattern_matches_only_the_py_script():
18+
"""Pattern ending in `.py` must NOT also match sibling files whose stem
19+
starts with the same prefix (e.g. `_jax`, `_jit`)."""
20+
py_file = Path("scripts/imaging/visualization.py")
21+
jax_file = Path("scripts/imaging/visualization_jax.py")
22+
jit_file = Path("scripts/imaging/visualization_jit.py")
23+
24+
assert _pattern_matches(py_file, "imaging/visualization.py") is True
25+
assert _pattern_matches(jax_file, "imaging/visualization.py") is False
26+
assert _pattern_matches(jit_file, "imaging/visualization.py") is False
27+
28+
29+
def test_prefix_pattern_substring_matches_siblings():
30+
"""A pattern without `.py` still substring-matches both the `.py` script
31+
and `_jax` / `_jit` siblings, because the prefix is shared. Authors who
32+
want to target only one must anchor with `.py`."""
33+
py_file = Path("scripts/imaging/visualization.py")
34+
jax_file = Path("scripts/imaging/visualization_jax.py")
35+
36+
assert _pattern_matches(py_file, "imaging/visualization") is True
37+
assert _pattern_matches(jax_file, "imaging/visualization") is True
38+
# _jax-specific pattern only matches the _jax file.
39+
assert _pattern_matches(py_file, "imaging/visualization_jax") is False
40+
assert _pattern_matches(jax_file, "imaging/visualization_jax") is True
41+
42+
43+
def test_stem_pattern_matches_exactly():
44+
"""Patterns without `/` must match the file stem exactly — not as a
45+
substring."""
46+
assert _pattern_matches(Path("scripts/a/foo.py"), "foo") is True
47+
assert _pattern_matches(Path("scripts/a/foobar.py"), "foo") is False
48+
assert _pattern_matches(Path("scripts/a/foo.py"), "foo.py") is False
49+
50+
51+
def test_directory_pattern_matches_anything_in_directory():
52+
"""A pattern ending in `/` matches every script under that directory."""
53+
assert _pattern_matches(Path("scripts/aggregator/test_x.py"), "aggregator/") is True
54+
assert _pattern_matches(Path("scripts/foo/bar.py"), "aggregator/") is False
55+
56+
57+
def test_should_skip_uses_same_convention():
58+
"""`should_skip` shares the convention with `_pattern_matches`; the same
59+
`.py` anchoring works there."""
60+
no_run = ["imaging/visualization.py"]
61+
assert should_skip(Path("scripts/imaging/visualization.py"), no_run) is True
62+
assert should_skip(Path("scripts/imaging/visualization_jax.py"), no_run) is False
63+
64+
65+
def test_should_skip_stem_pattern():
66+
no_run = ["foo"]
67+
assert should_skip(Path("scripts/a/foo.py"), no_run) is True
68+
assert should_skip(Path("scripts/a/foobar.py"), no_run) is False
69+
70+
71+
def test_should_skip_directory_pattern():
72+
no_run = ["aggregator/"]
73+
assert should_skip(Path("scripts/aggregator/test_x.py"), no_run) is True
74+
assert should_skip(Path("scripts/foo/bar.py"), no_run) is False
75+
76+
77+
def test_should_skip_empty_list():
78+
assert should_skip(Path("scripts/a/foo.py"), []) is False

0 commit comments

Comments
 (0)