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

Server parts of websocket tests are not affecting test outcome #760

Closed
2 tasks done
stefanw opened this issue Aug 18, 2020 · 2 comments
Closed
2 tasks done

Server parts of websocket tests are not affecting test outcome #760

stefanw opened this issue Aug 18, 2020 · 2 comments

Comments

@stefanw
Copy link

stefanw commented Aug 18, 2020

Checklist

  • The bug is reproducible against the latest release and/or master.
  • There are no similar issues or pull requests to fix it yet.

Describe the bug

While researching #757 I noticed that pytest assertions in server parts of websocket tests (with run_server(...)) are not working as intended in some cases.

Assertion failures are displayed (pytest -s) but do not affect the test run outcome. I suspect the reason lies in running the server in a loop in a thread and neither exceptions nor pytest failures are propagating to the main thread.

For example the pytest.raises in test_send_after_protocol_close is silently ignored.

To reproduce

I added an assert False in the server part of a test in #759 and all tests are passing.

Expected behavior

I expect the tests to fail when using assert False or pytest.fail("...") in the server parts of websocket tests.

Actual behavior

Exception or failures are printed to standard out, but do not affect the overall test outcome.

Debugging material

See test run for #759

Environment

  • OS / Python / Uvicorn version: Running uvicorn 0.11.8 with CPython 3.8.2 on Darwin

Additional context

I suspect that the server running in a loop in a thread is not propagating exceptions or pytest failures.

@Kludex
Copy link
Sponsor Member

Kludex commented Dec 5, 2022

We are on uvicorn 0.20.0, and by now we don't run the server in a thread anymore. We run it as an async task, but we still have the same behavior.

@Kludex
Copy link
Sponsor Member

Kludex commented Dec 16, 2022

I think this is fine. When the request-response cycle fails in the server, we can have an exception, but that will only be valid for that cycle, and we can't actually retrieve that exception, unless we mock some object.

We need to be mindful that assertions need to happen outside the server on the test suite. 🙏

@Kludex Kludex closed this as not planned Won't fix, can't repro, duplicate, stale Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants