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

Update tests for available_api_versions #211

Merged
merged 1 commit into from Mar 5, 2020

Conversation

CasperWA
Copy link
Member

@CasperWA CasperWA commented Mar 5, 2020

Fixes #204

Moved some of the validation for the fields "url" and "version" to the Field.
For "url", a regex validator cannot be added in Field, since the type "AnyHttpUrl" takes precedence, see https://pydantic-docs.helpmanual.io/usage/schema/#unenforced-field-constraints for more info.
Instead the validator is kept explicitly and the regex is added to the OpenAPI schema as pattern.

Note the extra part of the version validator has been removed, where it was checking whether or not each segment can be parsed as an integer.
This is now considered true always as a result of the regex validation.

Important: The regex for "url" currently only allows the MAJOR versions 0 and 1. I think this makes sense? It was to avoid invalid cases like v09924.

@CasperWA CasperWA requested a review from ml-evs March 5, 2020 14:55
@codecov
Copy link

codecov bot commented Mar 5, 2020

Codecov Report

Merging #211 into master will increase coverage by 0.10%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #211      +/-   ##
==========================================
+ Coverage   86.96%   87.06%   +0.10%     
==========================================
  Files          41       41              
  Lines        1887     1879       -8     
==========================================
- Hits         1641     1636       -5     
+ Misses        246      243       -3     
Flag Coverage Δ
#unittests 87.06% <100.00%> (+0.10%) ⬆️

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 7ed738d...52feec9. Read the comment docs.

ml-evs
ml-evs previously approved these changes Mar 5, 2020
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

Important: The regex for "url" currently only allows the MAJOR versions 0 and 1. I think this makes sense? It was to avoid invalid cases like v09924.

This makes sense to me, if we ever get to v2.0 this is a simple fix :)

Changes look fine to me, generally.

@CasperWA
Copy link
Member Author

CasperWA commented Mar 5, 2020

Important: The regex for "url" currently only allows the MAJOR versions 0 and 1. I think this makes sense? It was to avoid invalid cases like v09924.

This makes sense to me, if we ever get to v2.0 this is a simple fix :)

Exactly my thought.
I think it would be good to have a central set of regex's for this, and to be used in the models all over. But that's for future PR.

@CasperWA CasperWA force-pushed the fix_204_correctly_test_availableapiversion branch from e74b1d8 to 3a0f7a3 Compare March 5, 2020 15:40
Moved some of the validation for the fields "url" and "version" to the
Field.
For "url", a `regex` validator cannot be added in Field, since the type
"AnyHttpUrl" takes precedence, see
https://pydantic-docs.helpmanual.io/usage/schema/#unenforced-field-constraints
for more info.
Instead the validator is kept explicitly and the regex is added to the
OpenAPI schema as `pattern`.

Note the extra part of the version validator has been removed, where it
was checking whether or not each segment can be parsed as an integer.
This is now considered true always as a result of the regex validation.
@CasperWA CasperWA force-pushed the fix_204_correctly_test_availableapiversion branch from 3a0f7a3 to 52feec9 Compare March 5, 2020 15:59
@CasperWA CasperWA requested a review from ml-evs March 5, 2020 15:59
@ml-evs ml-evs merged commit 8d72b8f into master Mar 5, 2020
@ml-evs ml-evs deleted the fix_204_correctly_test_availableapiversion branch March 5, 2020 17:08
@CasperWA CasperWA mentioned this pull request Mar 6, 2020
CasperWA added a commit that referenced this pull request Mar 6, 2020
Up to v0.6.0.

**New features**:
- GitHub Action validator that runs `optimade_validator` for a locally running OPTiMaDe server (#191, @CasperWA, tested by @ml-evs)
- Support filter queries for `HAS ALL`, `HAS ANY` and `HAS ONLY` and value lists on MongoDB backends (#173, @ml-evs)
  Note: `OPERATOR` use in value lists are still _not_ supported.
- Debug mode. Start the server in debug mode to enable `debug` log-level in `uvicorn` and get a full Python traceback in the JSON response (#190, @CasperWA)
- Add testing of mandatory `filter` queries to `optimade_validator` (#205, @ml-evs)

**Updates**:
- Allow Cross-Origin requests from anywhere (`*`), i.e., enable CORS for both servers (#194, @CasperWA)
- Updates to models (correct misspelling, more transparent model class naming, streamline models with respect to python class inheritance, update field descriptions) (#195, @CasperWA)
- API change: Rename `optimade.models.toplevel.py` to `optimade.models.responses.py` (#195, @CasperWA)
- Update dependencies to newest versions (as of 02.03.2020) (#202, #196, @CasperWA)
- Move imports from `starlette` to `fastapi`, where possible (#202, @ml-evs)
- Remove custom middleware to redirect slashed URLs in favor of `starlette` implementation (#202, @ml-evs)
- CI tests are now performed with a real MongoDB in the backend. CI tests are also performed with a `mongomock` backend for the tests in `server/test_middleware.py`, `server/test_server_validation.py`, and `server/test_config.py` (#196, @ml-evs, additional testing by @CasperWA )
- Remove `/optimade` from base URLs. This was especially important for the OpenAPI schema (#201, #216, @CasperWA, @ml-evs)
- Check integrity of query part of the raw URL using a custom middleware (#209, @CasperWA)
- New `optimade/server/exceptions.py` to contain custom `HttpException`s, moved `BadRequest` here (#209, @CasperWA)
- Pattern and regex testing for `data.available_api_versions` parts in `/info` endpoint and fix tests for the same (#211, @CasperWA)
- Restructure import of test data for regular server (#212, @shyamd)

**Bug fixes**:
- New retrieval URL for Materials-Consortia's list of providers (#187, @CasperWA)
- Skip local `HAS ONLY` tests with a `mongomock` backend, since v3.19.0 does not support these (#206, @ml-evs)
- Resource ID's can now contain slashes (`/`) (#183, @ml-evs, @CasperWA)
- Remove _valid_ version part of base URL in `meta.query.representation` (#201, @CasperWA)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test for AvailableApiVersion is correct for the wrong reasons
2 participants