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

Test validation of parameters in path, header and cookies #29

Merged
merged 4 commits into from
Aug 1, 2019

Conversation

zupo
Copy link
Collaborator

@zupo zupo commented Jul 31, 2019

I took @gweis's commits and rebased them off of latest master:

@zupo zupo changed the title Test validation of parameters on path object Test validation of parameters in path, header and cookies Jul 31, 2019
@@ -181,3 +182,166 @@ def test_openapi_view_validate_HTTPExceptions() -> None:

with pytest.raises(HTTPForbidden):
view(context, request)
Copy link
Collaborator Author

@zupo zupo Jul 31, 2019

Choose a reason for hiding this comment

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

Hey @gweis I need your eyes on this: there used to be a line like this here: assert str(exc.value) == "Unknown response http status: 403", but is was removed in later commits in #26.

Looking at the test_openapi_view_validate_HTTPExceptions test case, it seems that it is not testing anything? At least the comments say it's testing that un-spec-ed responses should be re-raised as InvalidResponse but there are not asserts to confirm this?

Copy link
Contributor

@gweis gweis Jul 31, 2019

Choose a reason for hiding this comment

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

Good catch. (totally overlooked that doc string).

The difference is, that the response is now validated inside the tween (not in the view wrapper).
So either this type of test should be done inside test_validation.py, where tests are run through a pyramid Router or this tests needs to be update. One way to do that would be like this: gweis@7dc65ca

There is one small issue with the tween. It only validates the response correctly if there is an exception view handler configured in the pyramid app. Usually this is the case per default, but the normal pyramid testing test setup does not install one. (I'll create another pull request to handle that case as well)

maybe this is what this comment referred to #26 (comment)

@zupo zupo merged commit 05a1c1f into master Aug 1, 2019
@miketheman miketheman deleted the more_testcases branch March 20, 2024 18:38
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.

2 participants