Code Monkey home page Code Monkey logo

Comments (16)

tylercollier avatar tylercollier commented on May 18, 2024

As follow-up motivation, I have another developer running tests, and they are failing with that same message. Helping him debug this remotely is going to be a pain. The fact that he's a rookie is not the fault of php-vcr, but why doesn't it just say

Invalid http request because [REASON]

?

Also, I just noticed that the message says "The cassette inserted did not have the necessary response", but should the last word be "request"?

from php-vcr.

adri avatar adri commented on May 18, 2024

That you very much Tyler. I think so too, there should be a way to easier find out what is happening (#36 is related to this as well).

"The request header ("Content-Length: 1567") did not match an existing request": How can we find out which parts of a request did not match? Not sure if it is possible. What might work is a list of all requests in a cassette with information why it didn't match.

I'm thinking about using #86 to provide a debug mode which then logs information somewhere. I wonder where that ideally should be. It could be in the exception as part of the message:

The request body does not match a previously recorded request. 
[... more information about the request ...] Recorded requests in the cassette [...cassette path...]:
 * GET /example, didn't match the header
 * GET /example2, didn't match the body

What do you think?

from php-vcr.

tylercollier avatar tylercollier commented on May 18, 2024

It's easy to find out which parts didn't match a given request, right? It has to be; that's how VCR works. I assume what you mean is, how to convey that information to the user without overwhelming them, if there are multiple requests in a cassette? One idea is to only show what part didn't match if there's a single request in the given cassette. That works well for me, because I only ever have a single request per cassette. Even then, if it didn't match the header, I'd like to see why. Show me the current request and the one from the cassette. Any example where this works in (non-PHP) VCR is here: you can set the debug logger during bootstrapping.

But for the situation where there are multiple requests in a cassette, if you include a debug mode, there could still be 100 requests in a cassette. That'd be too many * GET /example, didn't match the header messages! I don't know how common this is though to have 100. If it's between 1 to 10 that might be fine. I suppose if you include a debug mode, then people can make the choice for themselves, which is a good starting point IMO.

I will add some thoughts directly to #86.

Also, did you see this question of mine?

Also, I just noticed that the message says "The cassette inserted did not have the necessary response", but should the last word be "request"?

(Because it matches on requests, not responses, doesn't it?) I want to know if I'm right to make sure I'm understanding PHP-VCR's intent!

from php-vcr.

adri avatar adri commented on May 18, 2024

Ah yes, some projects have many requests in one cassette. But when using different cassettes for each test (which is the preferred way) I can imagine very detailed debugging info. Thank you for your link to the VCR examples!

Answering your question: You are absolutely right. The last word should be "request" or at least "The cassette inserted did not have the necessary response for the request". But I prefer your proposal or something like:

The request does not match a previously recorded request.

from php-vcr.

aaa2000 avatar aaa2000 commented on May 18, 2024

Concerning the debug mode, if there are many requests records in the cassette, we could maybe detect the recorded request which looks like the most to the request via levenshtein or similar_text PHP function. Then, tell which part (header, body...) doesn't match or use a diff.

from php-vcr.

tylercollier avatar tylercollier commented on May 18, 2024

@adri Your new text is improved. I think it'd be clearer to change the word "a" to "any":

The request does not match any previously recorded request...

@aaa2000 That's a good idea. Deciding which match is "closest" is non-trivial. I think you could write some code that got you 95% of the way there. Then make sure the message mentions that the current request differs from PHP-VCR's best guess as to a previously recorded transaction. This will still be better than it is today, where you have to do the guesswork and comparing yourself.

from php-vcr.

tylercollier avatar tylercollier commented on May 18, 2024

@aaa2000, are you going to do this? Otherwise I'll take a stab.

from php-vcr.

aaa2000 avatar aaa2000 commented on May 18, 2024

@tylercollier No, I didn't try to implement it

from php-vcr.

tylercollier avatar tylercollier commented on May 18, 2024

Ok, I'm excited to give it a shot. I'll do that by the end of March 1st, MST.

from php-vcr.

adri avatar adri commented on May 18, 2024

I'm trying to find out what output is most helpful for users. From my point of view the following data might be interesting (inspired by vcr/vcr#478):

  • the path of the current cassette
  • the used request matchers
  • the record mode
  • which of the requests was the closest match (if there is one, see below)
  • which field(s) didn't match
  • the content of the fields that didn't match (maybe even a diff)

I like the idea of detecting the best matching request. The only drawback is that there could be a case when all requests match equally well. Then choosing a single closest match is not particularly helpful.

As far as I see, Tylers implementation gets already a lot of that, it looks a bit like this (please correct me if I'm wrong, Tyler).

The closest match was request 1 of 3.

   Stored request: Host: example.com
  Current request: Host: exmpl.com

@tylercollier The reason why I bring this up is because I have no clear idea how this should look like in the end. I would like to help you better, but first I need an idea of what the result is going to be.

from php-vcr.

adri avatar adri commented on May 18, 2024

In general I think it would be handy to have a logger like vcr (ruby) has. Not only for explaining mismatches but also for learning about how php-vcr reacts to the system under test.

The question is to me how much information there has/should be in the exception message vs. in the log output.

from php-vcr.

tylercollier avatar tylercollier commented on May 18, 2024

Those are good ideas. You are correct with the example output you gave.

You're right that there could be a case when all the requests match equally well. This "best match" idea is not a trivial task, so we want to be clear to the user that we have a best match, and not the match. I will write up whatever algorithm we end up using in the documentation (by the way, I still request feedback on my "best match" strategy, see here). Eventually, we could have multiple strategies and allow a user to configure one to use, or allow them to configure multiple as well as when to use a given strategy. But let's do this the simple way first :-)

I use (Ruby's) VCR myself. Personally, I find the mismatch explanation (bullets 4-6) more helpful than the long message it shows. But their message (aka bullets 1-3) is clear/useful, so having both is best. Someday I plan to port the mismatch explanation to Ruby VCR. It at least has the debug logger, though its not perfect either.

Timewise, I wanted to work on this today, so I'm not sure if I can get to bullets 1 and 2 (we already have 3-6).

I'm thinking (1) will be straightforward. IIRC PHP-VCR does not allow nested cassettes. For (2), it could be helpful to mention all used request matchers; it's likely only a subset would show up in the mismatch explanation.

from php-vcr.

adri avatar adri commented on May 18, 2024

I think the "best match" can be as simple as selecting the request with the minimum number of mismatches.

What is your opinion of the following exception message draft?

A request does not match any previously recorded request and the 'mode' is set to 'none'. 
If you want to send the request anyway, make sure your 'mode' is set to 'new_episodes'.

  POST https://example.com/user/login
  Host: example.com
  User-Agent: Guzzle/5.2.0 curl/7.37.1 PHP/5.6.0
  {"username": "test", "password": "test"}

PHP-VCR is using the cassette "tests/fixtures/login-with-correct-credentials.yml" and 
matches on host, url, headers, body and query_string. 

The closest match was request 2 of 3 in the cassette:

  POST https://example.com/user/login
  - User-Agent: Guzzle/5.2.0 curl/7.37.1 PHP/5.5.0
  + User-Agent: Guzzle/5.2.0 curl/7.37.1 PHP/5.6.0

from php-vcr.

tylercollier avatar tylercollier commented on May 18, 2024

I like it. Perhaps a sentence before the -/+ mentioning what they represent, i.e. current request and stored request.

One thing to be careful of is assuming the values are not too long. Following Ruby VCR's lead, you might want to truncate to some line length by default. It would be nice if the truncation length were configurable.

Your strategy to find the best match does not include a tie-breaker. I think it's likely that two stored requests would differ on the same Field (to keep using that term from #111) e.g. url. I like aaa2000's idea of the levenshtein function as a tie breaker.

The message format can be independent of the algorithm used to determine the best match.

from php-vcr.

adri avatar adri commented on May 18, 2024

Here is a project which does a diff of http. How do you like the format?
https://github.com/jgrahamc/httpdiff

from php-vcr.

tylercollier avatar tylercollier commented on May 18, 2024

It's cool that httpdiff has colored output, although the default looks bad to me.

I prefer the standard diff format. Isn't that what phpunit uses by default for diff-type tests? I'm not sure how easy it would be to get diff coloring into phpunit.

I'm sorry I haven't updated you. I haven't looked at this since the last time I posted except to read your comments. I likely will not have time to work on it until April 11/12. The cool thing about making this feature in a robust manner is that it should make future changes (such as formatting with httpdiff, etc) much easier.

from php-vcr.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.