bblimke / webmock

Library for stubbing and setting expectations on HTTP requests in Ruby.
MIT License
3.94k stars 553 forks source link

Proposal: Show request body diff to narrow test output #825

Open Adrian1707 opened 5 years ago

Adrian1707 commented 5 years ago

TLDR: Test output is often times very difficult to decipher as there can be too much noise. I'm here to recommend a custom solution I've written for one of my clients

When I run feature tests that make multiple requests and there's a small difference in the body of the stubbed request and the actual request, it's very difficult to see the diff of the bodies. You usually have to prune through lines and lines of output to discover what the difference was between the two. A solution I propose is to introduce a new helper called check_request_body_diff which takes two parameters; the request_uri and the stubbed request (which is an instance of RequestStub). The matching request is then found through the RequestRegistry and the body diffs are then compared and shown to the user.

This blog post essentially describes the entire problem & solution https://medium.com/@adrianbooth/how-to-significantly-improve-webmock-errors-3add6b5e93d2.

Adrian1707 commented 5 years ago

Just realised this belongs in this mailing list https://groups.google.com/forum/#!forum/webmock-users

bblimke commented 5 years ago

@Adrian1707 I think it's a fine case for an issue. Thank you for the blogpost, which clearly describes the problem and the solution.

It would be great to have support for that built in WebMock, rather than having users to invoke the check_request_body_diff method.

Perhaps WebMock should automatically show the diff for each registered stub if the uri and method matches and the diff is small. Defining what is "small" may be tricky.

odinhb commented 9 months ago

I'm currently drowning in XML and realized that while webmock has a (perhaps appropriately poorly advertised) configuration flag show_body_diff, it only diffs JSON or hashes (depending on the Hashdiff gem).

I'm wishing Webmock at least did a plain-text diff as a fallback, if not outright supporting other data such as xml.

Any thoughts on this?

odinhb commented 8 months ago

I created a gem which monkey-patches WebMock to provide plaintext diffs (in addition to the json/hash/array diffs from hashdiff). Because my XML happens to be indented this was enough to swim to the surface of Lake XML.

For non-indented XML (not to mention other types of data) I'm toying with a plugin interface to define your own differs.

I would appreciate any feedback on the gem (it works for me), and my approach to the problem.

bblimke commented 6 months ago

@odinhb thank you for re-raising the issue. I appreciate you solution implemented as specdiff. I can see you put some effort and though to implement it.

By just looking at the README or by looking in your code, I find it difficult to immediately see how the gem improves the output. I believe it would be great to provide a few OUTPUT examples in the README, to show the difference between what failure output is provided by WebMock, by default, compared to output produced when specdiff is used.

I agree it's worth adding support to WebMock, at least for XML requests. Would you consider adding support directly to WebMock or keep it as a separate gem?

odinhb commented 6 months ago

@bblimke Thanks for the feedback on the README, I've added some screenshots of my terminal. Let me know if you have any more thoughts.

My assessment of the situation is basically this:

My vision for specdiff was that it would improve text diffing in webmock while also improving nested json/hash/array diffing in rspec, kinda "cross-pollinating" the projects. Therefore webmock would depend on specdiff to do the type detection and diffing. (this means webmock starts ignoring the content-type header for this purpose) I haven't got around to monkey-patching rspec yet though. :sweat_smile:

When thinking about XML support my mind immediately jumps to some pretty heavy solutions, such as parsing the xml with nokogiri. This is why I constructed a plugin architecture, which could allow individual projects to implement their own xml differ (or missing diffing logic for any format, really). This way specdiff could bring some improvements by default and also potential for bespoke, per-project improvements or unusual datatypes.

We used specdiff in an internal project and promptly forgot about adding a plugin for XML, since the text differ (which is basically what rspec already implements) is sufficient (as long as the XML is indented). Thus we had diff parity between rspec and webmock (since there wasn't much json in that project).

I don't quite consider specdiff "finished". It is highly experimental, I'm not even sure if the hashdiff output should be printed as text or not? Is that even better? I need to try it in a real-world scenario where I have a big nested structure and the text diff would be inconvenient. (Many such cases in our projects, but I need to dig them up and try it out)

Also, is the ruby version support for specdiff suitable for webmock? spec.required_ruby_version = ">= 3.0.0" vs s.required_ruby_version = '>= 2.5'. I think It would be good to support some of these older ruby versions, but I believe this is non-trivial.

In conclusion: I'd like to replace the dependency from webmock on hashdiff with a dependency on specdiff, which will in turn depend on hashdiff and diff_lcs. I need to fix a few odds and ends in specdiff before it would be ready for depending on. I'm gonna pitch working on specdiff to my boss and see if I can make some progress. I'm willing to (eventually) create a PR to make the monkey-patch obsolete.

bblimke commented 6 months ago

@odinhb, thank you for providing a comprehensive explanation. The inclusion of screenshots has significantly enhanced my understanding of specdiff's purpose.

As specdiff progresses towards stability, I believe it would be advantageous to explore the possibility of implementing diff adapters. This approach would enable WebMock to seamlessly integrate with specdiff or other similar tools without resorting to monkeypatching. It could offer a more robust and adaptable solution for future compatibility and ease of integration.

odinhb commented 3 months ago

Just wanted to write a quick update to say I've worked a bit more on the gem and we have started using it more in our projects with very promising results. Still no ruby 2.7.0 or ruby 2.5.0 support though.

Also, I'm a little unsure what exactly you mean by "diff adapters". Could you elaborate? The interface that would need to be implemented against in WebMock. Is there something specific you would want to do here? @bblimke

bblimke commented 3 months ago

@odinhb by diff adapters I mean providing a plugin interface, so that instead of you monkeypatching RequestBodyDiff, WebMock would proviode an interface to replace default diff with the one from the plugin, example specdiff.