100% Test coverage

[tomchristie]

Let's get the test coverage up to 100%, and then force-pin it.

Any contributions towards this are welcome.

[yeraydiazdiaz]

The merged PRs and #64 is as much as I've been able to increase the coverage, the report on that branch is as follows:

+ venv/bin/coverage report -m
Name                                      Stmts   Miss  Cover   Missing
-----------------------------------------------------------------------
httpcore/__init__.py                         10      0   100%
httpcore/client.py                          132      0   100%
httpcore/concurrency.py                      90      3    97%   79, 93-94
httpcore/config.py                           88      0   100%
httpcore/decoders.py                         63      0   100%
httpcore/dispatch/__init__.py                 0      0   100%
httpcore/dispatch/connection.py              58      0   100%
httpcore/dispatch/connection_pool.py         96      3    97%   78-80
httpcore/dispatch/http11.py                  73      3    96%   69, 100-103
httpcore/dispatch/http2.py                   91      0   100%
httpcore/exceptions.py                       17      0   100%
httpcore/interfaces.py                       36      0   100%
httpcore/models.py                          482      0   100%
httpcore/status_codes.py                      2      0   100%
httpcore/utils.py                            22      0   100%
tests/client/test_async_client.py            72      0   100%
tests/client/test_client.py                  86      0   100%
tests/client/test_redirects.py              158      0   100%
tests/conftest.py                            49      0   100%
tests/dispatch/__init__.py                    0      0   100%
tests/dispatch/test_connection_pools.py      68      0   100%
tests/dispatch/test_connections.py           20      0   100%
tests/dispatch/test_http2.py                 32      0   100%
tests/dispatch/utils.py                      58      0   100%
tests/models/test_headers.py                 94      0   100%
tests/models/test_queryparams.py             26      0   100%
tests/models/test_requests.py                61      0   100%
tests/models/test_responses.py              113      0   100%
tests/models/test_url.py                     23      0   100%
tests/test_config.py                         56      0   100%
tests/test_decoders.py                       59      0   100%
tests/test_timeouts.py                       19      0   100%
-----------------------------------------------------------------------
TOTAL                                      2254      9    99%

• httpcore/dispatch/http11.py 73 3 96% 69, 100-103
  • Line 69 covers receiving an informational response which I haven't been able to reproduce from our server fixture.
  • Lines 100-103: Close with H11 protocol error, I've tried manually reproducing an error but it would require reaching into the connection pool and manually closing the connection simply to trigger a pass, I'd say it's a candidate for pragma: nocover
• httpcore/concurrency.py 90 3 97% 79, 93-94
  • Line 79: Covers the Writer.write_no_block method, which I'm not sure what it does or how to reproduce it.
  • Lines 93-94: Handles an exception on stream_writer.drain to raise a WriteTimeout, I don't know how to recreate its conditions in a test.
• httpcore/dispatch/connection_pool.py 96 3 97% 78-80
  • Lines 78-80: are the body of ConnectionStore.__getitem__ which I'm not clear on what it does as it seems to return the key if it's in the store or None otherwise, which seems like a membership test?

Any pointers?

[tomchristie]

Stonkin great work here!
Sending an Expect : 100-continue header ought to trigger Uvicorn to issue a 100 informational response. That’d knock off one more!

[tomchristie]

I think closing a steaming response without reading it should trigger the h11 protocol error case, but I’m not 100% without reviewing it more thoroughly.

[tomchristie]

For the WriteTimeout, try sending a large request in test_timeouts.py, to the slow_response endpoint, using a tiny write timeout.

[yeraydiazdiaz]

Awesome, thanks for the tips! I'll try those out (in separate PRs) today 👍

[tomchristie]

Rad.

[tomchristie]

@yeraydiazdiaz Final remaining line here is in write_no_block. We use that interface in the HTTP/2 case in order to strictly write the HTTP/2 connection initialization, without allowing for any race conditions of "we've hit an async/await, so the task switched out under us, and wrote some different data first".

Because uvicorn doesn't currently have HTTP/2 support we're only ever testing out against a mock HTTP/2 server implementation, and we're never hitting that line.

I'm just going to pragma: nocover it for now as unreachable in test.

[tomchristie]

Thanks so much for your help on this!

[yeraydiazdiaz]

No worries, glad we could get it done 🎉

[tomchristie]

🤩