dodoas / specdiff

https://rubygems.org/gems/specdiff
MIT License
1 stars 0 forks source link

Improve diff when using match RSpec matcher #5

Open stoivo opened 1 month ago

stoivo commented 1 month ago

Improve diff when using match RSpec matcher

Given the following test

    it "is" do
      expect({
        "cre" => "2",
        "created_at" => "2024",
      })
        .to match("created_at" => /\d+/)
    end

We get the following output

    1) Journal is
       Failure/Error:
         expect({
           "cre" => "2",
           "created_at" => "2024",
         })
           .to match("created_at" => /\d+/)

         expected {"cre"=>"2", "created_at"=>"2024"} to match {"created_at"=>/\d+/}
         Diff:
         @@ +1/-0/~1 @@
           extra key: "cre" ("2")
           new value: "created_at" (/\d+/ -> "2024") 

If we add "cre" => "2", to the expectation it passes

    it "is" do
      expect({
        "cre" => "2",
        "created_at" => "2024",
      })
        .to match("cre" => "2", "created_at" => /\d+/)
    end

Same thing for BigDecimal

  expect({
    "cre" => "2",
    "created_at" => BigDecimal(10),
  })
    .to match("created_at" => 10)
  # new value: "created_at" (10 -> #<BigDecimal: 10.0>)

Same thing for Comparing against class

  expect({
    "cre" => "2",
    "created_at" => BigDecimal(10),
  })
    .to match("created_at" => BigDecimal)
  # new value: "created_at" (BigDecimal -> #<BigDecimal: 10.0>)

Proposal It would be nice if we could hide new value: "created_at" (/\d+/ -> "2024") from the diff since it is actually matching. Current diffing logic don't take which matcher into account

Other notes If we figure out how we can get which matcher was i use we might want to make some changed to how we represent include diff?

expect({
  "cre" => "2",
  "created_at" => BigDecimal(10),
})
  .to include("a"=> 1,"created_at" => BigDecimal)
expected {"cre" => "2", "created_at" => #<BigDecimal: 10.0>} to include {"a" => 1}
Diff:
@@ +1/-1/~1 @@
missing key: "a" (1)
  extra key: "cre" ("2")
  new value: "created_at" (BigDecimal -> #<BigDecimal: 10.0>)

include matcher also uses this diff which isn't good

expect(["1", "2"])
  .to include("2", "3")
expect(["1", "2"])
 .to include("2", "3")

expected ["1", "2"] to include "3"
Diff:
@@ +0/-0/~2 @@
 new value: [0] ("2" -> "1")
 new value: [1] ("3" -> "2")
stoivo commented 1 month ago

I did a little bit of digging and I think we might have to patch https://github.com/rspec/rspec-expectations/blob/main/lib/rspec/expectations/handler.rb#L32-L42 too. I think this is the closed point where we have access to which matcher we have used.

I put a puts caller[0..10] in RSpec::Support::Differ.diff to see callstack

/3.2.0/gems/rspec-expectations-3.12.3/lib/rspec/matchers/expecteds_for_multiple_diffs.rb:70:in `block in diffs'
/3.2.0/gems/rspec-expectations-3.12.3/lib/rspec/matchers/expecteds_for_multiple_diffs.rb:69:in `map'
/3.2.0/gems/rspec-expectations-3.12.3/lib/rspec/matchers/expecteds_for_multiple_diffs.rb:69:in `diffs'
/3.2.0/gems/rspec-expectations-3.12.3/lib/rspec/matchers/expecteds_for_multiple_diffs.rb:48:in `message_with_diff'
/3.2.0/gems/rspec-expectations-3.12.3/lib/rspec/expectations/fail_with.rb:33:in `fail_with'
/3.2.0/gems/rspec-expectations-3.12.3/lib/rspec/expectations/handler.rb:38:in `handle_failure'
/3.2.0/gems/rspec-expectations-3.12.3/lib/rspec/expectations/handler.rb:56:in `block in handle_matcher'
/3.2.0/gems/rspec-expectations-3.12.3/lib/rspec/expectations/handler.rb:27:in `with_matcher'
/3.2.0/gems/rspec-expectations-3.12.3/lib/rspec/expectations/handler.rb:48:in `handle_matcher'
/3.2.0/gems/rspec-expectations-3.12.3/lib/rspec/expectations/expectation_target.rb:65:in `to'
/3.2.0/gems/rspec-expectations-3.12.3/lib/rspec/expectations/expectation_target.rb:101:in `to'