Skip to content

Commit f827640

Browse files
JulianJulian
authored
Merge pull request #579 from json-schema-org/sanity-check-strengthening
Strengthen the sanity checks, and fix some minor style or bugs uncovered
2 parents 2782d7c + e5d5e0a commit f827640

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

60 files changed

+242
-266
lines changed

‎bin/jsonschema_suite

Lines changed: 101 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@ def files(paths):
4040
Each test file in the provided paths, as an array of test cases.
4141
"""
4242
for path in paths:
43-
yield json.loads(path.read_text())
43+
yield path, json.loads(path.read_text())
4444

4545

4646
def cases(paths):
4747
"""
4848
Each test case within each file in the provided paths.
4949
"""
50-
for test_file in files(paths):
50+
for _, test_file in files(paths):
5151
yield from test_file
5252

5353

@@ -82,52 +82,115 @@ class SanityTests(unittest.TestCase):
8282
assert cls.remote_files, "Didn't find the remote files!"
8383
print(f"Found {len(cls.remote_files)} remote files")
8484

85+
def assertUnique(self, iterable):
86+
"""
87+
Assert that the elements of an iterable are unique.
88+
"""
89+
90+
seen, duplicated = set(), set()
91+
for each in iterable:
92+
if each in seen:
93+
duplicated.add(each)
94+
seen.add(each)
95+
self.assertFalse(duplicated, "Elements are not unique.")
96+
8597
def test_all_test_files_are_valid_json(self):
8698
"""
8799
All test files contain valid JSON.
88100
"""
89101
for path in self.test_files:
90-
try:
91-
json.loads(path.read_text())
92-
except ValueError as error:
93-
self.fail(f"{path} contains invalid JSON ({error})")
102+
with self.subTest(path=path):
103+
try:
104+
json.loads(path.read_text())
105+
except ValueError as error:
106+
self.fail(f"{path} contains invalid JSON ({error})")
94107

95108
def test_all_remote_files_are_valid_json(self):
96109
"""
97110
All remote files contain valid JSON.
98111
"""
99112
for path in self.remote_files:
100-
try:
101-
json.loads(path.read_text())
102-
except ValueError as error:
103-
self.fail(f"{path} contains invalid JSON ({error})")
113+
with self.subTest(path=path):
114+
try:
115+
json.loads(path.read_text())
116+
except ValueError as error:
117+
self.fail(f"{path} contains invalid JSON ({error})")
104118

105-
def test_all_descriptions_have_reasonable_length(self):
119+
def test_all_case_descriptions_have_reasonable_length(self):
120+
"""
121+
All cases have reasonably long descriptions.
122+
"""
123+
for case in cases(self.test_files):
124+
with self.subTest(description=case["description"]):
125+
self.assertLess(
126+
len(case["description"]),
127+
150,
128+
"Description is too long (keep it to less than 150 chars)."
129+
)
130+
131+
def test_all_test_descriptions_have_reasonable_length(self):
106132
"""
107133
All tests have reasonably long descriptions.
108134
"""
109135
for count, test in enumerate(tests(self.test_files)):
110-
description = test["description"]
111-
self.assertLess(
112-
len(description),
113-
70,
114-
f"{description!r} is too long! (keep it to less than 70 chars)"
115-
)
136+
with self.subTest(description=test["description"]):
137+
self.assertLess(
138+
len(test["description"]),
139+
70,
140+
"Description is too long (keep it to less than 70 chars)."
141+
)
116142
print(f"Found {count} tests.")
117143

118-
def test_all_descriptions_are_unique(self):
144+
def test_all_case_descriptions_are_unique(self):
145+
"""
146+
All cases have unique descriptions in their files.
147+
"""
148+
for path, cases in files(self.test_files):
149+
with self.subTest(path=path):
150+
self.assertUnique(case["description"] for case in cases)
151+
152+
def test_all_test_descriptions_are_unique(self):
119153
"""
120154
All test cases have unique test descriptions in their tests.
121155
"""
122156
for count, case in enumerate(cases(self.test_files)):
123-
descriptions = set(test["description"] for test in case["tests"])
124-
self.assertEqual(
125-
len(descriptions),
126-
len(case["tests"]),
127-
f"{case!r} contains a duplicate description",
128-
)
157+
with self.subTest(description=case["description"]):
158+
self.assertUnique(
159+
test["description"] for test in case["tests"]
160+
)
129161
print(f"Found {count} test cases.")
130162

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+
self.assertNotRegex(
189+
test["description"],
190+
r"(?i)\btest(s)? that\b",
191+
message,
192+
)
193+
131194
@unittest.skipIf(jsonschema is None, "Validation library not present!")
132195
def test_all_schemas_are_valid(self):
133196
"""
@@ -141,12 +204,14 @@ class SanityTests(unittest.TestCase):
141204
if Validator is not None:
142205
test_files = collect(version)
143206
for case in cases(test_files):
144-
try:
145-
Validator.check_schema(case["schema"])
146-
except jsonschema.SchemaError as error:
147-
self.fail(
148-
f"{case} contains an invalid schema ({error})",
149-
)
207+
with self.subTest(case=case):
208+
try:
209+
Validator.check_schema(case["schema"])
210+
except jsonschema.SchemaError:
211+
self.fail(
212+
"Found an invalid schema."
213+
"See the traceback for details on why."
214+
)
150215
else:
151216
warnings.warn(f"No schema validator for {version.name}")
152217

@@ -157,11 +222,12 @@ class SanityTests(unittest.TestCase):
157222
"""
158223
Validator = jsonschema.validators.validator_for(TESTSUITE_SCHEMA)
159224
validator = Validator(TESTSUITE_SCHEMA)
160-
for tests in files(self.test_files):
161-
try:
162-
validator.validate(tests)
163-
except jsonschema.ValidationError as error:
164-
self.fail(str(error))
225+
for path, cases in files(self.test_files):
226+
with self.subTest(path=path):
227+
try:
228+
validator.validate(cases)
229+
except jsonschema.ValidationError as error:
230+
self.fail(str(error))
165231

166232

167233
def main(arguments):

‎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/draft-next/dynamicRef.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@
374374
]
375375
},
376376
{
377-
"description": "Tests for implementation dynamic anchor and reference link. Reference should be independent of any possible ordering.",
377+
"description": "$ref and $dynamicAnchor are independent of order - $defs first",
378378
"schema": {
379379
"$id": "http://localhost:1234/strict-extendible-allof-defs-first.json",
380380
"allOf": [
@@ -424,7 +424,7 @@
424424
]
425425
},
426426
{
427-
"description": "Tests for implementation dynamic anchor and reference link. Reference should be independent of any possible ordering.",
427+
"description": "$ref and $dynamicAnchor are independent of order - $ref first",
428428
"schema": {
429429
"$id": "http://localhost:1234/strict-extendible-allof-ref-first.json",
430430
"allOf": [

‎tests/draft-next/items.json

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@
214214
]
215215
},
216216
{
217-
"description": "items should not look in applicators, valid case",
217+
"description": "items does not look in applicators, valid case",
218218
"schema": {
219219
"allOf": [
220220
{ "prefixItems": [ { "minimum": 3 } ] }
@@ -223,12 +223,12 @@
223223
},
224224
"tests": [
225225
{
226-
"description": "prefixItems in allOf should not constrain items, invalid case",
226+
"description": "prefixItems in allOf does not constrain items, invalid case",
227227
"data": [ 3, 5 ],
228228
"valid": false
229229
},
230230
{
231-
"description": "prefixItems in allOf should not constrain items, valid case",
231+
"description": "prefixItems in allOf does not constrain items, valid case",
232232
"data": [ 5, 5 ],
233233
"valid": true
234234
}
@@ -254,15 +254,15 @@
254254
]
255255
},
256256
{
257-
"description": "items should properly handle null data",
257+
"description": "items with null",
258258
"schema": {
259259
"items": {
260260
"type": "null"
261261
}
262262
},
263263
"tests": [
264264
{
265-
"description": "null items allowed",
265+
"description": "allows null elements",
266266
"data": [ null ],
267267
"valid": true
268268
}

‎tests/draft-next/optional/bignum.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
]
4444
},
4545
{
46-
"description": "integer comparison",
46+
"description": "maximum integer comparison",
4747
"schema": { "maximum": 18446744073709551615 },
4848
"tests": [
4949
{
@@ -67,7 +67,7 @@
6767
]
6868
},
6969
{
70-
"description": "integer comparison",
70+
"description": "minimum integer comparison",
7171
"schema": { "minimum": -18446744073709551615 },
7272
"tests": [
7373
{

‎tests/draft-next/optional/ecmascript-regex.json

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@
77
},
88
"tests": [
99
{
10-
"description": "matches in Python, but should not in jsonschema",
10+
"description": "matches in Python, but not in ECMA 262",
1111
"data": "abc\\n",
1212
"valid": false
1313
},
1414
{
15-
"description": "should match",
15+
"description": "matches",
1616
"data": "abc",
1717
"valid": true
1818
}
@@ -342,7 +342,7 @@
342342
]
343343
},
344344
{
345-
"description": "unicode characters do not match ascii ranges",
345+
"description": "pattern with ASCII ranges",
346346
"schema": { "pattern": "[a-z]cole" },
347347
"tests": [
348348
{
@@ -395,7 +395,7 @@
395395
]
396396
},
397397
{
398-
"description": "unicode digits are more than 0 through 9",
398+
"description": "pattern with non-ASCII digits",
399399
"schema": { "pattern": "^\\p{digit}+$" },
400400
"tests": [
401401
{
@@ -480,7 +480,7 @@
480480
]
481481
},
482482
{
483-
"description": "unicode characters do not match ascii ranges",
483+
"description": "patternProperties with ASCII ranges",
484484
"schema": {
485485
"type": "object",
486486
"patternProperties": {
@@ -534,7 +534,7 @@
534534
]
535535
},
536536
{
537-
"description": "unicode digits are more than 0 through 9",
537+
"description": "patternProperties with non-ASCII digits",
538538
"schema": {
539539
"type": "object",
540540
"patternProperties": {

‎tests/draft-next/optional/format/date-time.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,12 @@
119119
"valid": false
120120
},
121121
{
122-
"description": "non-ascii digits should be rejected in the date portion",
122+
"description": "invalid non-ASCII '৪' (a Bengali 4) in date portion",
123123
"data": "1963-06-1৪T00:00:00Z",
124124
"valid": false
125125
},
126126
{
127-
"description": "non-ascii digits should be rejected in the time portion",
127+
"description": "invalid non-ASCII '৪' (a Bengali 4) in time portion",
128128
"data": "1963-06-11T0৪:00:00Z",
129129
"valid": false
130130
}

‎tests/draft-next/optional/format/date.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@
214214
"valid": true
215215
},
216216
{
217-
"description": "non-ascii digits should be rejected",
217+
"description": "invalid non-ASCII '৪' (a Bengali 4)",
218218
"data": "1963-06-1৪",
219219
"valid": false
220220
}

‎tests/draft-next/optional/format/duration.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@
119119
"valid": false
120120
},
121121
{
122-
"description": "non-ascii digits should be rejected",
122+
"description": "invalid non-ASCII '২' (a Bengali 2)",
123123
"data": "P২Y",
124124
"valid": false
125125
}

‎tests/draft-next/optional/format/ipv4.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
"valid": false
6565
},
6666
{
67-
"description": "leading zeroes should be rejected, as they are treated as octals",
67+
"description": "invalid leading zeroes, as they are treated as octals",
6868
"comment": "see https://sick.codes/universal-netmask-npm-package-used-by-270000-projects-vulnerable-to-octal-input-data-server-side-request-forgery-remote-file-inclusion-local-file-inclusion-and-more-cve-2021-28918/",
6969
"data": "087.10.0.1",
7070
"valid": false
@@ -75,7 +75,7 @@
7575
"valid": true
7676
},
7777
{
78-
"description": "non-ascii digits should be rejected",
78+
"description": "invalid non-ASCII '২' (a Bengali 2)",
7979
"data": "1২7.0.0.1",
8080
"valid": false
8181
}

0 commit comments

Comments
 (0)