Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tests for minimum properties and items #432

Merged

Conversation

rochamatcomp
Copy link
Contributor

In JSON Schema validation for minimum properties and items the data should be non-empty, then I changed the minimum properties e items configuration to 2. To test empty cases as invalid I create corresponding tests.

I fixed some simples warnings such as warning from pytest trylast and about literal comparations with 'is'.

In JSON Schema validation for minimum properties and items the data should be non-empty.
Add validation messages for empty properties and items that should be
non-empty.
Use decorator @pytest.hookimpl(trylast=True) instead @pytest.mark.trylast
Change literal comparations from 'is' to '=='.
@rochamatcomp
Copy link
Contributor Author

The pipeline for python 3.8 has jsonschema==3.2.0, for other versions it has jsonschema>=3.2.0 (at present 4.21.1). The old version of JSON Schema allows valid empty properties and items, contrary to latest versions.

Can we change jsonschema version in tox.ini like this?

[testenv:min]
basepython = python3.8
deps =
    {[testenv]deps}
    jsonschema[format]>=3.2.0

@VMRuiz
Copy link
Collaborator

VMRuiz commented Feb 9, 2024

The pipeline for python 3.8 has jsonschema==3.2.0, for other versions it has jsonschema>=3.2.0 (at present 4.21.1). The old version of JSON Schema allows valid empty properties and items, contrary to latest versions.

Can we change jsonschema version in tox.ini like this?

[testenv:min]
basepython = python3.8
deps =
    {[testenv]deps}
    jsonschema[format]>=3.2.0

The reason for that test suite is to run the tests with the minimum valid package version, so it must be == not >=. However, we can increase the minimum required version for jsonschema to the lowest version that passes the new tests.

Please make sure to update setup.py too:

# tox.ini
[testenv:min]
basepython = python3.8
deps =
     {[testenv]deps}
     jsonschema[format]==x.x.x
     
# setup.py
install_requires=[
        "jsonschema[format]>=x.x.x",

Thank you so much for your interest in this project!

Edit: Fix typo in testenv:min where >= was supposed to be ==

@rochamatcomp
Copy link
Contributor Author

@VMRuiz thanks for your explanation! Until version 4.20.0 (included) the behaviour is the same of version 3.2.0. JSON Schema validation for minimum properties and items has recently changed from version 4.21.0 (at present the latest is 4.21.1).

Can we use the following configuration?

 # tox.ini
[testenv:min]
basepython = python3.8
deps =
     {[testenv]deps}
     jsonschema[format]==4.21.0
     
# setup.py
install_requires=[
        "jsonschema[format]>=4.21.0",

@VMRuiz
Copy link
Collaborator

VMRuiz commented Feb 15, 2024

@VMRuiz thanks for your explanation! Until version 4.20.0 (included) the behaviour is the same of version 3.2.0. JSON Schema validation for minimum properties and items has recently changed from version 4.21.0 (at present the latest is 4.21.1).

Can we use the following configuration?

 # tox.ini
[testenv:min]
basepython = python3.8
deps =
     {[testenv]deps}
     jsonschema[format]==4.21.0
     
# setup.py
install_requires=[
        "jsonschema[format]>=4.21.0",

Yes, please go ahead an introduce these changes in your PR

From version 4.21.0 JSON Schema to validation for minimum properties and items the data should be non-empty.
Copy link

codecov bot commented Feb 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (377c4ab) 79.39% compared to head (f404373) 79.39%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #432   +/-   ##
=======================================
  Coverage   79.39%   79.39%           
=======================================
  Files          76       76           
  Lines        3222     3223    +1     
  Branches      534      534           
=======================================
+ Hits         2558     2559    +1     
  Misses        593      593           
  Partials       71       71           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@VMRuiz VMRuiz merged commit 5214606 into scrapinghub:master Feb 16, 2024
7 checks passed
@VMRuiz
Copy link
Collaborator

VMRuiz commented Feb 16, 2024

Merged, thanks for you work @rochamatcomp

rochamatcomp added a commit to rochamatcomp/spidermon that referenced this pull request Feb 16, 2024
* Fix tests for minimum properties and items

In JSON Schema validation for minimum properties and items the data should be non-empty.

* Add tests for invalid empty properties and items

Add validation messages for empty properties and items that should be
non-empty.

* Fix warning from pytest trylast

Use decorator @pytest.hookimpl(trylast=True) instead @pytest.mark.trylast

* Fix warnings about literal comparations with 'is'

Change literal comparations from 'is' to '=='.

* Apply pre-commit

* Use jsonschema >= 3.2.0 for all python versions

* Define minimum version for JSON Schema

From version 4.21.0 JSON Schema to validation for minimum properties and items the data should be non-empty.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants