scrapinghub / spidermon

Scrapy Extension for monitoring spiders execution.
https://spidermon.readthedocs.io
BSD 3-Clause "New" or "Revised" License
524 stars 94 forks source link

Fix tests for minimum properties and items #432

Closed rochamatcomp closed 4 months ago

rochamatcomp commented 5 months ago

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'.

rochamatcomp commented 5 months ago

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 commented 5 months ago

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 commented 4 months ago

@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 commented 4 months ago

@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

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

VMRuiz commented 4 months ago

Merged, thanks for you work @rochamatcomp