Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: support Faraday::FlatParamsEncoder in Faraday::Adapter::Test #1312

Closed
yykamei opened this issue Aug 19, 2021 · 4 comments
Closed

Comments

@yykamei
Copy link
Contributor

yykamei commented Aug 19, 2021

#1291 introduced strict_mode in Faraday::Adapter::Test::Stubs, and I want more features for the mode: supporting Faraday::FlatParamsEncoder. For example, if I want to call an HTTP request with the same query parameter a multiple times, I expect the test stub to check whether the same parameters with different values are all correctly passed.

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(strict_mode: true) }
  let(:conn)   { Faraday.new(request: { params_encoder: Faraday::FlatParamsEncoder }) { |b| b.adapter(:test, stubs) } }
  let(:client) { Client.new(conn) }

  it 'handles the same multiple URL parameters' do
    # The same parameter `a` is expected to appear multiple times with different values.
    stubs.get('/ebi?a=x&a=y&a=z') { [200, { 'Content-Type' => 'application/json' }, '{"name": "shrimp"}'] }

    # Test should pass
    expect(client.sushi('ebi', params: { a: %w[x y z] })).to eq('shrimp')

    # Test should not pass due to the lack of "x" and "y" for `a`
    # expect(client.sushi('ebi', params: { a: %w[z] })).to eq('shrimp')
    stubs.verify_stubbed_calls
  end
end

What do you think? I want to open a pull request if it makes sense.

@iMacTia
Copy link
Member

iMacTia commented Aug 19, 2021

Hi @yykamei, the above makes sense to me, I'd welcome a PR.
Out of curiosity, what happens currently with the tests above? Is the last one passing?

@iMacTia
Copy link
Member

iMacTia commented Aug 19, 2021

Also, remember that main branch is now pointing at the upcoming v2.0 release.
If you'd like this in a 1.x release as well, you'll need to branch off the 1.x branch 👍

@yykamei
Copy link
Contributor Author

yykamei commented Aug 20, 2021

Hi @iMacTia, thanks for your reply.

Out of curiosity, what happens currently with the tests above? Is the last one passing?

Oh, it seems not to pass in the latest 1.x branch. I think this is because Stub#params is constructed with Faraday::Utils.parse_nested_query; it looks like overriding previous query parameters.

Randomized with seed 45661

Client
  handles the same multiple URL parameters (FAILED - 1)

Failures:

  1) Client handles the same multiple URL parameters
     Failure/Error:
       raise Stubs::NotFound, "no stubbed request for #{env[:method]} "\
                              "#{normalized_path} #{env[:body]}"

     Faraday::Adapter::Test::Stubs::NotFound:
       no stubbed request for get /ebi?a=x
     # ./lib/faraday/adapter/test.rb:253:in `call'
     # ./lib/faraday/rack_builder.rb:154:in `build_response'
     # ./lib/faraday/connection.rb:511:in `run_request'
     # ./lib/faraday/connection.rb:200:in `get'
     # ./ok.rb:11:in `sushi'
     # ./ok.rb:30:in `block (2 levels) in <top (required)>'

Finished in 0.00391 seconds (files took 0.78136 seconds to load)
1 example, 1 failure

Failed examples:

But, if I change the code like this, the test will pass. I rewrote the issue description.

diff --git a/a.rb b/a.rb
index c4b7084..91d795d 100644
--- a/a.rb
+++ b/a.rb
@@ -27,7 +27,7 @@ RSpec.describe Client do
     expect(client.sushi('ebi', params: { a: %w[x y z] })).to eq('shrimp')

     # Test should not pass due to the lack of "y" and "z" for `a`
-    expect(client.sushi('ebi', params: { a: %w[x] })).to eq('shrimp')
+    expect(client.sushi('ebi', params: { a: %w[z] })).to eq('shrimp')
     stubs.verify_stubbed_calls
   end
 end

Thanks anyway. I'm going to make some changes 💪

@yykamei
Copy link
Contributor Author

yykamei commented Sep 1, 2021

This issue was resolved with #1316.

Thank you!

@yykamei yykamei closed this as completed Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants