Skip to content

Commit a50585e

Browse files
redsun82Copilot
andcommitted
Just: test settings where they are now resolved
`env_value` is gone and `parse_arguments` reads `sys.argv` itself, so every test that called either of them stopped running. Precedence is now decided in `main`, where an assignment and an inherited variable first meet, so the tests about precedence go through `main` rather than through a function that no longer exists. A later empty value now erases an earlier setting instead of being skipped. That is only about what this script reads back: every assignment is still exported, so an empty one reaches the child set and empty rather than absent. Both halves are pinned, since the first is easy to mistake for the second. Empty arguments are dropped before parsing rather than while sorting. Only the `--` case tells the two placements apart, so that is the test worth having. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent fa5b782 commit a50585e

1 file changed

Lines changed: 118 additions & 33 deletions

File tree

‎misc/just/test_codeql_test_run.py‎

Lines changed: 118 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,61 @@
55
at each one: a word is a test, a `-` is a flag, `NAME=value` is an environment
66
assignment. Several of these pin down that an argument arrives whole, spaces and all,
77
which is what taking them as a list rather than re-splitting a string bought.
8+
9+
Sorting and resolving are tested at different levels because they happen at different
10+
levels. `parse_arguments` only sorts; a setting is not resolved until `main` has merged
11+
the assignments into the environment, so anything about precedence is observed there.
812
"""
913

1014
import os
15+
import sys
1116
import unittest
1217
from unittest import mock
1318

1419
import codeql_test_run
1520

1621

17-
def empty_args():
18-
"""A command line carrying nothing but the language `main` reads off the front."""
19-
return sorted_args()
22+
def sorted_args(*argv, semmle_code=None):
23+
"""Sort a command line, supplying the language that always precedes it."""
24+
with (
25+
mock.patch.object(codeql_test_run, "SEMMLE_CODE", semmle_code),
26+
mock.patch.object(sys, "argv", ["codeql_test_run.py", "alanguage", *argv]),
27+
):
28+
return codeql_test_run.parse_arguments()
2029

2130

22-
def sorted_args(*argv):
23-
"""Sort a command line, supplying the language that always precedes it."""
24-
with mock.patch.object(codeql_test_run, "SEMMLE_CODE", None):
25-
return codeql_test_run.parse_arguments(["alanguage", *argv])
31+
def run_main(*argv, environ=None):
32+
"""Run `main` with the executable and the child process stubbed out.
33+
34+
Returns the flags and tests handed to `codeql test run`, and the environment the
35+
child would have been given.
36+
"""
37+
codeql = mock.MagicMock()
38+
codeql.exists.return_value = True
39+
with (
40+
mock.patch.object(codeql_test_run, "SEMMLE_CODE", None),
41+
mock.patch.object(sys, "argv", ["codeql_test_run.py", "alanguage", *argv]),
42+
mock.patch.dict(os.environ, environ or {}, clear=True),
43+
mock.patch.object(codeql_test_run, "resolve_codeql", return_value=codeql),
44+
mock.patch.object(codeql_test_run, "invoke", return_value=0) as invoke,
45+
):
46+
codeql_test_run.main()
47+
(invocation,) = invoke.call_args.args
48+
separator = invocation.index("--")
49+
# Past the executable and `test run`, up to the separator `main` adds itself.
50+
return invocation[3:separator], invocation[separator + 1 :], dict(os.environ)
51+
52+
53+
def flags(*argv, environ=None):
54+
return run_main(*argv, environ=environ)[0]
55+
56+
57+
def paths(*argv, environ=None):
58+
return run_main(*argv, environ=environ)[1]
59+
60+
61+
def child_environ(*argv, environ=None):
62+
return run_main(*argv, environ=environ)[2]
2663

2764

2865
class TestParseArgs(unittest.TestCase):
@@ -35,17 +72,21 @@ def test_a_dash_is_a_flag(self):
3572
)
3673

3774
def test_an_uppercase_assignment_is_an_environment_variable(self):
38-
self.assertEqual(sorted_args("CPUS=4").env, ["CPUS=4"])
75+
self.assertEqual(sorted_args("CPUS=4").env, {"CPUS": "4"})
3976

4077
def test_a_lowercase_assignment_is_a_test(self):
4178
# Only shouting counts, so a path that happens to contain `=` stays a path.
4279
self.assertEqual(sorted_args("dir/a=b").tests, ["dir/a=b"])
4380

4481
def test_codeql_selects_the_executable(self):
45-
self.assertEqual(sorted_args("--codeql=built").codeql, "built")
82+
# `built` is rejected outright without an internal checkout to build in, so
83+
# this says what it means to sort the option, not to act on it.
84+
args = sorted_args("--codeql=built", semmle_code="/somewhere")
85+
self.assertEqual(args.codeql, "built")
4686

4787
def test_the_last_codeql_wins(self):
48-
self.assertEqual(sorted_args("--codeql=host", "--codeql=built").codeql, "built")
88+
args = sorted_args("--codeql=host", "--codeql=built", semmle_code="/somewhere")
89+
self.assertEqual(args.codeql, "built")
4990

5091
def test_all_checks_is_asked_for_by_either_spelling(self):
5192
self.assertTrue(sorted_args("--all-checks").all)
@@ -66,10 +107,6 @@ def test_a_double_dash_hands_everything_after_it_to_codeql(self):
66107
self.assertIn("--codeql=built", args.flags)
67108
self.assertNotIn("--", args.flags)
68109

69-
def test_an_empty_argument_is_ignored(self):
70-
# One of these comes of a caller interpolating a variable that was never set.
71-
self.assertEqual(sorted_args("", "test").tests, ["test"])
72-
73110
def test_a_test_path_containing_a_space_stays_one_argument(self):
74111
self.assertEqual(sorted_args("some dir/test").tests, ["some dir/test"])
75112

@@ -79,45 +116,93 @@ def test_an_assignment_whose_value_contains_a_space_stays_whole(self):
79116
An argument list carries this; a whitespace-separated string cannot, as there
80117
is nothing left in it to tell a separator from part of a value.
81118
"""
82-
self.assertEqual(sorted_args("EXTRA=a b").env, ["EXTRA=a b"])
119+
self.assertEqual(sorted_args("EXTRA=a b").env, {"EXTRA": "a b"})
83120

84121
def test_sorts_a_whole_command_line_at_once(self):
85122
args = sorted_args(
86123
"-j2", "CPUS=4", "ql/test", "+", "--extra-check=--check-diff"
87124
)
88125
self.assertEqual(args.flags, ["-j2"])
89-
self.assertEqual(args.env, ["CPUS=4"])
126+
self.assertEqual(args.env, {"CPUS": "4"})
90127
self.assertEqual(args.tests, ["ql/test"])
91128
self.assertEqual(args.extra_checks, ["--check-diff"])
92129
self.assertTrue(args.all)
93130

94131

95-
class TestEnvValue(unittest.TestCase):
132+
class TestEmptyArguments(unittest.TestCase):
133+
"""Dropped by `main` before parsing, which is earlier than it looks.
134+
135+
Filtering these inside the sorting loop instead would leave them in front of
136+
`argparse`, and an empty argument ahead of a `--` stops the separator being
137+
recognised, so the `--` would reach `codeql test run` as an argument of its own.
138+
"""
139+
140+
def test_an_empty_argument_is_ignored(self):
141+
# One of these comes of a caller interpolating a variable that was never set.
142+
self.assertEqual(paths("", "some/test"), ["some/test"])
143+
144+
def test_an_empty_argument_does_not_disturb_a_separator(self):
145+
self.assertEqual(flags("", "--", "--check-databases")[-1], "--check-databases")
146+
self.assertNotIn("--", flags("", "--", "--check-databases"))
147+
148+
149+
class TestSettings(unittest.TestCase):
150+
"""`RAM_PER_THREAD` and `CPUS` are read back after assignments are applied.
151+
152+
Resolution is what these pin down, so they go through `main`: an assignment and an
153+
inherited variable only meet once `main` has merged them.
154+
"""
155+
96156
def test_prefers_a_test_argument(self):
97-
args = sorted_args("CPUS=4")
98-
with mock.patch.dict(os.environ, {"CPUS": "8"}):
99-
self.assertEqual(codeql_test_run.env_value(args, "CPUS", "1"), "4")
157+
self.assertIn("-j4", flags("CPUS=4", environ={"CPUS": "8"}))
100158

101159
def test_falls_back_to_the_environment(self):
102-
with mock.patch.dict(os.environ, {"CPUS": "8"}):
103-
self.assertEqual(codeql_test_run.env_value(empty_args(), "CPUS", "1"), "8")
160+
self.assertIn("-j8", flags(environ={"CPUS": "8"}))
104161

105162
def test_falls_back_to_the_default(self):
106-
with mock.patch.dict(os.environ, {}, clear=True):
107-
self.assertEqual(codeql_test_run.env_value(empty_args(), "CPUS", "1"), "1")
163+
self.assertIn(f"-j{os.cpu_count()}", flags())
108164

109165
def test_the_last_assignment_wins(self):
110-
args = sorted_args("CPUS=4", "CPUS=2")
111-
self.assertEqual(codeql_test_run.env_value(args, "CPUS", "1"), "2")
166+
self.assertIn("-j2", flags("CPUS=4", "CPUS=2"))
167+
168+
def test_an_empty_value_falls_back_to_the_default(self):
169+
"""An empty value does not override, so a later one erases an earlier setting.
170+
171+
Compared against a run that never mentions the setting, so this pins the
172+
behaviour without restating what the default happens to be. It is about
173+
resolution only: see below for what the child is given.
174+
"""
175+
self.assertEqual(flags("CPUS=4", "CPUS="), flags())
112176

113-
def test_an_empty_value_does_not_count_as_a_setting(self):
114-
args = sorted_args("CPUS=")
115-
with mock.patch.dict(os.environ, {"CPUS": "8"}):
116-
self.assertEqual(codeql_test_run.env_value(args, "CPUS", "1"), "8")
177+
def test_an_empty_assignment_still_reaches_the_child(self):
178+
"""Falling back to the default is not the same as the assignment being dropped.
117179
118-
def test_a_value_containing_a_space_survives(self):
119-
args = sorted_args("EXTRA=a b")
120-
self.assertEqual(codeql_test_run.env_value(args, "EXTRA", "none"), "a b")
180+
`RAM_PER_THREAD` and `CPUS` are read back out of the environment, so an empty
181+
one reads as unset and the default stands. Every assignment is exported either
182+
way, so the child sees the variable set and empty rather than absent, and a
183+
variable this script does not read has no other behaviour to fall back to.
184+
"""
185+
self.assertEqual(child_environ("CPUS=")["CPUS"], "")
186+
self.assertNotIn("CPUS", child_environ())
187+
188+
def test_an_assignment_reaches_the_child(self):
189+
self.assertEqual(child_environ("EXTRA=a b")["EXTRA"], "a b")
190+
191+
def test_ram_is_per_thread(self):
192+
self.assertIn("--ram=200", flags("CPUS=2", "RAM_PER_THREAD=100"))
193+
194+
195+
class TestDefaults(unittest.TestCase):
196+
def test_the_current_directory_is_the_default_test(self):
197+
self.assertEqual(paths(), ["."])
198+
199+
def test_a_named_test_replaces_the_default(self):
200+
self.assertEqual(paths("ql/test/Foo"), ["ql/test/Foo"])
201+
202+
def test_an_offered_check_becomes_a_flag_once_asked_for(self):
203+
self.assertIn(
204+
"--check-databases", flags("--extra-check=--check-databases", "+")
205+
)
121206

122207

123208
if __name__ == "__main__":

0 commit comments

Comments
 (0)