Skip to content
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

Catch HTTP adapter run_forever failures in test fixture #165

Merged
merged 3 commits into from
Jul 25, 2023

Conversation

rosesyrett
Copy link
Contributor

Currently on master, running pytest if port 8080 on localhost is bound freezes the output. The terminal simply hangs. I have investigated the issue and fixed this by changing the fixture where this is happening so that it keeps track of the task that calls .run_forever on the adaptor as well as .wait_until_ready and waits until one of those completes.

wait_until_ready will only ever be set if run_forever gets far enough to set it; if, for example, the server cannot be instantiated, this fails silently, and wait_until_ready therefore waits forever.

@codecov
Copy link

codecov bot commented Jul 21, 2023

Codecov Report

Merging #165 (539d44d) into master (9380b1a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #165   +/-   ##
=======================================
  Coverage   95.03%   95.03%           
=======================================
  Files          44       44           
  Lines        1310     1310           
=======================================
  Hits         1245     1245           
  Misses         65       65           

@rosesyrett rosesyrett changed the title Catch adapter run_forever failures in test fixture Catch HTTP adapter run_forever failures in test fixture Jul 21, 2023
@rosesyrett rosesyrett requested a review from tpoliaw July 25, 2023 12:58
@abbiemery abbiemery merged commit 76c0032 into master Jul 25, 2023
16 checks passed
@abbiemery abbiemery deleted the fix-hanging-http-tests branch July 25, 2023 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants