Skip to content

Commit

Permalink
unittests: migrate from jsonschema to fastjsonschema
Browse files Browse the repository at this point in the history
The former has rust dependencies, which lead to max capping on Cygwin
since there is no rust compiler there. But it turns out there are other
disadvantages of jsonschema:

- it involves installing 5 wheels, instead of just 1
- it is much slower

To give some perspective to the latter issue, this is what it looks like
when I test with jsonschema:

```
===== 1 passed, 509 deselected in 3.07s =====
Total time: 3.341 seconds
```

And here's what it looks like when I test with fastjsonschema:
```
===== 1 passed, 509 deselected, 1 warning in 0.28s =====
Total time: 0.550 seconds
```

I cannot think of a good reason to use the former. Although in order to
work on old CI images, we'll support it as a fallback mechanism
  • Loading branch information
eli-schwartz committed Dec 11, 2023
1 parent 5883089 commit 17c6d5e
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 15 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/cygwin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ jobs:
- name: Run pip
run: |
export PATH=/usr/bin:/usr/local/bin:$(cygpath ${SYSTEMROOT})/system32
# jsonschema is max capped because the new version depends on rust dependencies which are... hard to get on cygwin
python3 -m pip --disable-pip-version-check install gcovr 'jsonschema<4.18' pefile pytest pytest-subtests pytest-xdist coverage
python3 -m pip --disable-pip-version-check install gcovr fastjsonschema pefile pytest pytest-subtests pytest-xdist coverage
shell: C:\cygwin\bin\bash.exe --noprofile --norc -o igncr -eo pipefail '{0}'

- uses: actions/cache/save@v3
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
python-version: '3.x'
- run: |
python -m pip install --upgrade pip
python -m pip install pytest pytest-xdist pytest-subtests jsonschema coverage
python -m pip install pytest pytest-xdist pytest-subtests fastjsonschema coverage
- run: brew install pkg-config ninja llvm qt@5
- env:
CPPFLAGS: "-I/usr/local/include"
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/msys2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ jobs:
mingw-w64-${{ matrix.MSYS2_ARCH }}-python-lxml
mingw-w64-${{ matrix.MSYS2_ARCH }}-python-setuptools
mingw-w64-${{ matrix.MSYS2_ARCH }}-python-pip
mingw-w64-${{ matrix.MSYS2_ARCH }}-python-jsonschema
mingw-w64-${{ matrix.MSYS2_ARCH }}-python-fastjsonschema
mingw-w64-${{ matrix.MSYS2_ARCH }}-${{ matrix.TOOLCHAIN }}
- name: Install dependencies
Expand All @@ -100,7 +100,7 @@ jobs:
- name: Run Tests
run: |
if [[ "${{ matrix.MSYS2_ARCH }}" == "x86_64" ]]; then
# There apparently is no clean way to add to the PATH in the
# There apparently is no clean way to add to the PATH in the
# previous step?
# See for instance https://github.com/msys2/setup-msys2/issues/171
export PATH=$PATH:$PWD/pypy3local
Expand Down
2 changes: 1 addition & 1 deletion ci/ciimage/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ base_python_pkgs=(
pytest-xdist
pytest-subtests
coverage
jsonschema
fastjsonschema
)

python_pkgs=(
Expand Down
2 changes: 1 addition & 1 deletion ci/run.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ python --version

# Needed for running unit tests in parallel.
echo ""
python -m pip --disable-pip-version-check install --upgrade pefile pytest-xdist pytest-subtests jsonschema coverage
python -m pip --disable-pip-version-check install --upgrade pefile pytest-xdist pytest-subtests fastjsonschema coverage

# Needed for running the Cython tests
python -m pip --disable-pip-version-check install cython
Expand Down
27 changes: 19 additions & 8 deletions unittests/internaltests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1012,19 +1012,30 @@ def test_dependency_factory_order(self):
def test_validate_json(self) -> None:
"""Validate the json schema for the test cases."""
try:
from jsonschema import validate, ValidationError
from fastjsonschema import compile, JsonSchemaValueException as JsonSchemaFailure
fast = True
except ImportError:
if is_ci():
raise
raise unittest.SkipTest('Python jsonschema module not found.')

schema = json.loads(Path('data/test.schema.json').read_text(encoding='utf-8'))
try:
from jsonschema import validate, ValidationError as JsonSchemaFailure
fast = False
except:
if is_ci():
raise
raise unittest.SkipTest('neither Python fastjsonschema nor jsonschema module not found.')

with open('data/test.schema.json', 'r', encoding='utf-8') as f:
data = json.loads(f.read())

if fast:
schema_validator = compile(data)
else:
schema_validator = lambda x: validate(x, schema=data)

errors: T.List[T.Tuple[Path, Exception]] = []
for p in Path('test cases').glob('**/test.json'):
try:
validate(json.loads(p.read_text(encoding='utf-8')), schema=schema)
except ValidationError as e:
schema_validator(json.loads(p.read_text(encoding='utf-8')))
except JsonSchemaFailure as e:
errors.append((p.resolve(), e))

for f, e in errors:
Expand Down

0 comments on commit 17c6d5e

Please sign in to comment.