Feature request: how about making verify_stubbed_calls verify the parameters of stubbed requests?

[yykamei]

verify_stubbed_calls is a useful method to stub whether a request is exactly called within a production code.
However, I found out it does not verify the parameters of a request though it does for the request path.

For example, I'm expecting this test to fail because the new parameter foo is added to sushi, but the stubbed Faraday connection is expecting that the only abc exists in its params. However, the current implementation seems to pass the test.

class Client
  def initialize(conn)
    @conn = conn
  end

  def sushi(jname, **params)
    res = @conn.get("/#{jname}", **params)
    data = JSON.parse(res.body)
    data['name']
  end
end

RSpec.describe Client do
  let(:stubs)  { Faraday::Adapter::Test::Stubs.new }
  let(:conn)   { Faraday.new { |b| b.adapter(:test, stubs) } }
  let(:client) { Client.new(conn) }

  it 'verifies the all parameter values are identical' do
    stubs.get('/ebi?abc=123') do # <-- This expects only `abc` added, not including `foo`. Currently, this test unexpectedly succeeds!
      [
        200,
        { 'Content-Type': 'application/javascript' },
        '{"name": "shrimp"}'
      ]
    end

    expect(client.sushi('ebi', abc: 123, foo: 'Kappa')).to eq('shrimp')
    stubs.verify_stubbed_calls
  end
end

I succinctly came up with an idea to check all parameter keys are identical to the request parameters like this, but it would cause the existing tests to fail because of such a strict stubbing.

diff --git a/lib/faraday/adapter/test.rb b/lib/faraday/adapter/test.rb
index 0230b85..739b7ec 100644
--- a/lib/faraday/adapter/test.rb
+++ b/lib/faraday/adapter/test.rb
@@ -184,6 +184,8 @@ module Faraday
         end

         def params_match?(request_params)
+          return false unless Set.new(params.keys) == Set.new(request_params.keys)
+
           params.keys.all? do |key|
             request_params[key] == params[key]
           end

What do you think? This idea might bring breaking changes but would be useful when it comes to verifying the parameters.

[iMacTia]

Hi @yykamei and thanks for raising this issue!
I had a quick look at the test adapter and I believe we suffer from the same issue also on the headers side!
When we fix this issue for params, it would be great to do the same for headers as well.

That said, you also correctly pointed out my main concern:

This idea might bring breaking changes
I completely agree this would be useful, but the only requirement I'd have is that it would be opt-in.
In order to achieve that, we could introduce a strict_mode on the stubs object that can be arbitrarily enabled. When this is enabled, params and headers will be checked for an exact match.
By default, it will be disabled and behave like it does today.

How does that sounds? I'm open to other suggestions

[yykamei]

Hi @iMacTia,

Thank you for your quick reply!
Yes, I think so. headers can be also checked more strictly as well.

strict_mode is fantastic! I couldn't conceive such an opt-in solution. This is great 👍

[yykamei]

I opened a PR to address this issue on #1298. I hope this would be applicable to the current Faraday implementation.

[iMacTia]

Fantastic work @yykamei 👏
PR merged, closing this