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

Project-targeted Schemathesis test and OpenAPI schema fix #685

Merged
merged 5 commits into from
Mar 23, 2023

Conversation

juhoinkinen
Copy link
Member

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

  • notation in SuggestionResult should be nullable because it is None if the vocabulary does not contain notations, which usually is the case
  • uri in subjects of IndexedDocument should be required because otherwise Internal error due to KeyError: 'uri' occurs

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:

if "text" in d and "subjects" in d

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

@juhoinkinen juhoinkinen added this to the 0.61 milestone Mar 22, 2023
@github-advanced-security
Copy link

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

codecov bot commented Mar 22, 2023

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
@@           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 Coverage Δ
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.

@osma
Copy link
Member

osma commented Mar 22, 2023

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

sonarcloud bot commented Mar 22, 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

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.

@juhoinkinen juhoinkinen merged commit 38ec228 into main Mar 23, 2023
@juhoinkinen juhoinkinen deleted the targeted-schemathesis-tests branch March 23, 2023 11:19
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.

2 participants