test_stream_timeout and test_urllib3_pool_connection_closed pass without asserting when no exception is raised
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 70/100
Research direction
Start in tests/test_requests.py, at TestTimeout.test_stream_timeout and the neighbouring test_invalid_timeout, which already uses pytest.raises. Run the timeout tests against the httpbin fixture to confirm they pass now. Then check test_urllib3_pool_connection_closed, where the assertion sits inside an except block. Done means each test fails when the expected exception is not raised. The second test may need a maintainer decision on what behaviour to assert.
Written by the indexing model from the issue text.
Description
Two tests in tests/test_requests.py put their only assertion inside an except block, so they pass without checking anything when the exception is not raised.
TestTimeout.test_stream_timeout: if the request to delay/10 completed without timing out, the test would pass. Switching it to pytest.raises (like test_invalid_timeout right below it) makes it fail in that case:
def test_stream_timeout(self, httpbin):
with pytest.raises(requests.exceptions.Timeout) as e:
requests.get(httpbin("delay/10"), timeout=2.0)
assert "Read timed out" in e.value.args[0].args[0]
I checked that this version passes as is and fails with DID NOT RAISE when pointed at delay/0. The change is in this commit on my fork, if it's useful: https://github.com/jeffev/requests/commit/b5058def5811
test_urllib3_pool_connection_closed: with the current urllib3, s.get() on an adapter mounted with pool_connections=0, pool_maxsize=0 succeeds (I get a 200), so the except ConnectionError branch never runs and the test checks nothing. I'm not sure what behavior it should pin down now, so I didn't touch it.
I'd have opened a PR, but pull requests seem limited to collaborators.
- Dominant language
- Python
- Stars
- 54.7k
- Forks
- 10.8k
- Avg merge
- 18m
- Merged PRs (30d)
- 1
Getting set up
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from psf/requests
-
Difficulty 1/5 Under an hour Newbie friendliness 85/100
-
`iter_lines(decode_unicode=True, delimiter=<str>)` raises `TypeError` when `Response.encoding` is `None`May be free again A pull request for this issue was closed without being merged. Open
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
-
get_encoding_from_headers treats any content type containing 'text' as text/* (ISO-8859-1)Possibly taken @suyashgitt claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
Invalid `params` type (e.g. int) raises misleading "TypeError: Cannot mix str and non-str arguments"Open
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
OCA/account-invoicing#2529 ·
Maintainers usually reply within 1 day
-
area/ci
Difficulty 1/5 Under an hour Newbie friendliness 62/100
Maintainers usually reply within 1 day
-
ImageGen generate-batch aborts valid JSONL jobs containing a Unicode line separator in poster textOpenbug imagen skills
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
Email validation accepts a trailing newlinePossibly taken @marcello-pichini claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
Maintainers usually reply within 1 day