Skip to content

Commit 721397c

Browse files
committed
Prevent tests from using 'should' in sanity check.
It makes descriptions longer (which we independently check for) but also is stylistically a bit awkward. https://jml.io/pages/test-docstrings.html is a good post on the subject, with a sort of algorithm for removing this and related words. Fix a first file (anchor) where descriptions are shortened and clarified a bit. Further files will be fixed in successive commits.
1 parent 68f380c commit 721397c

File tree

4 files changed

+32
-6
lines changed

4 files changed

+32
-6
lines changed

bin/jsonschema_suite

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,32 @@ class SanityTests(unittest.TestCase):
160160
)
161161
print(f"Found {count} test cases.")
162162

163+
def test_descriptions_do_not_use_modal_verbs(self):
164+
"""
165+
Instead of saying "test that X frobs" or "X should frob" use "X frobs".
166+
167+
See e.g. https://jml.io/pages/test-docstrings.html
168+
169+
This test isn't comprehensive (it doesn't catch all the extra
170+
verbiage there), but it's just to catch whatever it manages to
171+
cover.
172+
"""
173+
174+
message = (
175+
"In descriptions, don't say 'Test that X frobs' or 'X should "
176+
"frob' or 'X should be valid'. Just say 'X frobs' or 'X is "
177+
"valid'. It's shorter, and the test suite is entirely about "
178+
"what *should* be already. "
179+
"See https://jml.io/pages/test-docstrings.html for help."
180+
)
181+
for test in tests(self.test_files):
182+
with self.subTest(description=test["description"]):
183+
self.assertNotRegex(
184+
test["description"],
185+
r"\bshould\b",
186+
message,
187+
)
188+
163189
@unittest.skipIf(jsonschema is None, "Validation library not present!")
164190
def test_all_schemas_are_valid(self):
165191
"""

tests/draft-next/anchor.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,12 @@
159159
},
160160
"tests": [
161161
{
162-
"description": "$ref should resolve to /$defs/A/allOf/1",
162+
"description": "$ref resolves to /$defs/A/allOf/1",
163163
"data": "a",
164164
"valid": true
165165
},
166166
{
167-
"description": "$ref should not resolve to /$defs/A/allOf/0",
167+
"description": "$ref does not resolve to /$defs/A/allOf/0",
168168
"data": 1,
169169
"valid": false
170170
}

tests/draft2019-09/anchor.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,12 @@
159159
},
160160
"tests": [
161161
{
162-
"description": "$ref should resolve to /$defs/A/allOf/1",
162+
"description": "$ref resolves to /$defs/A/allOf/1",
163163
"data": "a",
164164
"valid": true
165165
},
166166
{
167-
"description": "$ref should not resolve to /$defs/A/allOf/0",
167+
"description": "$ref does not resolve to /$defs/A/allOf/0",
168168
"data": 1,
169169
"valid": false
170170
}

tests/draft2020-12/anchor.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,12 @@
159159
},
160160
"tests": [
161161
{
162-
"description": "$ref should resolve to /$defs/A/allOf/1",
162+
"description": "$ref resolves to /$defs/A/allOf/1",
163163
"data": "a",
164164
"valid": true
165165
},
166166
{
167-
"description": "$ref should not resolve to /$defs/A/allOf/0",
167+
"description": "$ref does not resolve to /$defs/A/allOf/0",
168168
"data": 1,
169169
"valid": false
170170
}

0 commit comments

Comments
 (0)