@mohammednasser-32 Many thanks for submitting this - at first glance it looks like you've been very thorough with the tests and documentation, so thank you for that! I'll try to find some time soon to review it properly and get it merged and released.

@floehopper Thank you so much 🙏 once the approach is validated I can do the same for has_exact_entries

@mohammednasser-32

Thanks again for your work on this. I wanted to let you know that I have started to review the PR. However, I noticed that the implementation of IncludesExactly#matches? does not use the #include? predicate method, which seems a bit surprising to me. Was that intentional or just a natural result of trying to get all the tests passing?

Also I'm wondering why we can't configure the existing Includes matcher with an optional exact argument like we do for HasEntries:

In fact I have had an initial attempt at using this approach locally and my implementation is passing all but two of the unit tests. The failing tests have made me wonder a bit about the semantics of this matcher and what the least surprising behaviour would be. I apologise that I hadn't thought this through further before I gave you the green light to work on it.

Unfortunately I've run out of time to look at it today, but I'll have a think about it overnight and take another look tomorrow. I haven't yet had a close look at the examples you've included in the YARD docs for the new matcher, so perhaps they will clarify some things for me.

@floehopper
Thank you so much for your precise review 🙏
It was intentional not to use #include? as an attempt to save an extra loop.

The goal of the matcher is to check if two objects that respond to include are identical regardless of the order, so my first approach to achieve that was:

@items.each do |item| #loop 1
  if parameters.include?(item) #loop 2
    item_index = parameters.index(item) #loop 3
    parameters.delete_at(item_index) #I am deleting by index here so I only delete the first occurrence
  end
end

 parameters.empty? #if the two arrays are identical, we should have deleted all occurances

so as an attempt to improve that I changed it to

  @items.each do |item| #loop1
      matched_index = parameters.each_index.find { |i| item.to_matcher.matches?([parameters[i]]) } #loop2 
      #now we are checking inclusion and getting index in the same loop
      return false unless matched_index
  
      parameters.delete_at(matched_index)
  end

  parameters.empty?

I share the same doubts of the precise expectations of the matcher, if it is supposed to support only arrays/strings/hashes (which was my first impression)..we could just sort them and check for equality..however I tried to make it as close as possible to Includes matcher by supporting matchers and regex too, and that`s why I went with that approach.

@mohammednasser-32

Thanks for explaining how you came up with your solution - it was very helpful.

Having thought about it afresh this morning and discussed it with a colleague, I'm not convinced that an exact matcher relating to the #includes? predicate makes complete sense - mainly because of the String case.

So for now, I don't think I want to accept this PR. I apologise again for not fully thinking it all through before encouraging you to do the work. I need to give it some more thought and unfortunately I don't have loads of time at the moment...

Thinking back to your original motivation in #673, did you know you can pass a block to Expectation#with to effectively make a custom matcher? I know this isn't perfect, but it might unblock your current problem...

require "bundler/setup"
require "minitest/autorun"
require "mocha/minitest"

class FooTest < Minitest::Test
  def test_matches
    object = mock("object")
    object.expects(:foo).with(&matches_set([1,2]))
    object.foo([2,1])
  end

  def test_does_not_match
    object = mock("object")
    object.expects(:foo).with(&matches_set([1,2]))
    object.foo([2,1,3])
  end

  private

  def matches_set(expected)
    -> actual { expected.to_set == actual.to_set }
  end
end

Hey @floehopper , sure no worries..and thanks for your suggestion! 🙏

Closing this for now - we still have #673 as a reminder