Add verbosity/explanations on CannotOverwriteExistingCassetteException #439

arthurHamon2 · 2019-06-12T12:01:06Z

First of all, thanks @kevin1024 and to all who have contributed to this project, it is awesome and I'm using it for my professional projects for the last 4 years.

I'm working on a project with particularly big requests bodies and query parameters, when I record a cassette and make changes to my code, the cassette fails sometimes but without any information on what requests changes made the cassette to fail. This can be annoying to spot any differences with a large body (and the YAML encoding making it harder as the body is stored as base64, but I saw the community had a workaround : #241). My point is we have little information about why we are getting the "CannotOverwriteExistingCassetteException" exception.

The PR #309 and #332 are related to this topic but do not improve much as the logs in debug mode is very verbose and we still don't have the two different values printed, only the matchers that are failing.

My idea here is to get the closest request(s) in the cassette to the one being search, based on the number of matchers that have succeeded and get a detailed assertion message about the differences. This PR includes a big refactoring on how a matcher works, but it is compatible with the old way. Basically I use the assert statement to spot the differences instead of a boolean value.

This is an example of the new CannotOverwriteExistingCassetteException message :

CannotOverwriteExistingCassetteException: Can't overwrite existing cassette ('/private/var/folders/0t/wwygv0ys3ll2ydlq636m7_p00000gn/T/pytest-of-arthurhamon/pytest-157/test_default_matcher_does_not_3/test.yml') in your current record mode ('once').
No match for the request (<Request (GET) http://127.0.0.1:52107/get?p1=q1&a=b>) was found.
Similar request found : (<Request (GET) http://127.0.0.1:52107/get?p1=q1&p2=q2>).
Matcher failed : query
--------------- DETAILS ---------------
[('a', 'b'), ('p1', 'q1')] != [('p1', 'q1'), ('p2', 'q2')]
----------------------------------------

It gets even better when you tell pytest to re-rewrite the assert statements of the vcr.matchers module (this should be added to the pytest plugin if the PR is merged : https://github.com/ktosiek/pytest-vcr):

tests/integarations/__init__.py

import pytest

pytest.register_assert_rewrite("vcr.matchers")

CannotOverwriteExistingCassetteException: Can't overwrite existing cassette ('/private/var/folders/0t/wwygv0ys3ll2ydlq636m7_p00000gn/T/pytest-of-arthurhamon/pytest-161/test_default_matcher_does_not_3/test.yml') in your current record mode ('once').
No match for the request (<Request (GET) http://127.0.0.1:52766/get?p1=q1&a=b>) was found.
Similar request found : (<Request (GET) http://127.0.0.1:52766/get?p1=q1&p2=q2>).
Matcher failed : query
--------------- DETAILS ---------------
[('a', 'b'), ('p1', 'q1')] != [('p1', 'q1'), ('p2', 'q2')]
assert [('a', 'b'), ('p1', 'q1')] == [('p1', 'q1'), ('p2', 'q2')]
  At index 0 diff: ('a', 'b') != ('p1', 'q1')
  Full diff:
  - [('a', 'b'), ('p1', 'q1')]
  + [('p1', 'q1'), ('p2', 'q2')]
----------------------------------------

I hope I made myself clear and I'm looking forward to have any review/suggestions/questions on this !

kevin1024 · 2019-06-12T12:29:12Z

Wow, this looks like a great quality-of-life improvement.

I wonder why Travis isn’t running.

Also wondering about backwards-compat for people who have created custom matchers

arthurHamon2 · 2019-06-12T13:30:36Z

Also wondering about backwards-compat for people who have created custom matchers

Custom matchers are not a problem I think, I made sure that matcher that returns a boolean value is still working (see unit tests in my commit : 43830ea)
I may change the documentation in order to say that custom matchers can be written in a different manner, what do you think @kevin1024 ?

arthurHamon2 · 2019-06-13T14:11:13Z

@kevin1024 I have added some documentation about the new matcher behavior.

About Travis-CI it seems disabled but I don't have the rights to investigate more on this, but I saw multiple issues regarding the dependency updates when launching the all test suite with tox on my local, I have created another PR to deal with the dependencies changes : #441

neozenith · 2019-06-27T08:50:11Z

@arthurHamon2 yeah it looks like we are trying to achieve the same thing. No point in duplicated efforts.

All I was proposing in #445 was adding the original request and cassette as args on the thrown exception to be handled by others however they see fit.

But displaying the diffs is the end goal.

Increasing the logging to DEBUG says which matcher failed (header or path or body) but the diffs can help point to specific failures.

How do I go about folding my change in #445 in to this request?

Also what can we do to get CI passing? Is that a Travis thing? Or as long as we can get tox working locally?

Happy to help how I can. In particular work is allocating me time to chase this one down.

arthurHamon2 · 2019-06-27T11:59:05Z

How do I go about folding my change in #445 in to this request?

Fork the vcrpy repo of my repository and make a PR with your changes ;)

Also what can we do to get CI passing? Is that a Travis thing? Or as long as we can get tox working locally?

I have an hard time getting the tests to pass but I finally achieved to update/fix all of them here : #441
About TravisCI, no idea why it is not running ... @kevin1024 can we get a chance to fix this by being a contributor of the vcrpy project with @neozenith, he can be an active contributor as his work allocated time for this :)

@kevin1024 it seems that the project is not heavily maintained, issues and PRs are starting to stack in. I'll be happy to help on this point.

kevin1024 · 2019-06-27T12:31:22Z

I’m happy to have help! I’ll add you both as maintainers.

This function is used to prettify the assertion message when a matcher failed and return an assertion error.

A matcher can now return other results than a boolean : - An AssertionError exception meaning that the matcher failed, with the exception we get the assertion failure message. - None, in case we do an assert in the matcher, meaning that the assertion has passed, the matcher is considered as a success then. - Boolean that indicates if a matcher failed or not. If there is no match, a boolean does not give any clue what it is the differences compared to the assertion.

… of matchers The function returns two list: - the first one is the list of matchers names that have succeeded. - the second is a list of tuples with the failed matchers names and the related assertion message like this ("matcher_name", "assertion_message"). If the second list is empty, it means that all the matchers have passed.

…uests_match function In order to use the new assert mechanism that returns explicit assertion failure message, all the default matchers does not return a boolean, but only do an assert statement with a basic assertion message (value_1 != value_2). The requests_match function has been refactored to use the 'get_matchers_results' function in order to have explicit failures that are logged if any. Many unit tests have been changed as the matchers does not return a boolean value anymore. Note: Only the matchers "body" and "raw_body" does not have an assertion message, the body values might be big and not useful to be display to spot the differences.

This method get the requests in the cassettes with the most matchers that succeeds.

…ilding a more detailed message The 'CannotOverwriteExistingCassetteException' exception now takes two kwargs, cassette and failed requests, in order to get the request(s) in the cassettes with the less differences and put those details in the exception message.

Add documentation on creating a matcher with an `assert` statement that provides assertion messages in case of failures.

arthurHamon2 · 2019-06-27T20:24:54Z

@neozenith I just fixed the tests on master, I will wait your PR to include your improvements in order to merge this one :)

neozenith · 2019-06-27T23:33:11Z

@arthurHamon2 I'm happy to close out #445 in favor of this PR as it achieves exactly the goal I need in vcrpy/stubs/__init__.py::VCRConnection::getresponse

Only comment is that I was getting errors that CannotOverwriteExistingCassetteException does not accept keyword arguments.

Thank you @kevin1024 for the maintainer privileges. 🙏

arthurHamon2 · 2019-06-28T06:41:13Z

@neozenith I saw your changes, totally agree with you if you need to implement custom behavior using the exception properties !

arthurHamon2 mentioned this pull request Jun 27, 2019

Add context to thrown CannotOverwriteExistingCassetteException in write_protected mode #445

Closed

arthurHamon2 added 8 commits June 27, 2019 22:22

add function to format the assertion message

de244a9

This function is used to prettify the assertion message when a matcher failed and return an assertion error.

add cassette's method to find the most similar request(s) of a request

396c435

This method get the requests in the cassettes with the most matchers that succeeds.

update the documentation of custom matchers

f414e04

Add documentation on creating a matcher with an `assert` statement that provides assertion messages in case of failures.

add reference to pytest-vcr plugin in the documentation

b203fd4

arthurHamon2 force-pushed the master branch from 0f358ac to b203fd4 Compare June 27, 2019 20:22

neozenith merged commit 0830f60 into kevin1024:master Jun 28, 2019

This was referenced Jun 28, 2019

Add verbosity inside CannotOverwriteExistingCassetteException. #309

Closed

Return names of the matchers that differ #332

Closed

Show matcher name instead of matcher memory address in log #370

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add verbosity/explanations on CannotOverwriteExistingCassetteException #439

Add verbosity/explanations on CannotOverwriteExistingCassetteException #439

arthurHamon2 commented Jun 12, 2019 •

edited

Loading

kevin1024 commented Jun 12, 2019

arthurHamon2 commented Jun 12, 2019

arthurHamon2 commented Jun 13, 2019

neozenith commented Jun 27, 2019

arthurHamon2 commented Jun 27, 2019

kevin1024 commented Jun 27, 2019

arthurHamon2 commented Jun 27, 2019

neozenith commented Jun 27, 2019 •

edited

Loading

arthurHamon2 commented Jun 28, 2019

Add verbosity/explanations on CannotOverwriteExistingCassetteException #439

Add verbosity/explanations on CannotOverwriteExistingCassetteException #439

Conversation

arthurHamon2 commented Jun 12, 2019 • edited Loading

kevin1024 commented Jun 12, 2019

arthurHamon2 commented Jun 12, 2019

arthurHamon2 commented Jun 13, 2019

neozenith commented Jun 27, 2019

arthurHamon2 commented Jun 27, 2019

kevin1024 commented Jun 27, 2019

arthurHamon2 commented Jun 27, 2019

neozenith commented Jun 27, 2019 • edited Loading

arthurHamon2 commented Jun 28, 2019

arthurHamon2 commented Jun 12, 2019 •

edited

Loading

neozenith commented Jun 27, 2019 •

edited

Loading