arrays_match? is not indifferent to array order

[lake-effect]

Subject of the issue

Support::FuzzyMatcher.arrays_match? is not indifferent to array ordering. This affects a number of things upstream that depend on it such as constructions like expect(SomeClass).to receive(:method).with(..., some_array) that developers might use, unaware that if some_array is different than what SomeClass might receive in test.

Normally we would prevent this by using a Rubocop rule like MatchArray but I think this example will go undetected.

Attempted fixes

1. Add return true if (expected_list - actual_list).empty? to arrays_match? This works until object hashes are different (as in the below example).
2. Try sorting both lists before zipping This doesn't work when there is no order comparison between types, also in below example.
3. Converting both lists to sets and then trying a set difference This also fails with different object hashes.

Your environment

• Ruby version: 3.1.2
• rspec-support version: d43930e

Steps to reproduce

Example of failing test

# frozen_string_literal: true
#
# NOTE: Run from rspec-support/.

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"

  gem "rspec", "3.7.0" # Activate the gem and version you are reporting the issue against.
end

puts "Ruby version is: #{RUBY_VERSION}"
require 'rspec/autorun'

require 'spec/rspec/support/spec'
require 'spec/rspec/support/fuzzy_matcher'

RSpec.describe FuzzyMatcher, ".values_match?" do
  matcher :match_against do |actual|
    match { |expected| FuzzyMatcher.values_match?(expected, actual) }
  end

  it 'is indifferent to element order' do
    a1 = [
      [String, Integer, [be_within(0.1).of(2)]],
      3, [[[ /foo/ ]]]
    ]

    a2 = [
      3, [[[ /foo/ ]]],
      [String, Integer, [be_within(0.1).of(2)]]
    ]

    expect(a1).to match_against(a2)
  end
end

Expected behavior

Devs at my current company are clearly expecting a1 to match a2 when they write a .with expression that ultimately ends up calling FuzzyMatcher.

Actual behavior

FuzzyMatcher zips both arrays together (where the order dependency comes in) and then fuzzy matches recursively on the elements.

[pirj]

I'd expect .with([1, 2, 3]) to only match when the method was called with exactly [1, 2, 3], like with an .with(eq([1, 2, 3]). To get the desired order indifference, you may use match_array/contain_exactly as an argument matcher: .with(contain_exactly(3, 2, 1)). Defaulting to order indifference seems to be a breaking change to me, and it affects usages of FuzzyMatcher here and there. There's no clear way to make matching order sensitive apart from explicitly using eq(ary) which is not quite obvious.

contain_exactly contains some sophisticated logic to make sure elements are matched despite type differences. More on this topic is in this fantastic PR that dramatically improves performance with long arrays.

[JonRowe]

This is a private api, and I'd expect your example to fail anyway, closing here but if you can demonstrate an issue with the upstream api's that we agree is broken I'll reconsider

[lake-effect]

@JonRowe Gotcha. I'll take the example of expect(SomeClass).to receive(:method).with(..., some_array) upstream, since that's where the FuzzyMatcher gets put on the stack.

[lake-effect]

It looks like .with(array_including(...)) is what should be used, but I've never seen it in practice. https://github.com/rspec/rspec-mocks/blob/af013b424cbff4beb13a1fc2d87c781fe6f9fa1d/lib/rspec/mocks/argument_matchers.rb#L74-L83

[JonRowe]

I'm not clear on what your original issue was:

expect(SomeClass).to receive(:my_method).with(:a, :b)

Will only match:

SomeClass.my_method(:a, :b)

Where :a and :b are arguments to the method, thats naturally order dependent and should fail if you tried:

SomeClass.my_method(:b, :a)

If you're using a matcher e.g.

expect(SomeClass).to receive(:my_method).with(:a, my_matcher)

Then the ordering here is still the same, it should only pass with:

SomeClass.my_method(:a, something_satisifies_my_matcher)

Dence why I closed this, argument order is always dependent and your example should fail because you're matching things in the wrong order.

By default Ruby matches literal arrays, (e.g. if :b / something_satisifies_my_matcher were [:a, :b, :c]) as being exact matches only, so they are also order dependent.

If you use a matcher like contain_exactly(:a, :b, :c) as my_matcher then that would then match any combination of :a, :b, :c e.g.

it 'will pass' do
  expect(SomeClass).to receive(:my_method).with(:a, containing_exactly(:a, :b, :c))
  SomeClass.my_method(:a, [:c, :a, :b])
end

Again this is all by design because usually array order is important and we use Ruby for literal matching wherever possible so the ordering has to match, or we provide matchers for when you want a derivative to pass or a subset to pass.

[lake-effect]

Just for clarity, the original issue is something like this (proprietary info removed):

#<Thinger (class)> received :method with unexpected arguments
  expected: (arg1, [:a, b])
  got: (arg1, [:b, a])

# rspec backtrace follows

I'm surprised that array order is normally regarded as important in Ruby since it's such a common place for flaky tests to appear in many companies I've worked at, but I'll record that contain_exactly is also acceptable along with array_including.

[JonRowe]

Arrays have order, thats just how they are, I agree its common to see them being a source of flaky tests, but its often because people have forgotten to properly order a query rather than the arrays themselves at fault.