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
Add test coverage for HeaderElement
and AcceptElement
#2027
base: main
Are you sure you want to change the base?
Conversation
This is in preparation for issue cherrypy#2014 Establishing tests to validate these objects so we can pull in the deprecated cgi.parse_header code
@webknjaz I pushed up some updates to address comments last week. Do I need to do anything else for this to go in? |
Thanks for reminding! I'm going to take another look at this now as I'm on the PyCon US Sprints. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So.. I identified a few problems here. I'll try applying some patches and rebasing myself and will see where to go from there. Stay tuned!
cherrypy/test/test_httputil.py
Outdated
def test_accept_element_raises_400(header_content, media_type, qvalue): | ||
""" Test that headers passed to AcceptElement from httputils | ||
raise an expected HTTPError """ | ||
httputil.AcceptElement.from_str(header_content) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the other comment I made, I'm surprised that this doesn't fail in the CI — it seems like you expect that it would.
|
||
The expected exception is :exc:`~cherrypy.HTTPError`. | ||
""" | ||
with pytest.raises( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@radez so as I implemented this, the problem with the expectation in the test got evident. Could you see if you can identify what inputs would actually lead to the validation error expected here...
HeaderElement
and AcceptElement
This is in preparation for issue #2014
Establishing tests to validate these objects so we can pull in the deprecated cgi.parse_header code
What kind of change does this PR introduce?