Remove swagger-tester dependency

[juhoinkinen]

The swagger-tester seems unmaintained and depends on connexion. Since Annif is switching to install connexion (#549) from its new PyPI project (named connexion2), the result is that connexion ends up to be installed twice (currently as connexion v2.7.0 and connexion2 v2.10.0).

This PR removes the swagger-tester dev dependency from Annif and adds self-written tests for REST API methods, so only connexion2 will be installed.

Also avoids running Flask in a background thread (at least explicitly) by using Flask's TestClient.

Now the tests cover basically only the status codes related to cases defined in swagger/annif.yaml. Should there be more detailed tests as in the tests in test_rest.py? (Or are these tests already overlapping, like testing the same functionality...?)

[codecov]

Codecov Report

Merging #551 (3e69a8f) into master (b9d8644) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #551   +/-   ##
=======================================
  Coverage   99.52%   99.53%           
=======================================
  Files          80       80           
  Lines        5313     5341   +28     
=======================================
+ Hits         5288     5316   +28     
  Misses         25       25           

Impacted Files	Coverage Δ
tests/conftest.py	100.00% <100.00%> (ø)
tests/test_swagger.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9d8644...3e69a8f. Read the comment docs.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

[osma]

Great work!

These are indeed already somewhat overlapping with the tests in test_rest.py. IIRC originally the idea was to separate the test files as follows:

• test_swagger.py does some basic exercise of the methods defined in the OpenAPI spec (as is done automatically by swagger-tester) using actual HTTP requests; as well as checking for CORS headers (which also requires HTTP)
• test_rest.py does more detailed tests (checking that the methods work as intended, including things like access control) only on a Python code level, without actual HTTP requests.

I think it would be useful to keep this kind of separation in mind even though we abandon swagger-tester - the tests in test_swagger.py should be kept in sync with the OpenAPI spec file and operate on a HTTP level, while test_rest.py can go a bit deeper into the application logic using Python only. On a quick reading, this seems to be the case in the tests you've written in this PR.

[osma]

Thinking aloud here: What this PR is missing is that it doesn't check that the returned data conforms to the shapes (return data types) specified in the OpenAPI spec file. I think swagger-tester did that, though I'm not 100% sure. There's a greater risk now that the app will not actually honor the contracts in the spec file.

The tests could be extended so that they perform more detailed testing of the returned data, but that still wouldn't guarantee that it follows the spec file, just that the person who wrote the tests thinks that the tests are checking for the right things. It would be better to derive the tests automatically from the spec file, not to craft them manually. But unfortunately we cannot use swagger-tester anymore for this.

[osma]

lgtm

[juhoinkinen]

For the future, I checked some alternatives for swagger-tester:

• swagger-unittest is a fork from swagger-tester, but cannot handle 204 responses from learn-method (and has no license).
• flex seems suitable, but it would require writing manually a test for each API method and has not been updated in 2 years. However, flex noticed null to be a wrong type of response for modification_time.
• Schemathesis is a large testing tool, seems a bit too big gun for Annif API at the moment. Also there was some package conflicts when trying to install.
• swagger-conformance is unmaintained.

[Stranger6667]

Hi @juhoinkinen

Schemathesis author here :)

Schemathesis is a large testing tool, seems a bit too big gun for Annif API at the moment. Also there was some package conflicts when trying to install.

I'd be happy to review the dependencies on the Schemathesis side so it is easier to install :) Let me know if there is anything I can help with on this matter

[juhoinkinen]

Hi @Stranger6667

I took another look at this, and on current master Schemathesis actually gets installed, but by backtracking Schemathesis to version 0.7.3.

Trying to install the latest Schemathesis v3.12.3 this conflict message is printed out:

ERROR: Cannot install annif and connexion2[swagger-ui]==2.10.0 because these package versions have conflicting dependencies.

The conflict is caused by:
optuna 2.10.0 depends on PyYAML
schemathesis 3.12.3 depends on pyyaml<7.0 and >=6.0
connexion2[swagger-ui] 2.10.0 depends on PyYAML<6 and >=5.1

In Connexion the pyyaml version has been limited to <6 last June.

However, because Annif is not using any Click 8 feature, at the moment the easiest way to resolve this for us would be to allow older Click, e.g. >= 7, which would allow Schemathesis older than 3.11.1. But as we have just updated to Click 8, which required some work to avoid other conflicts (#533), it's best just wait for Connexion to catch up.

But maybe some other people could be facing the same conflict, so allowing older Click in Schemathesis could be considered, if Click 8 is not needed there.

[Stranger6667]

@juhoinkinen

Thank you so much for providing the context! Indeed, there is no hard dependency on Click, so we can relax it a bit - I'll take a look at it soon :)