Repository: NatLibFi/Annif (language: python)

Issue:
Remove swagger-tester dependency 
The [swagger-tester](https://github.com/Trax-air/swagger-tester) seems unmaintained and depends on [connexion](https://github.com/zalando/connexion). Since Annif is switching to install connexion (#549) from its new PyPI project (named [connexion2](https://pypi.org/project/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](https://flask.palletsprojects.com/en/2.0.x/testing/#the-testing-skeleton).

Now the tests cover basically only the status codes related to cases defined in [`swagger/annif.yaml`](https://github.com/NatLibFi/Annif/blob/master/annif/swagger/annif.yaml). Should there be more detailed tests as in the tests in [`test_rest.py`](https://github.com/NatLibFi/Annif/blob/master/tests/test_rest.py)? (Or are these tests already overlapping, like testing the same functionality...?)


Discussion / hints:
# [Codecov](https://codecov.io/gh/NatLibFi/Annif/pull/551?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi) Report
> Merging [#551](https://codecov.io/gh/NatLibFi/Annif/pull/551?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi) (3e69a8f) into [master](https://codecov.io/gh/NatLibFi/Annif/commit/b9d86448f5ce87b888d1d4a9281ecad0bfd5104c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi) (b9d8644) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/NatLibFi/Annif/pull/551/graphs/tree.svg?width=650&height=150&src=pr&token=zzQRZwJLut&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi)](https://codecov.io/gh/NatLibFi/Annif/pull/551?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi)

```diff
@@           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](https://codecov.io/gh/NatLibFi/Annif/pull/551?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/551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi#diff-dGVzdHMvY29uZnRlc3QucHk=) | `100.00% <100.00%> (ø)` | |
| [tests/test\_swagger.py](https://codecov.io/gh/NatLibFi/Annif/pull/551/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi#diff-dGVzdHMvdGVzdF9zd2FnZ2VyLnB5) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/NatLibFi/Annif/pull/551?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/NatLibFi/Annif/pull/551?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi). Last update [b9d8644...3e69a8f](https://codecov.io/gh/NatLibFi/Annif/pull/551?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NatLibFi).

Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; ![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed.svg 'Quality Gate passed')

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.svg 'Bug')](https://sonarcloud.io/project/issues?id=NatLibFi_Annif&pullRequest=551&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.svg 'A')](https://sonarcloud.io/project/issues?id=NatLibFi_Annif&pullRequest=551&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=NatLibFi_Annif&pullRequest=551&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.svg 'Vulnerability')](https://sonarcloud.io/project/issues?id=NatLibFi_Annif&pullRequest=551&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.svg 'A')](https://sonarcloud.io/project/issues?id=NatLibFi_Annif&pullRequest=551&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=NatLibFi_Annif&pullRequest=551&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.svg 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=NatLibFi_Annif&pullRequest=551&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.svg 'A')](https://sonarcloud.io/project/security_hotspots?id=NatLibFi_Annif&pullRequest=551&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=NatLibFi_Annif&pullRequest=551&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.svg 'Code Smell')](https://sonarcloud.io/project/issues?id=NatLibFi_Annif&pullRequest=551&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.svg 'A')](https://sonarcloud.io/project/issues?id=NatLibFi_Annif&pullRequest=551&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=NatLibFi_Annif&pullRequest=551&resolved=false&types=CODE_SMELL)

[![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.svg 'No Coverage information')](https://sonarcloud.io/component_measures?id=NatLibFi_Annif&pullRequest=551) No Coverage information  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.svg '0.0%')](https://sonarcloud.io/component_measures?id=NatLibFi_Annif&pullRequest=551&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=NatLibFi_Annif&pullRequest=551&metric=new_duplicated_lines_density&view=list)


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.
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.
For the future, I checked some alternatives for swagger-tester:
- [swagger-unittest](https://github.com/mild-blue/swagger-unittest) is a fork from swagger-tester, but cannot handle 204 responses from learn-method (and has no license).
- [flex](https://github.com/pipermerriam/flex/) seems suitable, but it would require writing [manually a test for each API method](https://flex-swagger.readthedocs.io/en/latest/#api-call-validation) and has not been updated in 2 years. However, flex noticed null to be a wrong type of response for `modification_time`.
- [Schemathesis](https://github.com/schemathesis/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](https://github.com/olipratt/swagger-conformance) is unmaintained.
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
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](https://github.com/zalando/connexion/pull/1367).

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](https://github.com/schemathesis/schemathesis/releases/tag/v3.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.


@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 :)

Fix patch:
diff --git a/.gitignore b/.gitignore
index 610dbc178..7538833b2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,3 +12,4 @@ docs/_build/
 poetry.lock
 projects.cfg
 venv/
+.hypothesis/
diff --git a/annif/openapi/annif.yaml b/annif/openapi/annif.yaml
index 7d39e1d1f..117faef9c 100644
--- a/annif/openapi/annif.yaml
+++ b/annif/openapi/annif.yaml
@@ -12,7 +12,7 @@ paths:
       summary: get basic information about the API service
       operationId: annif.rest.show_info
       responses:
-        200:
+        "200":
           description: successful operation
           content:
             application/json:
@@ -25,7 +25,7 @@ paths:
       summary: get a list of projects
       operationId: annif.rest.list_projects
       responses:
-        200:
+        "200":
           description: successful operation
           content:
             application/json:
@@ -40,13 +40,13 @@ paths:
       parameters:
       - $ref: '#/components/parameters/project_id'
       responses:
-        200:
+        "200":
           description: successful operation
           content:
             application/json:
               schema:
                 $ref: '#/components/schemas/Project'
-        404:
+        "404":
           description: Project not found
           content:
             application/problem+json:
@@ -89,25 +89,25 @@ paths:
                   description: language of subject labels
         required: true
       responses:
-        200:
+        "200":
           description: successful operation
           content:
             application/json:
               schema:
                 $ref: '#/components/schemas/SuggestionResultList'
-        400:
+        "400":
           description: Bad Request
           content:
             application/problem+json:
               schema:
                 $ref: '#/components/schemas/Problem'
-        404:
+        "404":
           description: Project not found
           content:
             application/problem+json:
               schema:
                 $ref: '#/components/schemas/Problem'
-        503:
+        "503":
           description: Service Unavailable
           content:
             application/problem+json:
@@ -131,16 +131,16 @@ paths:
                 $ref: '#/components/schemas/IndexedDocument'
         required: true
       responses:
-        204:
+        "204":
           description: successful operation
           content: {}
-        404:
+        "404":
           description: Project not found
           content:
             application/problem+json:
               schema:
                 $ref: '#/components/schemas/Problem'
-        503:
+        "503":
           description: Service Unavailable
           content:
             application/problem+json:
@@ -193,10 +193,12 @@ components:
         is_trained:
           type: boolean
           example: true
+          nullable: true
         modification_time:
           type: string
           format: date-time
           example: 2020-05-19T16:42:42Z
+          nullable: true
       description: A project definition
     ProjectList:
       type: object
diff --git a/pyproject.toml b/pyproject.toml
index df6981341..4c7fb93a9 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -70,6 +70,7 @@ flake8 = "*"
 bumpversion = "*"
 black = "23.*"
 isort = "*"
+schemathesis = "3.18.*"
 
 [tool.poetry.extras]
 fasttext = ["fasttext-wheel"]


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index fcccb268f..ea53ba579 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -33,12 +33,6 @@ def app_with_initialize():
     return app
 
 
-@pytest.fixture
-def app_client(app):
-    with app.test_client() as app_client:
-        yield app_client
-
-
 @pytest.fixture(scope="module")
 def registry(app):
     with app.app_context():
diff --git a/tests/projects.cfg b/tests/projects.cfg
index f4c1a8bb2..5f7ce793c 100644
--- a/tests/projects.cfg
+++ b/tests/projects.cfg
@@ -72,6 +72,7 @@ name=Dummy with no backend
 language=en
 vocab=dummy
 analyzer=snowball(english)
+access=private
 
 [noname]
 language=en
diff --git a/tests/test_openapi.py b/tests/test_openapi.py
new file mode 100644
index 000000000..e77230fd9
--- /dev/null
+++ b/tests/test_openapi.py
@@ -0,0 +1,17 @@
+"""Unit tests for Annif REST API / Swagger spec"""
+import schemathesis
+from hypothesis import settings
+
+schema = schemathesis.from_path("annif/openapi/annif.yaml")
+
+
+@schemathesis.check
+def check_cors(response, case):
+    assert response.headers["access-control-allow-origin"] == "*"
+
+
+@schema.parametrize()
+@settings(max_examples=10)
+def test_api(case, app):
+    response = case.call_wsgi(app)
+    case.validate_response(response, additional_checks=(check_cors,))
diff --git a/tests/test_swagger.py b/tests/test_swagger.py
deleted file mode 100644
index 037d17195..000000000
--- a/tests/test_swagger.py
+++ /dev/null
@@ -1,74 +0,0 @@
-"""Unit tests for Annif REST API / Swagger spec"""
-
-
-def test_swagger_cors(app_client):
-    # test that the service supports CORS
-    req = app_client.get("http://localhost:8000/v1/projects")
-    assert req.headers["access-control-allow-origin"] == "*"
-
-
-def test_swagger_list_projects(app_client):
-    req = app_client.get("http://localhost:8000/v1/projects")
-    assert req.status_code == 200
-    assert "projects" in req.get_json()
-
-
-def test_swagger_show_project(app_client):
-    req = app_client.get("http://localhost:8000/v1/projects/dummy-fi")
-    assert req.status_code == 200
-    assert req.get_json()["project_id"] == "dummy-fi"
-
-
-def test_swagger_show_project_nonexistent(app_client):
-    req = app_client.get("http://localhost:8000/v1/projects/nonexistent")
-    assert req.status_code == 404
-
-
-def test_swagger_suggest(app_client):
-    data = {"text": "example text"}
-    req = app_client.post(
-        "http://localhost:8000/v1/projects/dummy-fi/suggest", data=data
-    )
-    assert req.status_code == 200
-    assert "results" in req.get_json()
-
-
-def test_swagger_suggest_nonexistent(app_client):
-    data = {"text": "example text"}
-    req = app_client.post(
-        "http://localhost:8000/v1/projects/nonexistent/suggest", data=data
-    )
-    assert req.status_code == 404
-
-
-def test_swagger_suggest_novocab(app_client):
-    data = {"text": "example text"}
-    req = app_client.post(
-        "http://localhost:8000/v1/projects/novocab/suggest", data=data
-    )
-    assert req.status_code == 503
-
-
-def test_swagger_learn(app_client):
-    data = [
-        {
-            "text": "the quick brown fox",
-            "subjects": [{"uri": "http://example.org/fox", "label": "fox"}],
-        }
-    ]
-    req = app_client.post("http://localhost:8000/v1/projects/dummy-fi/learn", json=data)
-    assert req.status_code == 204
-
-
-def test_swagger_learn_nonexistent(app_client):
-    data = []
-    req = app_client.post(
-        "http://localhost:8000/v1/projects/nonexistent/learn", json=data
-    )
-    assert req.status_code == 404
-
-
-def test_swagger_learn_novocab(app_client):
-    data = []
-    req = app_client.post("http://localhost:8000/v1/projects/novocab/learn", json=data)
-    assert req.status_code == 503