Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Schemathesis to test REST API #682

Merged
merged 7 commits into from Mar 15, 2023
Merged

Use Schemathesis to test REST API #682

merged 7 commits into from Mar 15, 2023

Conversation

juhoinkinen
Copy link
Member

@juhoinkinen juhoinkinen commented Mar 14, 2023

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:

nullable: true

I marked this only to make the test pass. One project for the testing config is deliberately missing the backend specification:

Annif/tests/projects.cfg

Lines 70 to 74 in 6fc3157

[nobackend]
name=Dummy with no backend
language=en
vocab=dummy
analyzer=snowball(english)

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

@juhoinkinen juhoinkinen added this to the 0.61 milestone Mar 14, 2023
@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01 ⚠️

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

Additional details and impacted files
@@            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 Coverage Δ
tests/conftest.py 100.00% <ø> (ø)
tests/test_openapi.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@juhoinkinen juhoinkinen marked this pull request as ready for review March 14, 2023 15:29
pyproject.toml Outdated Show resolved Hide resolved

def test_swagger_cors(app_client):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does schemathesis replace this test, and all other tests below? Or should we also keep these tests?

Also, I think we should rename test_swagger.py to something else, maybe test_openapi.py, because we're not actually using Swagger (2.0) anymore. I just forgot to rename this in the OpenAPI 3.0 PR #649

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a custom check that is run in addition to the default Schemathesis tests to cover checking CORS support:

@schemathesis.check
def check_cors(response, case):
assert response.headers["access-control-allow-origin"] == "*"

Schemathesis validates the following on the responses:

  • not_a_server_error. The response has 5xx HTTP status;
  • status_code_conformance. The response status is not defined in the API schema;
  • content_type_conformance. The response content type is not defined in the API schema;
  • response_schema_conformance. The response content does not conform to the schema defined for this specific response;
  • response_headers_conformance. The response headers does not contain all defined headers.

If I make methods in rest.py to return nonsense strings, Schemathesis tests fail:

tests/test_openapi.py::test_api[GET /v1/] FAILED                                                                                                                [ 20%]
tests/test_openapi.py::test_api[GET /v1/projects] FAILED                                                                                                        [ 40%]
tests/test_openapi.py::test_api[GET /v1/projects/{project_id}] FAILED                                                                                           [ 60%]
tests/test_openapi.py::test_api[POST /v1/projects/{project_id}/suggest] FAILED                                                                                  [ 80%]
tests/test_openapi.py::test_api[POST /v1/projects/{project_id}/learn] FAILED                                                                                    [100%]

If I make error cases to return code 204 instead of 404/503/400 some tests fail:

tests/test_openapi.py::test_api[GET /v1/] PASSED                                                                                                                [ 20%]
tests/test_openapi.py::test_api[GET /v1/projects] PASSED                                                                                                        [ 40%]
tests/test_openapi.py::test_api[GET /v1/projects/{project_id}] FAILED                                                                                           [ 60%]
tests/test_openapi.py::test_api[POST /v1/projects/{project_id}/suggest] FAILED                                                                                  [ 80%]
tests/test_openapi.py::test_api[POST /v1/projects/{project_id}/learn] PASSED                                                                                    [100%]

(/learn method has 204 in the schema, so testing that is passed.)

Both cases are covered also by test_rest.py, so I would not keep the old tests here.

@osma
Copy link
Member

osma commented Mar 14, 2023

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
Copy link

sonarcloud bot commented Mar 15, 2023

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
Copy link
Member Author

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.

Copy link
Member

@osma osma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LVGTM

@juhoinkinen juhoinkinen merged commit 3a5fc50 into main Mar 15, 2023
@juhoinkinen juhoinkinen deleted the schemathesis branch March 15, 2023 16:33
@juhoinkinen
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants