Skip to content

Commit 8b5cd40

Browse files
committed
refactor(tools): modernise test-runner scripts
Bring all four test-runner tools onto the same shape: - Import _common for REPO_ROOT, TOOLS_DIR, CHECK_PLUGINS_DIR, SKIP_PLUGINS, iter_check_plugins() and die(), dropping the hand-rolled `os.path.abspath(os.path.join(__file__, '..'))` discovery that each tool used to have. - Add __author__ and __version__ at the top of every tool so the -V/--version output matches the convention the rest of the repo uses. - Add the CONTRIBUTING.md header comment that the plugin scripts carry, so new contributors can navigate from any tool to the style guide. - Replace os.path string juggling with pathlib.Path wherever it makes the call site shorter. - Unify the argparse pattern: every tool that takes options has an explicit parse_args() function that also registers -V / --version alongside its real options. - Replace the duplicated SKIP_PLUGINS set in run-unit-tests with _common.SKIP_PLUGINS, and fold the plugin discovery loop onto iter_check_plugins(). - run-linter-checks: rewrite discover_python_files() to walk the two target directories with Path.rglob() instead of os.walk, dropping a level of nesting. No behaviour changes: run-unit-tests still partitions fast and container tests the same way, run-linter-checks still filters the two classes of bandit nosec-bookkeeping warnings, the --no-container / --only-container / positional plugin-name semantics are unchanged. Verified: tools/run-linter-checks runs clean (ruff, bandit, vulture); tools/run-unit-tests --no-container with a few plugins still discovers, filters and dispatches as before.
1 parent 3ea7f91 commit 8b5cd40

File tree

4 files changed

+239
-171
lines changed

4 files changed

+239
-171
lines changed

tools/run-container-tests

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,16 @@
66
# https://www.linuxfabrik.ch/
77
# License: The Unlicense, see LICENSE file.
88

9+
# https://github.com/Linuxfabrik/monitoring-plugins/blob/main/CONTRIBUTING.md
10+
911
"""Discover and run all container-based plugin unit tests.
1012
11-
A plugin's unit test counts as container-based when its `unit-test/`
12-
directory has a `containerfiles/` subdirectory. These tests build a
13-
podman image per target OS, inject the plugin plus `lib/`, and exercise
14-
the check against a live service. They take minutes per plugin and
15-
need podman on the host.
13+
A plugin's unit test counts as container-based when its `run` file
14+
imports testcontainers-python or uses one of the lib.lftest container
15+
helpers. These tests build a podman container (or pull an upstream
16+
image), inject the plugin plus `lib/`, and exercise the check against
17+
a live service. They take minutes per plugin and need podman on the
18+
host.
1619
1720
This runner is a thin wrapper around `tools/run-unit-tests
1821
--only-container` so the two entry points (fast tests and container
@@ -21,22 +24,25 @@ tests) are clearly separated. `tox` runs `tools/run-unit-tests
2124
a full integration sweep before a release.
2225
2326
Usage:
24-
python tools/run-container-tests # run all container tests
25-
python tools/run-container-tests keycloak-version # run one
27+
tools/run-container-tests # run all container tests
28+
tools/run-container-tests keycloak-version # run one plugin
2629
"""
2730

28-
import os
2931
import subprocess
3032
import sys
3133

34+
import _common
35+
36+
__author__ = 'Linuxfabrik GmbH, Zurich/Switzerland'
37+
__version__ = '2026041301'
38+
3239

3340
def main():
34-
here = os.path.dirname(os.path.abspath(__file__))
35-
runner = os.path.join(here, 'run-unit-tests')
36-
argv = [sys.executable, runner, '--only-container', *sys.argv[1:]]
41+
runner = _common.TOOLS_DIR / 'run-unit-tests'
42+
argv = [sys.executable, str(runner), '--only-container', *sys.argv[1:]]
3743
result = subprocess.run(argv, check=False)
38-
sys.exit(result.returncode)
44+
return result.returncode
3945

4046

4147
if __name__ == '__main__':
42-
main()
48+
sys.exit(main())

tools/run-linter-checks

Lines changed: 113 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -6,146 +6,174 @@
66
# https://www.linuxfabrik.ch/
77
# License: The Unlicense, see LICENSE file.
88

9+
# https://github.com/Linuxfabrik/monitoring-plugins/blob/main/CONTRIBUTING.md
10+
911
"""Run lint and security checks (ruff, bandit, vulture) over the whole repo.
1012
11-
Pre-commit hooks only run on staged files. This script sweeps every plugin
12-
script, every tool, and every file under lib/ so that long-standing issues
13-
cannot hide simply because nobody has touched the file in a while.
13+
Pre-commit hooks only run on staged files. This script sweeps every
14+
plugin script, every tool and every Python file under the repo so
15+
that long-standing issues cannot hide simply because nobody has
16+
touched the file in a while.
1417
1518
pylint is intentionally NOT invoked here. Its metric-based checks
16-
(R0801 duplicate lines, R09xx complexity, etc.) produce thousands of
17-
false positives across a collection of check plugins that all share
18-
the same boilerplate, and suppressing the noise with a disable list
19-
would go against the house rule of running pylint without `--disable`.
20-
Run pylint by hand when you want to audit a single plugin.
19+
(R0801 duplicate lines, R09xx complexity, etc.) produce thousands
20+
of false positives across a collection of check plugins that all
21+
share the same boilerplate, and suppressing the noise with a
22+
disable list would go against the house rule of running pylint
23+
without `--disable`. Run pylint by hand when you want to audit a
24+
single plugin.
2125
22-
Plugin scripts use a shebang instead of a `.py` extension, so the script
23-
discovers them by `#!/usr/bin/env python` header and passes them explicitly
24-
to each analyzer.
26+
Plugin scripts use a shebang instead of a `.py` extension, so the
27+
script discovers them by looking for a `#!/usr/bin/env python`
28+
header and passes them explicitly to each analyzer.
2529
2630
Usage:
27-
tools/run-linter-checks # run all analyzers
28-
tools/run-linter-checks --only=ruff # run only ruff
31+
tools/run-linter-checks # run all analyzers
32+
tools/run-linter-checks --only=ruff # run only ruff
2933
tools/run-linter-checks --only=ruff,bandit
3034
"""
3135

3236
import argparse
33-
import os
3437
import re
3538
import subprocess
3639
import sys
3740

38-
REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))
41+
import _common
42+
43+
__author__ = 'Linuxfabrik GmbH, Zurich/Switzerland'
44+
__version__ = '2026041301'
3945

40-
# bandit emits two classes of stderr WARNING messages that come from its
41-
# internal `# nosec` bookkeeping and do not correspond to any actual
42-
# security finding. Both look alarming in the wrapper's output but cannot
43-
# be fixed at the source level without rewriting otherwise-correct code:
46+
47+
# bandit emits two classes of stderr WARNING messages that come
48+
# from its internal `# nosec` bookkeeping and do not correspond to
49+
# any actual security finding. Both look alarming in the wrapper's
50+
# output but cannot be fixed at the source level without rewriting
51+
# otherwise-correct code:
4452
#
45-
# 1. `nosec encountered (BXXX), but no failed test` - bandit runs each
46-
# test against every AST node on a given line, not just the node
47-
# that would actually produce the finding. When several sub-nodes
48-
# share a line and only one of them triggers the test, bandit sees
49-
# the `# nosec BXXX` comment from the context of the other sub-nodes
50-
# too and warns that the nosec "was never used", even though it was
51-
# used by the one sub-node that mattered.
53+
# 1. `nosec encountered (BXXX), but no failed test` - bandit
54+
# runs each test against every AST node on a given line, not
55+
# just the node that would actually produce the finding. When
56+
# several sub-nodes share a line and only one of them
57+
# triggers the test, bandit sees the `# nosec BXXX` comment
58+
# from the context of the other sub-nodes too and warns that
59+
# the nosec "was never used", even though it was used by the
60+
# one sub-node that mattered.
5261
#
53-
# 2. `Test in comment: X is not a test name or id, ignoring` - bandit
54-
# tokenizes every word after `# nosec` and tries to match each one
55-
# against its test id catalog. Free-text explanations on the same
56-
# line (e.g. `# nosec B105 - Keycloak install default, ...`) trip
57-
# this. We keep source-level comments free of prose after `# nosec`
58-
# by convention, but upstream libs occasionally slip through.
62+
# 2. `Test in comment: X is not a test name or id, ignoring` -
63+
# bandit tokenizes every word after `# nosec` and tries to
64+
# match each one against its test id catalog. Free-text
65+
# explanations on the same line trip this. We keep source-
66+
# level comments free of prose after `# nosec` by convention,
67+
# but upstream libs occasionally slip through.
5968
#
60-
# Both warnings are logged at WARNING level on stderr and do not affect
61-
# bandit's exit code. We filter them here so the linter sweep stays
62-
# readable; real security issues still surface because bandit is run at
63-
# `--severity-level=high --confidence-level=high` and would exit non-zero
64-
# on any actual finding.
69+
# Both warnings are logged at WARNING level on stderr and do not
70+
# affect bandit's exit code. We filter them here so the linter
71+
# sweep stays readable; real security issues still surface because
72+
# bandit runs at `--severity-level=high --confidence-level=high`
73+
# and would exit non-zero on any actual finding.
6574
BANDIT_NOISE_PATTERNS = (
6675
re.compile(
6776
r'^\[tester\]\s+WARNING\s+nosec encountered \(B\d+\), '
68-
r'but no failed test on file'
77+
r'but no failed test on file',
6978
),
7079
re.compile(
7180
r'^\[manager\]\s+WARNING\s+Test in comment: .* '
72-
r'is not a test name or id, ignoring'
81+
r'is not a test name or id, ignoring',
7382
),
7483
)
7584

7685

77-
def _bandit_line_is_noise(line):
86+
def bandit_line_is_noise(line):
7887
return any(pattern.search(line) for pattern in BANDIT_NOISE_PATTERNS)
7988

8089

8190
def discover_python_files():
8291
"""Discover every Python file in the repo.
8392
84-
Includes shebang-prefixed plugin scripts under check-plugins/ and tools/
85-
as well as *.py files under tools/. The shared `lib` library lives in
86-
a separate repository and is not scanned here.
93+
Includes shebang-prefixed plugin scripts under `check-plugins/`
94+
and `tools/` as well as `*.py` files under `tools/`. The shared
95+
`lib` library lives in a separate repository and is not scanned
96+
here.
8797
"""
8898
python_files = []
89-
for walk_root in ('check-plugins', 'tools'):
90-
full_walk_root = os.path.join(REPO_ROOT, walk_root)
91-
for dirpath, dirnames, filenames in os.walk(full_walk_root):
92-
dirnames[:] = [d for d in dirnames if d not in ('__pycache__', '.git')]
93-
for filename in filenames:
94-
path = os.path.join(dirpath, filename)
95-
if filename.endswith('.py'):
96-
python_files.append(path)
97-
continue
98-
if not os.access(path, os.X_OK):
99-
continue
100-
try:
101-
with open(path, encoding='utf-8', errors='ignore') as f:
102-
first_line = f.readline()
103-
except OSError:
104-
continue
105-
if first_line.startswith('#!') and 'python' in first_line:
106-
python_files.append(path)
99+
for walk_root in (_common.CHECK_PLUGINS_DIR, _common.TOOLS_DIR):
100+
for path in walk_root.rglob('*'):
101+
if any(part in {'__pycache__', '.git'} for part in path.parts):
102+
continue
103+
if not path.is_file():
104+
continue
105+
if path.suffix == '.py':
106+
python_files.append(path)
107+
continue
108+
if not path.stat().st_mode & 0o111:
109+
continue
110+
try:
111+
first_line = path.open(encoding='utf-8', errors='ignore').readline()
112+
except OSError:
113+
continue
114+
if first_line.startswith('#!') and 'python' in first_line:
115+
python_files.append(path)
107116
return sorted(python_files)
108117

109118

110-
def run(analyzer, argv, stderr_filter=None):
111-
print(f'\n=== {analyzer} ===', flush=True)
119+
def run_analyzer(name, argv, stderr_filter=None):
120+
"""Run a single analyzer, print a pass/fail line, return its exit code."""
121+
print(f'\n=== {name} ===', flush=True)
112122
try:
113123
if stderr_filter is not None:
114124
result = subprocess.run(
115125
argv,
116-
cwd=REPO_ROOT,
126+
cwd=_common.REPO_ROOT,
117127
check=False,
118128
stderr=subprocess.PIPE,
119129
text=True,
120130
)
121131
for line in result.stderr.splitlines():
122132
if not stderr_filter(line):
123-
sys.stderr.write(line + '\n')
124-
sys.stderr.flush()
133+
_common.err(line)
125134
else:
126-
result = subprocess.run(argv, cwd=REPO_ROOT, check=False)
135+
result = subprocess.run(argv, cwd=_common.REPO_ROOT, check=False)
127136
except FileNotFoundError:
128-
print(f'{analyzer}: not installed, skipping')
137+
print(f'{name}: not installed, skipping')
129138
return 0
139+
130140
# Normalize the end-of-run signal so every analyzer prints the
131-
# same pass/fail line. ruff is run with `--quiet` (see below) so
132-
# its own "All checks passed!" message does not fire and we avoid
133-
# a duplicate line. bandit and vulture stay silent on a clean run.
141+
# same pass/fail line. ruff is run with `--quiet` so its own
142+
# "All checks passed!" message does not fire and we avoid a
143+
# duplicate line. bandit and vulture stay silent on a clean run.
134144
if result.returncode == 0:
135145
print('All checks passed!', flush=True)
136146
else:
137147
print(f'Issues found (exit {result.returncode}).', flush=True)
138148
return result.returncode
139149

140150

141-
def main():
142-
parser = argparse.ArgumentParser(description=__doc__)
151+
def parse_args():
152+
"""Parse command line arguments using argparse."""
153+
parser = argparse.ArgumentParser(
154+
description=__doc__.strip().split('\n\n')[0],
155+
)
156+
157+
parser.add_argument(
158+
'-V',
159+
'--version',
160+
action='version',
161+
version=f'%(prog)s: v{__version__} by {__author__}',
162+
)
163+
143164
parser.add_argument(
144165
'--only',
145-
help='Comma-separated list of analyzers to run (ruff, bandit, vulture)',
166+
help='Comma-separated list of analyzers to run '
167+
'(ruff, bandit, vulture). Default: all three.',
146168
default='',
147169
)
148-
args = parser.parse_args()
170+
171+
return parser.parse_args()
172+
173+
174+
def main():
175+
"""Discover Python files and run each analyzer over them."""
176+
args = parse_args()
149177

150178
selected = set(args.only.split(',')) if args.only else None
151179

@@ -156,28 +184,32 @@ def main():
156184
print(f'Found {len(python_files)} Python files.')
157185

158186
exit_code = 0
187+
file_args = [str(p) for p in python_files]
159188

160189
if selected is None or 'ruff' in selected:
161-
exit_code |= run(
190+
exit_code |= run_analyzer(
162191
'ruff',
163-
['ruff', 'check', '--no-fix', '--quiet', *python_files],
192+
['ruff', 'check', '--no-fix', '--quiet', *file_args],
164193
)
165194

166195
if selected is None or 'bandit' in selected:
167-
exit_code |= run(
196+
exit_code |= run_analyzer(
168197
'bandit',
169198
[
170199
'bandit',
171200
'-q',
172201
'--severity-level=high',
173202
'--confidence-level=high',
174-
*python_files,
203+
*file_args,
175204
],
176-
stderr_filter=_bandit_line_is_noise,
205+
stderr_filter=bandit_line_is_noise,
177206
)
178207

179208
if selected is None or 'vulture' in selected:
180-
exit_code |= run('vulture', ['vulture', '--min-confidence=80', *python_files])
209+
exit_code |= run_analyzer(
210+
'vulture',
211+
['vulture', '--min-confidence=80', *file_args],
212+
)
181213

182214
return 0 if exit_code == 0 else 1
183215

tools/run-tox-tests

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
# https://www.linuxfabrik.ch/
77
# License: The Unlicense, see LICENSE file.
88

9+
# https://github.com/Linuxfabrik/monitoring-plugins/blob/main/CONTRIBUTING.md
10+
911
"""Run unit tests across every supported Python version via tox.
1012
1113
This is a thin wrapper around `tox` that lives in `tools/` so the
@@ -30,23 +32,27 @@ against the currently activated venv; use this wrapper when you
3032
want the multi-Python sweep that catches interpreter-specific bugs.
3133
"""
3234

33-
import os
3435
import subprocess
3536
import sys
3637

37-
REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))
38+
import _common
39+
40+
__author__ = 'Linuxfabrik GmbH, Zurich/Switzerland'
41+
__version__ = '2026041301'
3842

3943

4044
def main():
4145
try:
42-
result = subprocess.run(['tox', *sys.argv[1:]], cwd=REPO_ROOT, check=False)
46+
result = subprocess.run(
47+
['tox', *sys.argv[1:]],
48+
cwd=_common.REPO_ROOT,
49+
check=False,
50+
)
4351
except FileNotFoundError:
44-
print(
52+
_common.die(
4553
'tox is not installed. Install it with `pip install tox` '
4654
'(or your distribution package).',
47-
file=sys.stderr,
4855
)
49-
return 1
5056
return result.returncode
5157

5258

0 commit comments

Comments
 (0)