search

vinissimus / async-asgi-testclient

A framework-agnostic library for testing ASGI web applications
MIT License
161 stars 20 forks source link

Issues with compliance to the ASGI specification #49

Open LucidDan opened 2 years ago

LucidDan commented 2 years ago

This package currently treats lifespan protocol as mandatory - if the application raises an exception in a lifespan.startup message, it treats the testclient as failed.

The ASGI spec states:

If an exception is raised when calling the application callable with a lifespan.startup message or a scope with type lifespan, the server must continue but not send any lifespan events. This allows for compatibility with applications that do not support the lifespan protocol. If you want to log an error that occurs during lifespan startup and prevent the server from starting, then send back lifespan.startup.failed instead.

So to test correctly, the TestClient should really allow an ASGI application to raise an exception, and if so then continue without sending any further lifespan messages, including on aexit.

LucidDan commented 2 years ago

Along with this one, I found a few other issues with specification compliance - I made a draft PR #50 that provides tests for as much of the ASGI spec as I could cover. There are xfail that demonstrate a few areas where the package doesn't follow the spec.

Of those xfails, five stand out as worthy of special mention, I think:

  1. test_http:test_response_body_can_be_missing - the 'body' key should be optional in http.response.body.
  2. test_http:test_response_headers_can_be_missing - the 'headers' key should be optional in http.response.start
  3. test_websocket:test_asgi_version_is_present_in_websocket_scope - websocket sessions do not include the 'asgi' key in scope, but it is required
  4. test_lifespan:test_lifespan_not_supported_is_allowed - what this ticket was originally about; the lifespan protocol must be optional, if the application raises an exception, continue as if it completed successfully 5. test_application:test_custom_scope_is_isolated_between_calls - custom scope is not copied on new requests, so it can contaminate between open()s. (changed my mind on the last one, it's not really a bug, although it might be worthy of documentation; will amend the PR to remove the test)