I’m also struggling to understand how this works.
We do pass expected and actual there, and they end up wrapped in an array when flatten is called on it?

Indeed... that's weird... What if I remove the star operator from #no_procs and no_numbers method?

The purpose of this no_procs_and_no_numbers? method is to reduce the number of && operators used in the #diff method, thus appeasing rubocop by removing the Metrics::PerceivedComplexity offense.

args here will be an array of two elements?
How does it work to flatten an array of two hashes?

Can we check no_procs?(expected) && no_procs?(actual) && no_numbers? ….

Each if those methods runs the (potentially expensive?) safe_flatten. I’d prefer this to be done just once (or, better - never!)

Can’t we just take top-level keys of each of those arrays without flattening recursively?

This would work on 1.8.7
No tricks with has/array structures
Early exit if there is a proc/number
No extra checks in nested values that we don’t care about
Less code
No splatting/desplatting

Now when I look at this, this is indeed and improvement to -anything_key +anything_key we had.
But do we need a potentially huge value here? An uuid is fine, but what about a 5kb json-like thing?

Most certainly if we’ve anythinged it in the test, we don’t care what’s inside, including the diff?

@JonRowe does this make sense?

Are you concerned by the case a 5kb json-like thing would be copied into the expected -> anything value? that will definitively be a performance issue. Would it be worth doing the change? Is there a way we can make sure the 5kb json-like thing will be a shallow copy? Maybe a shallow copy will mitigate the performance issue.

Maybe a shallow copy will not impact memory ram consumption. But what about CPU time? does diff_as_object iterates the actual and expected vars regardless of their .object_id? or it checks the object_id and if they match it will cease any iteration?

Another question: rspec-support module should be allowed to mutate expected vars? regardless they will be later on return them to their original state? My proposed changes on this PR are a hacky way to fold noise when the developers use anything fuzzy matcher on their hash, I understand this may not be allowed to do here.

Apparently, a 5kb json-like thing will impact CPU time ONLY if the matcher fails.

  def diff_as_object(actual, expected)
    actual_as_string = object_to_string(actual) # STEP 1
    expected_as_string = object_to_string(expected) # STEP 2
    diff_as_string(actual_as_string, expected_as_string) # STEP 3
  end

STEP 1 and STEP 2 will get the string representation of 5kb json-like thing in two different strings. STEP 3 will perform the comparison of both strings. This will be an expensive operation because both strings will be huge. In contrast, without the changes on this PR, comparing the string representation of this 5kb json-like thing with anything fuzzy matcher would have been easy-peasy

AFAIK, RSpec::Support::Differ is called when an expectation matcher fails AND the expectation matcher responds to diffable?. If the matcher does not responds to diffable, expected and actual vars are assigned nil and RSpec::Support::Differ will not perform any action at all.

My assessment is this will impact performance only when the test fails and the developer is using a matcher that produces an output-diff. But this is my assessment about performance, I still don't know the answer to: should rspec-support module be allowed to mutate expected vars in the developer test bench (ie, his _spec.rb files)? or rspec-support should remain a "pure" read-only module?

I think @pirj's concern was about larger diffs, but as things should be identical the differ will often not print anything at all

Identical - yes. But if it’s neighbouring with the difference, like the ‘anything_key’ here on line 567, and it is big, i would prefer to see the literal “anything” to be printed rather than 5kb of json.