search

splitwise / super_diff

A more helpful way to view differences between complex data structures in RSpec.
https://splitwise.github.io/super_diff/
MIT License
986 stars 53 forks source link

What should happen when you use an `anything` fuzzy matcher inside a Hash on your specs? #241

Open KarlHeitmann opened 6 months ago

KarlHeitmann commented 6 months ago

Hi!

I've opened a PR in rspec-support with a hacky fix to the problem you get when you compare an actual hash with a expected hash and they differ, but the expected hash includes some values whose values correspond to the anything matcher.

My PR contains some tests showing the failure and the fix. But the code changes I've made on the rspec differ engine is trivial:

  1. Navigate the expected hash and retrieve all keys you must dig in that hash in order to find values that are anything fuzzy matcher.
  2. Use these keys to fetch the data on the actual hash, and replace the anything values on your expected hash by the values you fetched on the actual hash.
  3. Later, when the diff string is generated, with the morph you made on the expected var on step (2), the diff string will NOT calculate the diff on the key-value pairs you had before the anything fuzzy matcher. The result is you will have a clean diff showing you only the right values that differ and you are interested in.

I'd like to implement this behavior on super_diff, but I know my proposal may be hacky. What are your thoughts about it? Any feedback is appreciated.

I am reading your specs and the source code of super_differ, I'm looking forward to collaborate here or in rspec-support, because diffs is a topic that I found very interesting.

Best.

mcmire commented 6 months ago

Hi @KarlHeitmann! I've taken a look at your PR over on rspec-support and I think the behavior you demonstrate in the tests there make sense to me. I don't believe that super_diff handles anything yet, but I also think there's a way to implement it here without hacks. I can give you some pointers if you need help.

KarlHeitmann commented 6 months ago

Hey @mcmire ! Thank you! I'll implement anything handle in your project this week or next week. I took a sneak peak at your tests and understand how you are writing them. I also need to know in which Differ I should implement the anything handle.... I'll explore it the next days in my spare time and I'll let you know how the progress will be going on.

See you later!

KarlHeitmann commented 6 months ago

Hey @mcmire ! I wanted to catch up / update my status.

I've read your guides and now I have a slightly better understanding of RSpec. I've noted you wrote some specs about fuzzy finding, but you wrote these examples using the include matcher. I've had problems with the match matcher instead, and that's what I am trying to fix. (eg: expect(actual).to match(expected)). Here is the example I found on the source code: https://github.com/mcmire/super_diff/blob/92253b57854d36bd27fdb3c370fdb45df26c608f/spec/integration/rspec/include_matcher_spec.rb#L266 I wrote a similar example on my local machine, but I'm using match instead of include matcher.

Do you have any tips to debug the source code? Since you are running the examples inside a TestPrograms::Plain class wrapper, it is difficult to put a binding.pry to tinker with the variables and understand what is happening there. I discovered some of your wrapper classes write a temporary file ./tmp/integration_spec.rb that I can run with this command: bin/rspec tmp/integration_spec.rb. This way, it is easier to write a binding.pry statement. Do you also use this method to debug the source code? Or do you have another way / trick to do it?

I understand there are integration tests and unit tests. The integration tests are "end to end", I mean: on one end you've got the input data (ie: the variables you want to diff) and you check the right output data (ie: the diff string) at the other end.

To me, integration tests are great to start looking under the hood of a project. Because there I've got the overview of what is happening. But right now, I am thinking to myself that ~ maybe ~ it is a good idea to tackle this issue by starting with a unit test, because I ~ believe ~ my code change should be small. What do you think? It is a good or bad idea to fix the issue by working on a unit? I think there is a good chance I may be underestimating the issue.

I grepped the source code with: rg TieredLinesFormatter spec/unit/ (TieredLinesFormatter is one of the classes involved with my issue) and found only one relevant match: https://github.com/mcmire/super_diff/blob/92253b57854d36bd27fdb3c370fdb45df26c608f/spec/unit/core/tiered_lines_formatter_spec.rb#L4 , and there is only one test there. I've also grepped to_diff inside the spec/unit/ folder and found no matches at all. What does this result means? My conclusion is that even though #to_diff methods are ~ important ~ and they are defined and used in AbstractDiffer and AbstractOperationTree classes, they don't have any unit test at all either because there is some meta programming going on, or because they involve multiple layers of different classes interacting with each other that it is only worth test the to_diff method with integration. Am I right or wrong with this statement?