NatLibFi / Annif

Annif is a multi-algorithm automated subject indexing tool for libraries, archives and museums.
https://annif.org
Other
195 stars 41 forks source link

Use Schemathesis to test REST API #682

Closed juhoinkinen closed 1 year ago

juhoinkinen commented 1 year ago

For testing the REST API there have been manually written tests since abandoning the unmaintained swagger-tester in #551. Already at that time Schemathesis was considered as a replacement of swagger-tester, but there was a dependency conflict that prevented using it.

Now Schemathesis can be installed to restore testing that the API actually conforms to the OpenAPI specification. Also the specification will now be validated.

With the default settings Schemathesis takes 12-15 s to run the tests in tests/test_swagger.py, but using @settings(max_examples=10) only 1-2 s.

~Due to a bug the pyproject.yaml includes pinned starlette installation (starlette = ">=0.13,<0.21").~ Edit: Pinning starlette is not necessary when pinning to recent version Schemathesis 3.18.*.

When fixing the errors noted by Schemathesis I marked some properties in the specification as nullable. is_trained and modification_time should be nullable, but backend_id probably should not be: https://github.com/NatLibFi/Annif/blob/6fc315763a395dff86420f3d1f07e3b3fe3ca869/annif/openapi/annif.yaml#L173 I marked this only to make the test pass. One project for the testing config is deliberately missing the backend specification: https://github.com/NatLibFi/Annif/blob/6fc315763a395dff86420f3d1f07e3b3fe3ca869/tests/projects.cfg#L70-L74

I'm not really sure what to do for that.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 :warning:

Comparison is base (e9fd177) 99.57% compared to head (08bf4ce) 99.57%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #682 +/- ## ========================================== - Coverage 99.57% 99.57% -0.01% ========================================== Files 88 88 Lines 6170 6138 -32 ========================================== - Hits 6144 6112 -32 Misses 26 26 ``` | [Impacted Files](https://codecov.io/gh/NatLibFi/Annif/pull/682?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi) | Coverage Δ | | |---|---|---| | [tests/conftest.py](https://codecov.io/gh/NatLibFi/Annif/pull/682?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi#diff-dGVzdHMvY29uZnRlc3QucHk=) | `100.00% <ø> (ø)` | | | [tests/test\_openapi.py](https://codecov.io/gh/NatLibFi/Annif/pull/682?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi#diff-dGVzdHMvdGVzdF9vcGVuYXBpLnB5) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

osma commented 1 year ago

is_trained and modification_time should be nullable, but backend_id probably should not be:

Agree, backend_id should not be nullable. We should ensure that the API never returns data like that.

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

juhoinkinen commented 1 year ago

is_trained and modification_time should be nullable, but backend_id probably should not be:

Agree, backend_id should not be nullable. We should ensure that the API never returns data like that.

I marked nobackend project as private, which I think is enough for this PR.

juhoinkinen commented 1 year ago

I started to feel that maybe some of the manually written tests should have not been deleted.

Schemathesis does use the examples defined in the API schema (indirectly stated in the CLI part of the documentation for the --hypothesis-phases=explicit option). This is true also when using it via pytest: I removed dummy-fi project from Annif test projects and 404 responses from the API schema, and this made those tests fail that use dummy-fi as an example project id in the schema. Reason for fails was shown to be

Received a response with a status code, which is not defined in the schema: 404

and in details "Project 'dummy-fi' not found".

But still this does not mean that e.g. the query parameters for /suggest-batch method are correctly passed and processed in rest.py. However it seems to need some amount of effort to make up an error that either tests in test_openapi.py or test_rest.py wont catch.