req icon indicating copy to clipboard operation
req copied to clipboard

`Req.Test`: stub anything

Open wojtekmach opened this issue 7 months ago • 7 comments

In https://github.com/wojtekmach/req/pull/349/files#diff-455e75050e3f9495a2db20296942753a0f4b6e1a65b96d48127d119a60e38cc4R361 we added a restriction that we can only stub and and expectations for plugs.

@whatyouhide so on Livebook we just had a PR where it would be really convenient if we didn't have this restriction. To give some context, we're integrating with Fly API. Most of the time we want to be hitting a stub. But in integration tests we want to hit the real thing. Our unit and integration tests are both in Mix.env() == :test.

My idea was the following: https://github.com/livebook-dev/livebook/pull/2708/files#diff-489b337badb7f480b22ff4c701659f5aef800fb470112d4ddf51f39256c2bbbeR249-R273

and then in unit test we'd do:

Livebook.FlyAPI.stub(fn conn -> ... end)

and in integration tests:

Livebook.FlyAPI.passthrough()

(Of course these test-only helpers are totally optional, the more verbose Req.Test.stub(Livebook.FlyAPI, fn conn -> ... end) and Req.Test.stub(Livebook.FlyAPI, :passthrough) is fine too.)

if we forget to set a stub whenever we call FlyAPI in Mix.env :test, we'd get a crash.

When we discussed making Req.Test.stub/2 stricter, our consensus was let's keep it focused on plugs so people don't abuse this feature to stub random unrelated things willy nilly. People can use Mox and/or NimbleOwnership too. That being said, I think this is a legitimate use case that I'd like to support directly in Req.

I think the easiest thing to do is lift the restriction and add Req.Test.fetch_stub!(name) so we could implement the above mentioned code using public API. Dunno if we add Req.Test.fetch_expectations!(name) and what are the semantics.

Another idea is to make stubs still stricter but in a different way. Instead of stubbing plugs we stub Req options, usually plug: .... And so:

# fly_api.ex
def new do
  Req.new(test_options())
end

if Mix.env() == :test do
  defp test_options do
    Req.Test.fetch_stub!(__MODULE__)
  end
else
  defp test_options do
    []
  end
end

# unit test
Req.Test.stub(FlyAPI,
  plug: fn conn ->
    conn
  end
)

# integration test
Req.Test.stub(FlyAPI, [])

Maybe this is weird because vast majority of time people would be in fact stubbing plugs.

Anyway, regarding stubbing options, we could even do something like:

# config/test.exs
config :livebook, fly_api_req_options: [test_options: FlyAPI]

# fly_api.ex
def new do
  Req.new(Application.fetch_env!(:livebook, :fly_api_req_options))
end

# unit test
Req.Test.stub(FlyAPI,
  plug: fn conn ->
    conn
  end
)

That is, there is a new fetch_test_options step (which only makes sense as the very first step and we'd put it as such) and a corresponding test_options: name option which fetches options from stub/mock with that name. And so there is no longer need for plug: {Req.Test, name} which always felt tiny bit hacky. I don't know if I really like this approach but thought there's maybe something to it?

Btw, since day 1 we have a Req.default_options() and Req.default_options(options) which simply store stuff in app env. I used it in my .iex.exs and occasionally in test/test_helper.exs. This is completely global so at the moment doesn't play well with multiple Req clients in a single project but maybe using/changing this feature can be helpful in what we want to achieve.

cc @jonatanklosko

wojtekmach avatar Jul 16 '24 08:07 wojtekmach