kevin1024 / vcrpy

Automatically mock your HTTP interactions to simplify and speed up testing
MIT License
2.69k stars 387 forks source link

Differences between requests and aiohttp cassettes #463

Open dmfigol opened 5 years ago

dmfigol commented 5 years ago

Thank you for the library. My open-source library supports sync and async at the same time, so I'd love to use the same cassette for both tests. However, there are some challenges. Here are the cassettes fragments for requests and aiohttp: requests:

interactions:
- request:
    ...
  response:
    body:
      string: ...
      headers:
        Access-Control-Expose-Headers:
        - Link
        Cache-Control:
        - no-cache
        Content-Type:
        - application/json;charset=UTF-8
        Date:
        - Wed, 31 Jul 2019 17:32:09 GMT
        Link:
        - <url>
        Server:
        - Redacted
        Strict-Transport-Security:
        - max-age=63072000; includeSubDomains; preload
        Transfer-Encoding:
        - chunked
        Vary:
        - Accept-Encoding
        Via:
        - 1.1 linkerd
        content-length:
        - '38937'
    status:
      code: 200
      message: OK

aiohttp:

interactions:
- request:
    ...
  response:
    body:
      string: ...
    headers:
      Access-Control-Expose-Headers: Link
      Cache-Control: no-cache
      Content-Encoding: gzip
      Content-Type: application/json;charset=UTF-8
      Date: Wed, 31 Jul 2019 17:32:12 GMT
      Link: <url>
      Server: Redacted
      Strict-Transport-Security: max-age=63072000; includeSubDomains; preload
      Transfer-Encoding: chunked
      Vary: Accept-Encoding
      Via: 1.1 linkerd
    status:
      code: 200
      message: OK
    url: <url>

We can see two main differences:
1) in requests cassette response headers are serialized as key -> list of strings, in aiohttp cassette they are serialized as key -> str 2) in aiohttp cassette there is a response url key which equals to the request url, requests cassette does not have that

Also, because of 2), requests cassette played in aiohttp code will crash:

cls = <class 'yarl.URL'>, val = None

    def __new__(cls, val="", *, encoded=False, strict=None):
        if strict is not None:  # pragma: no cover
            warnings.warn("strict parameter is ignored")
        if type(val) is cls:
            return val
        if type(val) is str:
            val = urlsplit(val)
        elif type(val) is SplitResult:
            if not encoded:
                raise ValueError("Cannot apply decoding to SplitResult")
        elif isinstance(val, str):
            val = urlsplit(str(val))
        else:
>           raise TypeError("Constructor parameter should be str")
E           TypeError: Constructor parameter should be str
arthurHamon2 commented 5 years ago

Please feel free to submit a PR on this matter, it's definitely relevant to have identical cassettes whatever library we use to do a request.

neozenith commented 5 years ago

Hi, thanks for raising this issue! It makes sense that the cassettes should be consistent.

If you don't have the bandwidth to open a PR. Linking to a toy repo that replicates the issue helps a lot if others want to pick up this issue and contribute the fix/enhancement.

nickdirienzo commented 4 years ago

I was poking around and saw the release milestones (thanks for putting them, such up @neozenith!). This is in-scope for the 2.2.x release (https://github.com/kevin1024/vcrpy/milestone/3).

I opened up an example cassette of how the two compare more specifically (after some modifications to how aiohttp's stub works).

I'm in favor of sharing cassettes across clients; it makes sense for library development where you want HTTP request parity across sync and async implementations. There are some other specific differences to figure out how to handle, such as:

  1. requests adds an implicit / to root URLs
  2. requests adds default headers like User-Agent: python-requests/2.22.0 which may have issues with default matching

How should we handle these two cases?

nickdirienzo commented 4 years ago

Hmmm... maybe I was overthinking that. According to test runs, I'm able to share a requests cassette with aiohttp after some changes with how headers are handled on specific endpoints. The root path is still finicky with matching, it seems.

Perhaps https://github.com/nickdirienzo/vcrpy/pull/1 is on to something?

If someone wants to share cassettes between requests and aiohttp, should they ensure the encoding is handled correctly between the two, or should that be on vcrpy? I feel like it should be on the implementer because that complicates what the stubs are doing and makes them more aware of other clients, but I imagine there are reasons to not want that.