Skip to content

Commit 26463f3

Browse files
redsun82Copilot
andcommitted
Just: test the tooling here, whose failures are all silent ones
Nothing under this directory had tests, and the ways it can break mostly exit zero: an exclusion that stops excluding, a verb that stops being found, an argument that arrives split in two. None of those is visible until someone notices work not being done, which is why a paired repository was carrying a redundant path pattern purely as a hedge. Each such test comes with a positive control, asserting the same setup can produce the loud outcome, so that a zero means the rule held rather than that nothing ran. Writing them turned up one crash: an argument list of nothing but blanks was counted before the blanks were dropped, leaving no root to take and an IndexError instead of the usage. Plain unittest and no new dependency, this being the layer that runs other tooling and so a poor place to need a package manager. A `test` recipe reaches them by the usual verb, and CI runs them on any change here. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 1a5de7a commit 26463f3

9 files changed

Lines changed: 929 additions & 4 deletions

File tree

‎.github/workflows/python-tooling.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ on:
55
paths:
66
- "misc/bazel/**"
77
- "misc/codegen/**"
8+
- "misc/just/**"
89
- "misc/scripts/models-as-data/*.py"
910
- "*.bazel*"
1011
- .github/workflows/codegen.yml
@@ -33,3 +34,7 @@ jobs:
3334
shell: bash
3435
run: |
3536
bazel test //misc/codegen/...
37+
- name: Run just tooling tests
38+
shell: bash
39+
run: |
40+
bazel test //misc/just/...

‎misc/just/BUILD.bazel‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
load("@rules_python//python:defs.bzl", "py_library", "py_test")
2+
3+
py_library(
4+
name = "tooling",
5+
srcs = [
6+
"codeql_test_run.py",
7+
"forward_command.py",
8+
"language_tests.py",
9+
"run_on_files.py",
10+
],
11+
imports = ["."],
12+
visibility = ["//visibility:public"],
13+
)
14+
15+
[
16+
py_test(
17+
name = src[:-len(".py")],
18+
size = "small",
19+
srcs = [src],
20+
deps = [":tooling"],
21+
)
22+
for src in glob(["test_*.py"])
23+
]
24+
25+
test_suite(
26+
name = "test",
27+
)

‎misc/just/README.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,3 +186,17 @@ With no terminal to ask — a pipe, a log, a shell without `stty` — it falls b
186186
fixed 57 columns, so logs and CI output are the same width every time. That is one
187187
branch rather than a platform test: `just` runs `sh` everywhere, so Windows takes
188188
whichever arm fits rather than a path of its own.
189+
190+
# Tests
191+
192+
The scripts here have tests, run by `just test misc/just`, by `bazel test
193+
//misc/just/...`, or by CI on any change under this directory. They are plain
194+
`unittest`: this is the layer that runs other tooling, so it should not need a package
195+
manager to check itself.
196+
197+
They exist for the class of bug that leaves no trace. An exclusion that stops excluding,
198+
a verb that stops being found, an argument that arrives split in two — each of those
199+
still exits zero, and the only symptom is work quietly not done. So the tests come in
200+
pairs: one asserting the quiet outcome, and one positive control asserting the same
201+
setup can produce the loud one. Without the second, a passing first test is also what a
202+
test that runs nothing at all looks like.

‎misc/just/justfile‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,15 @@
1+
import "defs.just"
2+
13
[no-cd]
24
[positional-arguments]
35
format *ARGS=".":
46
npx prettier --write "$@"
7+
8+
# Test the tooling in this directory.
9+
# Arguments are `unittest` ones, so a test file or a dotted test name rather than a
10+
# directory. A path given stays valid as this does not change directory: `PYTHONPATH` is
11+
# what lets the tests import the modules they are about, wherever they are run from.
12+
[no-cd]
13+
[positional-arguments]
14+
@test *ARGS=['discover', '-s', source_dir()]:
15+
PYTHONPATH="{{ source_dir() }}" {{ py }} -m unittest "$@"

‎misc/just/language_tests.py‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@
1515

1616

1717
def main():
18-
argv = sys.argv[1:]
18+
# Blank arguments are dropped before the count is taken: one comes of a caller
19+
# interpolating a variable that was never set, and a list of nothing but those is no
20+
# arguments at all rather than a root to find a justfile above.
21+
argv = [arg for arg in sys.argv[1:] if arg]
1922
if not argv:
2023
print("Usage: language_tests.py ROOT [ARG...]", file=sys.stderr)
2124
return 1
@@ -25,9 +28,7 @@ def main():
2528
# the internal checkout, so relativize them there to keep command lines readable.
2629
# Anything else (flags, environment assignments, relative paths) is passed verbatim.
2730
args = [
28-
os.path.relpath(arg, semmle_code) if os.path.isabs(arg) else arg
29-
for arg in argv
30-
if arg
31+
os.path.relpath(arg, semmle_code) if os.path.isabs(arg) else arg for arg in argv
3132
]
3233

3334
just = os.environ.get("JUST_EXECUTABLE", "just")

‎misc/just/test_codeql_test_run.py‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
#!/usr/bin/env python3
2+
"""Tests for `codeql_test_run.py`.
3+
4+
Sorting arguments is the whole of what this file decides, and it decides it by looking
5+
at each one: a word is a test, a `-` is a flag, `NAME=value` is an environment
6+
assignment. Several of these pin down that an argument arrives whole, spaces and all,
7+
which is what taking them as a list rather than re-splitting a string bought.
8+
"""
9+
10+
import os
11+
import unittest
12+
from unittest import mock
13+
14+
import codeql_test_run
15+
16+
17+
def empty_args():
18+
"""What `main` builds before sorting, limited to what `parse_args` fills in."""
19+
return {
20+
"tests": [],
21+
"flags": [],
22+
"env": [],
23+
"all_checks": [],
24+
"codeql": "host",
25+
"all": False,
26+
}
27+
28+
29+
def sorted_args(*argv):
30+
args = empty_args()
31+
codeql_test_run.parse_args(args, list(argv))
32+
return args
33+
34+
35+
class TestParseArgs(unittest.TestCase):
36+
def test_a_plain_word_is_a_test(self):
37+
self.assertEqual(sorted_args("ql/test/Foo")["tests"], ["ql/test/Foo"])
38+
39+
def test_a_dash_is_a_flag(self):
40+
self.assertEqual(sorted_args("--fail-on-trap-errors")["flags"], ["--fail-on-trap-errors"])
41+
42+
def test_an_uppercase_assignment_is_an_environment_variable(self):
43+
self.assertEqual(sorted_args("CPUS=4")["env"], ["CPUS=4"])
44+
45+
def test_a_lowercase_assignment_is_a_test(self):
46+
# Only shouting counts, so a path that happens to contain `=` stays a path.
47+
self.assertEqual(sorted_args("dir/a=b")["tests"], ["dir/a=b"])
48+
49+
def test_codeql_selects_the_executable(self):
50+
self.assertEqual(sorted_args("--codeql=built")["codeql"], "built")
51+
52+
def test_the_last_codeql_wins(self):
53+
self.assertEqual(sorted_args("--codeql=host", "--codeql=built")["codeql"], "built")
54+
55+
def test_all_checks_is_asked_for_by_either_spelling(self):
56+
self.assertTrue(sorted_args("--all-checks")["all"])
57+
self.assertTrue(sorted_args("+")["all"])
58+
59+
def test_an_extra_check_is_held_back_until_it_is_asked_for(self):
60+
held = sorted_args("--all-checks=--check-databases")
61+
self.assertEqual(held["all_checks"], ["--check-databases"])
62+
# Held back means held back: it is not a flag until `--all-checks` arrives.
63+
self.assertEqual(held["flags"], [])
64+
self.assertFalse(held["all"])
65+
66+
def test_an_empty_argument_is_ignored(self):
67+
# One of these comes of a caller interpolating a variable that was never set.
68+
self.assertEqual(sorted_args("", "test")["tests"], ["test"])
69+
70+
def test_a_test_path_containing_a_space_stays_one_argument(self):
71+
self.assertEqual(sorted_args("some dir/test")["tests"], ["some dir/test"])
72+
73+
def test_an_assignment_whose_value_contains_a_space_stays_whole(self):
74+
"""The value is the point, and a split one used to arrive as three characters.
75+
76+
An argument list carries this; a whitespace-separated string cannot, as there
77+
is nothing left in it to tell a separator from part of a value.
78+
"""
79+
self.assertEqual(sorted_args("EXTRA=a b")["env"], ["EXTRA=a b"])
80+
81+
def test_sorts_a_whole_command_line_at_once(self):
82+
args = sorted_args("-j2", "CPUS=4", "ql/test", "+", "--all-checks=--check-diff")
83+
self.assertEqual(args["flags"], ["-j2"])
84+
self.assertEqual(args["env"], ["CPUS=4"])
85+
self.assertEqual(args["tests"], ["ql/test"])
86+
self.assertEqual(args["all_checks"], ["--check-diff"])
87+
self.assertTrue(args["all"])
88+
89+
90+
class TestEnvValue(unittest.TestCase):
91+
def test_prefers_a_test_argument(self):
92+
args = sorted_args("CPUS=4")
93+
with mock.patch.dict(os.environ, {"CPUS": "8"}):
94+
self.assertEqual(codeql_test_run.env_value(args, "CPUS", "1"), "4")
95+
96+
def test_falls_back_to_the_environment(self):
97+
with mock.patch.dict(os.environ, {"CPUS": "8"}):
98+
self.assertEqual(codeql_test_run.env_value(empty_args(), "CPUS", "1"), "8")
99+
100+
def test_falls_back_to_the_default(self):
101+
with mock.patch.dict(os.environ, {}, clear=True):
102+
self.assertEqual(codeql_test_run.env_value(empty_args(), "CPUS", "1"), "1")
103+
104+
def test_the_last_assignment_wins(self):
105+
args = sorted_args("CPUS=4", "CPUS=2")
106+
self.assertEqual(codeql_test_run.env_value(args, "CPUS", "1"), "2")
107+
108+
def test_an_empty_value_does_not_count_as_a_setting(self):
109+
args = sorted_args("CPUS=")
110+
with mock.patch.dict(os.environ, {"CPUS": "8"}):
111+
self.assertEqual(codeql_test_run.env_value(args, "CPUS", "1"), "8")
112+
113+
def test_a_value_containing_a_space_survives(self):
114+
args = sorted_args("EXTRA=a b")
115+
self.assertEqual(codeql_test_run.env_value(args, "EXTRA", "none"), "a b")
116+
117+
118+
if __name__ == "__main__":
119+
unittest.main()

0 commit comments

Comments
 (0)