Skip to content

Conversation

@julianz-
Copy link
Contributor

@julianz- julianz- commented Oct 1, 2025

These fixes address:

  1. The test_keepalive_conn_management test was intermittently failing in CI with http.client.NotConnected. This was due to the combined execution time of test logic and the explicit time.sleep() exceeding the server timeout (2.0s).

  2. The http_request_timeout was also too short and causing failures in test_http_over_https_error().

This commit addresses the flakiness by increasing both timeout values.


This change is Reviewable

@codecov
Copy link

codecov bot commented Oct 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.31%. Comparing base (4f040de) to head (8dea118).
⚠️ Report is 160 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #777   +/-   ##
=======================================
  Coverage   79.31%   79.31%           
=======================================
  Files          29       29           
  Lines        4197     4197           
  Branches      538      538           
=======================================
  Hits         3329     3329           
  Misses        726      726           
  Partials      142      142           

@julianz- julianz- force-pushed the fix-timeouts branch 2 times, most recently from 53df248 to 0f1d213 Compare October 1, 2025 19:06
@webknjaz
Copy link
Member

webknjaz commented Oct 3, 2025

@julianz- could you also link the example failures you were seeing in actual CI logs?

@julianz- julianz- force-pushed the fix-timeouts branch 10 times, most recently from adae5be to 8848593 Compare October 3, 2025 20:20
These fixes address:

1. The `test_keepalive_conn_management` test was intermittently failing in CI
with `http.client.NotConnected`. This was due to the combined execution time
of test logic and the explicit `time.sleep()` exceeding
the server timeout (2.0s).

2. The http_request_timeout was also too short and causing failures in
test_http_over_https_error().

This commit addresses the flakiness by increasing both timeout values.
@julianz-
Copy link
Contributor Author

julianz- commented Oct 4, 2025

@julianz- could you also link the example failures you were seeing in actual CI logs?

Here's a link for the failure with test_keepalive_conn_management().
https://github.com/cherrypy/cheroot/actions/runs/18145322195/job/51645700584?pr=764 for test_keepalive_conn_management() failure

Unfortunately, I can't find a log for the other timeout failure now. I saw it many times but it's bit of a nightmare rummaging through all these builds. I can drop this change and see it if comes back again.

Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

Alright, I've skimmed through the second test and even though there's no log to check, the timeout change doesn't seem harmful so I'll just go with it. Thanks!

@webknjaz webknjaz merged commit e9b4361 into cherrypy:main Oct 5, 2025
68 of 76 checks passed
mr-c added a commit to mr-c/cheroot that referenced this pull request Jan 8, 2026
For the Debian package of cheroot we need Python 3.14 support, and I noticed that applying cherrypy#800 cherrypy#801 cherrypy#777 on top of the 11.1.2 release was enough to get the tests to pass with Python 3.14.

Fixes: cherrypy#767
mr-c added a commit to mr-c/cheroot that referenced this pull request Jan 8, 2026
For the Debian package of cheroot we need Python 3.14 support, and I noticed
that applying cherrypy#800 cherrypy#801 cherrypy#777 on top of the 11.1.2 release was enough to get
the tests to pass with Python 3.14.

Fixes: cherrypy#767
mr-c added a commit to mr-c/cheroot that referenced this pull request Jan 8, 2026
For the Debian package of cheroot we need Python 3.14 support, and I noticed
that applying cherrypy#800 cherrypy#801 cherrypy#777 on top of the 11.1.2 release was enough to get
the tests to pass with Python 3.14.

Fixes: cherrypy#767
mr-c added a commit to mr-c/cheroot that referenced this pull request Jan 8, 2026
For the Debian package of cheroot we need Python 3.14 support, and I noticed
that applying cherrypy#800 cherrypy#801 cherrypy#777 on top of the 11.1.2 release was enough to get
the tests to pass with Python 3.14.

Fixes: cherrypy#767
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants