-
Notifications
You must be signed in to change notification settings - Fork 41
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
Project-targeted Schemathesis test and OpenAPI schema fix #685
Conversation
Noted by targeted Schemathesis test
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 ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #685 +/- ##
=======================================
Coverage 99.57% 99.57%
=======================================
Files 88 88
Lines 6138 6146 +8
=======================================
+ Hits 6112 6120 +8
Misses 26 26
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. |
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. |
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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. |
PR #682 introduced Schemathesis for testing REST API. However, as noted in #664 (comment), 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-iddummy-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:
However I think the fixes should be harmless for backward compatibility and should be done:
notation
in SuggestionResult should benullable
because it isNone
if the vocabulary does not contain notations, which usually is the caseuri
in subjects of IndexedDocument should be required because otherwise Internal error due toKeyError: 'uri'
occursAt first I thought that also
text
andsubjects
in IndexedDocument should be required, but maybe not, at least that case is covered with an if condition:Annif/annif/rest.py
Line 116 in 4bf5610
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: