NatLibFi / Annif

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

Project-targeted Schemathesis test and OpenAPI schema fix #685

Closed juhoinkinen closed 1 year ago

juhoinkinen commented 1 year ago

PR #682 introduced Schemathesis for testing REST API. However, as noted in https://github.com/NatLibFi/Annif/pull/664#issuecomment-1476336156, Schemathesis creates random path (and query) parameters, which means most of the test requests target non-existent projects giving just 404 error.

This PR adds a test that targets the methods in the endpoint /v1/projects/{project_id}* and uses a fixed project-id dummy-fi. A project with this id exists, so the test requests probe the actions that are (mostly) missed otherwise.

This test noted two attributes in the OpenAPI specification that should be required or nullable, which are fixed. openapi-diff tool tool complains that the fixes break backward compatibility:

==========================================================================
==                            API CHANGE LOG                            ==
==========================================================================
                              Annif REST API                              
--------------------------------------------------------------------------
--                            What's Changed                            --
--------------------------------------------------------------------------
- POST   /projects/{project_id}/learn
  Request:
        - Changed application/json
          Schema: Broken compatibility
--------------------------------------------------------------------------
--                                Result                                --
--------------------------------------------------------------------------
                 API changes broke backward compatibility                 
--------------------------------------------------------------------------

However I think the fixes should be harmless for backward compatibility and should be done:

At first I thought that also text and subjects in IndexedDocument should be required, but maybe not, at least that case is covered with an if condition:https://github.com/NatLibFi/Annif/blob/4bf5610219e3e2dd23a0e0464b834338f0465009/annif/rest.py#L116

Also, this test is quite slow (10-15 s) and when the suggest-batch endpoint will be added it becomes even slower (20-25 s), so I marked it "slow" with a pytest decorator and configured pytest to skip it by default. It can be run using -m slow option:

 pytest -m slow
github-advanced-security[bot] commented 1 year ago

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (4bf5610) 99.57% compared to head (3f25671) 99.57%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #685 +/- ## ======================================= Coverage 99.57% 99.57% ======================================= Files 88 88 Lines 6138 6146 +8 ======================================= + Hits 6112 6120 +8 Misses 26 26 ``` | [Impacted Files](https://codecov.io/gh/NatLibFi/Annif/pull/685?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi) | Coverage Δ | | |---|---|---| | [tests/test\_openapi.py](https://codecov.io/gh/NatLibFi/Annif/pull/685?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

Changes to the schema look OK and necessary.

Regarding slow tests: it's good to not run them by default, but what about GitHub Actions CI? Now it seems they will also not be run under Actions, which Codecov complains about. I think it would make sense to run them there, otherwise nobody will ever remember to run them.

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

The new slow Schemathesis test is now a bit faster as it uses only 50 examples (which was enough to capture the now-fixed schema shortcomings).

Also I set the test to be run in the Python 3.9 GH Actions job, which has been the fastest one, so now jobs for different Python versions take about the same time.