Get to 100% coverage on everything except compat and utf8validator #30
Conversation
Codecov Report
@@ Coverage Diff @@
## master #30 +/- ##
==========================================
+ Coverage 87.98% 99.3% +11.31%
==========================================
Files 7 7
Lines 849 860 +11
Branches 184 184
==========================================
+ Hits 747 854 +107
+ Misses 68 6 -62
+ Partials 34 0 -34
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This looks awesome to me. A few questions below.
This actually gets to 100% coverage on everything except Utf8Validator (thanks to codecov aggregating across the different CI builds), and the only uncovered code in Utf8Validator is the decode method that we don't use. Do you want to go ahead and delete decode too, and then this PR would close #26?
| if event is h11.NEED_DATA: | ||
| break | ||
| elif self.client and isinstance(event, h11.InformationalResponse): | ||
| elif self.client and isinstance(event, (h11.InformationalResponse, | ||
| h11.Response)): |
There was a problem hiding this comment.
What's the reasoning behind treating InformationalResponse and Response the same here?
There was a problem hiding this comment.
So we can deal with HTTP responses that we don't like more effectively.
test/test_connection.py
Outdated
| from wsproto.frame_protocol import CloseReason, FrameProtocol | ||
|
|
||
|
|
||
| class FakeProtocol(object): |
There was a problem hiding this comment.
Interesting... in h11 I wrote these kinds of tests by having a bit of utility code to glue together two connection objects and do the initial setup, and then I would like, inject weird data or make assertions about the data on the wire. (Utility code, example test)
This potentially has some advantages, since it makes fewer assumptions about the internals of the connection object, and thus hopefully survives better as the code evolves. Not every test can be written this way, but it might be a useful tool to have around.
I'm not suggesting you need to rewrite all these or anything like that, just bringing it up as something for us to think about :-)
Addresses #26