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

Header matching inconsistency with strict_mode and verify_stubbed_calls #1488

Open
grjones opened this issue Jan 24, 2023 · 4 comments
Open

Comments

@grjones
Copy link

grjones commented Jan 24, 2023

Basic Info

  • Faraday Version: 2.7.4
  • Ruby Version: 3.1.2p20

Issue description

When adding rspec tests using Faraday's test facility, an error is falsely raised when the following are true:

  1. request(:json) is set
  2. 'Content-Type': 'application/json' is set in a :test adapter stub
  3. use_strict = true is set in the :test adapter
  4. verify_stubbed_calls is called

The resulting error is:

Faraday::Adapter::Test::Stubs::NotFound: no stubbed request for ...

This appears to be due to inconsistent header case normalization and case sensitive header matching in strict stubs.

Explicitly setting the Content-type header (with lowercase -type) is a workaround like so:

Faraday.new(url:) do |f|
  f.request(:json)

  # work-a-round
  f.headers = { 'Content-type' => 'application/json' }
end

Steps to reproduce

Create a faraday object like so:

client = Faraday.new(url: <your_base_url>) do |f|
  f.request(:json)
end

Set the expected stubs:

headers = { 'Content-Type': 'application/json' }

client.adapter(:test) do |stub|
  stub.post('/api/push', {}, headers) { [200, {}, {}] }
  stub.strict_mode = true
end

Use the client:

client.post('/api/push') do |req|
  req.body = {}
end

Run verify_stubbed_calls and observe the unexpected failure:

client.adapter(:test).build.stubs.verify_stubbed_calls
@yykamei
Copy link
Contributor

yykamei commented Jan 25, 2023

Hi, thank you for reporting the issue.

There might be problems on Faraday::Utils::Headers; it can't handle Symbol header including - here.

def []=(key, val)
key = KeyMap[key]
key = (@names[key.downcase] ||= key)
# join multiple values with a comma
val = val.to_ary.join(', ') if val.respond_to?(:to_ary)
super(key, val)
end

Also, testing adapter does not type-cast non-String body as String, so stub.post('/api/push', {}, headers) should be stub.post('/api/push', '{}', headers) (I think testing adapter can cast the value 😅 )

I think you don't have to explicitly set Content-Type in the block of Faraday.new. Instead, the workaround is like this:

headers = { 'Content-Type' => 'application/json' }

client.adapter(:test) do |stub|
  stub.post('/api/push', '{}', headers) { [200, {}, {}] }
  stub.strict_mode = true
end

Here is the change:

diff --git a/a.rb b/a.rb
index a0a1cba..ef6599a 100644
--- a/a.rb
+++ b/a.rb
@@ -1,6 +1,6 @@
-headers = { 'Content-Type': 'application/json' }
+headers = { 'Content-Type' => 'application/json' }

 client.adapter(:test) do |stub|
-  stub.post('/api/push', {}, headers) { [200, {}, {}] }
+  stub.post('/api/push', '{}', headers) { [200, {}, {}] }
   stub.strict_mode = true
 end

@grjones
Copy link
Author

grjones commented Jan 25, 2023

Hi @yykamei. Yes, that also works. The bug though is that you'd want request(:json) to be able to work, which should be taking care of encoding the request body as json and setting the correct content-type. So, yours is also a workaround but without the request(:json), you'll need to encode as json your self. Eg:

stub.post('/api/push', payload.to_json, headers) { [200, {}, {}] }

@grjones
Copy link
Author

grjones commented Jan 25, 2023

Actually, we're both partly right. It is indeed that symbols aren't supported in the testing framework. So your headers change works. But, f.request(:json) also works. So, my working version is:

client = Faraday.new(url: <your_base_url>) do |f|
  f.request(:json)
end

headers = { 'Content-Type' => 'application/json' } # no longer a symbol

client.adapter(:test) do |stub|
  stub.post('/api/push', {}, headers) { [200, {}, {}] }
  stub.strict_mode = true
end

Thanks for your help. I don't know if this should just be closed? Maybe others would get tripped up by the symbol comparison.

@yykamei
Copy link
Contributor

yykamei commented Jan 25, 2023

Maybe, I understood your point; the testing adapter should consider the JSON middleware when comparing the actually used request header with expected one. If developers use request(:json), testing adapter should also encode the passed stub body as JSON. Is it correct?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants