Skip to content

Commit 5eccb16

Browse files
authored
chore: speed up lint.py and run in parallel with other tests (#14172)
This PR modifies `lint.py` to only look at files tracked by git, not all files. Previously, the old lint script would occasionally fail when running in parallel with the other tests, but this change should prevent those failures. Also, this PR improves the performance by roughly an order of magnitude.
1 parent c50c2ba commit 5eccb16

2 files changed

Lines changed: 66 additions & 48 deletions

File tree

tests/CMakeLists.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,9 +302,7 @@ foreach(T ${LEANLAKETESTS})
302302
endforeach(T)
303303

304304
# Lint test suite and parts of the repository.
305-
# Must not run in parallel with any other tests that may create or delete files.
306305
add_test(NAME lint.py COMMAND python3 lint.py WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}")
307-
set_tests_properties(lint.py PROPERTIES RUN_SERIAL TRUE)
308306

309307
add_test_pile(../doc/examples *.lean)
310308
add_test_pile(compile *.lean BENCH PART2)

tests/lint.py

Lines changed: 66 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,44 @@
11
#!/usr/bin/env python3
22

3+
import glob as glob_
34
import os
5+
import re
6+
import subprocess
47
import sys
58
from pathlib import Path
69

710
# Run in repo root
811
os.chdir(Path(__file__).parent.parent)
912

1013

14+
# We only inspect files tracked by git, and those should never be modified by
15+
# tests, so this should safely run in parallel with other tests.
16+
TRACKED_LIST = sorted(
17+
name
18+
for name in subprocess.run(
19+
["git", "ls-files", "-z"],
20+
capture_output=True,
21+
text=True,
22+
check=True,
23+
).stdout.split("\0")
24+
if name
25+
)
26+
TRACKED_SET = {Path(name) for name in TRACKED_LIST}
27+
28+
29+
def glob(*patterns: str):
30+
# `Path.full_match` recompiles the pattern on every call, which makes this
31+
# script a lot slower than it needs to be.
32+
compiled = [
33+
re.compile(glob_.translate(pattern, recursive=True, include_hidden=True))
34+
for pattern in patterns
35+
]
36+
37+
for name in TRACKED_LIST:
38+
if any(regex.match(name) for regex in compiled):
39+
yield Path(name)
40+
41+
1142
ERROR = False
1243

1344

@@ -20,7 +51,7 @@ def nag(reason: str, path: Path, fatal: bool = True) -> None:
2051

2152
# Files and directories that will no longer work.
2253

23-
for pattern in (
54+
for file in glob(
2455
"**/*.exit.expected",
2556
"**/*.expected.out",
2657
"**/*.expected.ret",
@@ -29,32 +60,24 @@ def nag(reason: str, path: Path, fatal: bool = True) -> None:
2960
"**/run_test",
3061
"tests/speedcenter.exec.velcom.yaml",
3162
):
32-
for file in Path().glob(pattern):
33-
nag("removed file", file)
34-
35-
for dir in (
36-
"tests/bench-radar",
37-
"tests/bench/cbv",
38-
"tests/bench/inundation",
39-
"tests/compiler",
40-
"tests/lean/docparse",
41-
"tests/lean/interactive",
42-
"tests/lean/run",
43-
"tests/lean/server",
44-
"tests/lean/trust0",
45-
"tests/plugin",
63+
nag("removed file", file)
64+
65+
for file in glob(
66+
"tests/bench-radar/*",
67+
"tests/bench/cbv/*",
68+
"tests/bench/inundation/*",
69+
"tests/compiler/*",
70+
"tests/lean/docparse/*",
71+
"tests/lean/interactive/*",
72+
"tests/lean/run/*",
73+
"tests/lean/server/*",
74+
"tests/lean/trust0/*",
75+
"tests/plugin/*",
76+
"tests/lean/*.lean",
77+
"tests/lean/*.expected.out",
78+
"tests/lean/*.expected.ret",
4679
):
47-
for file in Path().glob(f"{dir}/*"):
48-
nag("removed dir", file)
49-
50-
for dir in ("tests/lean",):
51-
for pattern in (
52-
f"{dir}/*.lean",
53-
f"{dir}/*.expected.out",
54-
f"{dir}/*.expected.ret",
55-
):
56-
for file in Path().glob(pattern):
57-
nag("removed dir", file)
80+
nag("removed dir", file)
5881

5982

6083
# Files that use the old naming convention in the new directories.
@@ -72,13 +95,12 @@ def nag(reason: str, path: Path, fatal: bool = True) -> None:
7295
"tests/server",
7396
"tests/server_interactive",
7497
):
75-
for pattern in (
98+
for file in glob(
7699
f"{dir}/*.no_interpreter",
77100
f"{dir}/*.expected.out",
78101
f"{dir}/*.expected.ret",
79102
):
80-
for file in Path().glob(pattern):
81-
nag("old suffix", file)
103+
nag("old suffix", file)
82104

83105

84106
# Files that expect a corresponding base file
@@ -106,11 +128,11 @@ def nag(reason: str, path: Path, fatal: bool = True) -> None:
106128
("**/*.out.expected", 2),
107129
("**/*.out.ignored", 2),
108130
):
109-
for file in Path().glob(pattern):
131+
for file in glob(pattern):
110132
basefile = file
111133
for _ in range(drop):
112134
basefile = basefile.with_suffix("")
113-
if basefile.exists():
135+
if basefile in TRACKED_SET:
114136
continue
115137
if basefile == Path(
116138
"tests/pkg/leanchecker/LeanCheckerTests/PrivateConflictC.lean.fresh"
@@ -121,54 +143,52 @@ def nag(reason: str, path: Path, fatal: bool = True) -> None:
121143

122144
# Files that shouldn't be empty
123145

124-
for pattern in (
146+
for file in glob(
125147
"**/*.init.sh",
126148
"**/*.before.sh",
127149
"**/*.after.sh",
128150
):
129-
for file in Path().glob(pattern):
130-
if file.read_text().strip():
131-
continue
132-
nag("empty", file)
151+
if file.read_text().strip():
152+
continue
153+
nag("empty", file)
133154

134155
# We need to be a bit more peculiar about whitespace here
135-
for file in Path().glob("**/*.out.expected"):
156+
for file in glob("**/*.out.expected"):
136157
if file.read_bytes():
137158
continue
138159
nag("empty", file)
139160

140161

141162
# .out.ignored and .out.expected collision
142163

143-
for file in Path().glob("**/*.out.ignored"):
144-
if file.with_suffix(".expected").exists():
164+
for file in glob("**/*.out.ignored"):
165+
if file.with_suffix(".expected") in TRACKED_SET:
145166
nag("has .expected", file)
146167

147168

148169
# .no_test but .out.expected/.out.ignored
149170

150-
for file in Path().glob("**/*.no_test"):
151-
if file.with_suffix(".out.expected").exists():
171+
for file in glob("**/*.no_test"):
172+
if file.with_suffix(".out.expected") in TRACKED_SET:
152173
nag("has .no_test", file)
153-
if file.with_suffix(".out.ignored").exists():
174+
if file.with_suffix(".out.ignored") in TRACKED_SET:
154175
nag("has .no_test", file)
155176

156177

157178
# Special rules for certain directories
158179

159-
for pattern in (
180+
for file in glob(
160181
"tests/compile_bench/*.no_bench",
161182
"tests/elab_bench/*.no_bench",
162183
"tests/misc_bench/*.no_bench",
163184
):
164-
for file in Path().glob(pattern):
165-
nag("forbidden", file)
185+
nag("forbidden", file)
166186

167187

168188
# File confusion by case insensitive filesystems
169189

170190
seen: set[str] = set()
171-
for file in Path().glob("**/*"):
191+
for file in glob("**/*"):
172192
path = str(file).lower()
173193
if path in seen:
174194
nag("case sensitive", file)

0 commit comments

Comments
 (0)