Mock no longer works on spawned processes

[yastanotheruser]

Since version 1.14.0, Tesla.Mock.mock has stopped working for processes spawned by a test or a callback. I think this is a regression due to #750.

Reproduce

Version 1.13

# tesla_1.13_mock.exs
Mix.install([
  {:jason, "~> 1.4"},
  {:tesla, "~> 1.13.0"}
])

Application.ensure_all_started(:tesla)
Application.put_env(:tesla, :adapter, Tesla.Mock)

defmodule MyAgent do
  use Agent

  def start_link(_arg) do
    Agent.start_link(fn -> Tesla.get!("https://httpbin.org/get") end, name: __MODULE__)
  end
end

Tesla.Mock.mock(fn _ -> Tesla.Mock.json(%{"url" => "https://httpbin.org/get"}) end)
{:ok, _pid} = MyAgent.start_link([])
IO.inspect(:sys.get_state(MyAgent), label: "state")

$ elixir tesla_1.13_mock.exs
state: %Tesla.Env{
  method: nil,
  url: "",
  query: [],
  headers: [{"content-type", "application/json"}],
  body: "{\"url\":\"https://httpbin.org/get\"}",
  status: 200,
  opts: [],
  __module__: nil,
  __client__: nil
}

Version 1.14

# tesla_1.14_mock.exs
Mix.install([
  {:jason, "~> 1.4"},
  {:tesla, "~> 1.14.0"}
])

Application.ensure_all_started(:tesla)
Application.put_env(:tesla, :adapter, Tesla.Mock)

defmodule MyAgent do
  use Agent

  def start_link(_arg) do
    Agent.start_link(fn -> Tesla.get!("https://httpbin.org/get") end, name: __MODULE__)
  end
end

Tesla.Mock.mock(fn _ -> Tesla.Mock.json(%{"url" => "https://httpbin.org/get"}) end)
{:ok, _pid} = MyAgent.start_link([])
IO.inspect(:sys.get_state(MyAgent), label: "state")

$ elixir tesla_1.14_mock.exs
** (EXIT from #PID<0.99.0>) an exception was raised:
    ** (Tesla.Mock.Error) There is no mock set for process #PID<0.9.0>.
Use Tesla.Mock.mock/1 to mock HTTP requests.

See https://github.com/teamon/tesla#testing

        (tesla 1.14.1) lib/tesla/mock.ex:206: Tesla.Mock.call/2
        (tesla 1.14.1) lib/tesla.ex:75: Tesla.execute!/3
        (elixir 1.17.3) lib/agent/server.ex:8: Agent.Server.init/1
        (stdlib 5.2.3.2) gen_server.erl:980: :gen_server.init_it/2
        (stdlib 5.2.3.2) gen_server.erl:935: :gen_server.init_it/6
        (stdlib 5.2.3.2) proc_lib.erl:241: :proc_lib.init_p_do_apply/3

[yordis]

@tomekowal are you able to help me here? Otherwise, I will get to it, I have been using Tesla.Test lately more and more

[tomekowal]

Hey! Sure, I'll check this out.

[tomekowal]

Firstly, thanks @yastanotheruser for awesome report, it made it super easy to test and confirm ❤

In 1.14, we've decided to base the hierarchy on $callers instead of $ancestors. It is a breaking change, but it is intended one. It is similar to how mox library works. We often want to mock things for a process that is not directly spawned by the test process. Especially Tasks should be spawned under Task supervisor, so they are not direct descendants of the test process.

In your example, the test process starts an Agent, so the test process is an ancestor, but not a caller.

I am wondering how to best approach the issue in your case and there are three approaches:
a) change from Agent to Task: this makes sense because Agents are designed to only hold state and not to perform any operations (making HTTP calls is an operation that can fail, so it is not well suited for an Agent), if you start the task with start_supervised that would work correctly
b) if you still want to use Agent, it would be better to fetch the HTTP request in the test process and put only the resulting value in the Agent
c) it is possible to force $callers like this:

defmodule MyAgent do
  use Agent

  # Don't run it directly(!) Use `start_supervised`
  def start_link(_arg) do
    Process.get(:"$callers", [])

    Agent.start_link(
      fn ->
        Process.put(:"$callers", callers)
        IO.inspect(Process.get(:"$callers"), label: "Callers In Agent")
        IO.inspect(Process.get(:"$ancestors"), label: "Ancestors In Agent")
        ...
      end,
      name: __MODULE__
    )
  end
end

I am sorry for the breaking change. I still think it is a good change overall.

Does that solve your problem? Should I add some more explanation in the docs?

For more info about $callers vs $ancestors, see https://hexdocs.pm/elixir/1.18.3/Task.html#module-ancestor-and-caller-tracking
and last paragraph of start_supervised https://hexdocs.pm/ex_unit/main/ExUnit.Callbacks.html#start_supervised/2

[tomekowal]

@yastanotheruser Are the explanation and suggested workarounds satisfying? Do you need any further assistance or can we close the issue?

[yordis]

@tomekowal is there anyway to avoid the regression?

[tomekowal]

Yes. We could potentially use both $callers and $ancestors.

It is semantically somewhat weird because mocks should be set in calling process and not necessarily in the ancestor.

So, the decision is between doing something backwards compatible, but non-standard or standard and backwards incompatible. If you value backwards compatibility more, I can make a PR.

[yordis]

Right, the frustrating situation is that we already have existing behaviour, so we can't have regressions.

Ideally, we are backward compatible and leave a TODO to remove the hack, with proper context about it

[MzudemO]

Hi, thanks for the new version and your work on Tesla! Unfortunately, the changes (1.14.2) are not backwards compatible for us.

Previously only $ancestors were considered, now $callers are considered first. We are experiencing all tests that use both mock and mock_global now failing. $callers contains the local mock, so a matching global mock is never found due to the short circuit https://github.com/tomekowal/tesla/blob/0a23f61cce5f2ddd5b7e268a1fb93a41b6f3c0fa/lib/tesla/mock.ex#L204 and we fail with a Tesla.Mock.Error due to the local mock not matching.

It would be nice to keep the minor version either fully backwards compatible, or release these changes as a major version with breaking changes if you don't want to support the old behaviour.

Separately, there seems to be an issue due to reversing the $ancestors - we see non-PID entries MyApp.Supervisor which break during Process.info - although this might be an issue related to our code or in the worst case Elixir itself.

[tomekowal]

Hi @MzudemO,

Thanks for your feedback. Indeed, I failed to consider using both mock and mock_global at the same time. This is quite unusual to me. Is the expectation that global mocks are always considered before local ones? It is surprising to me, but backwards compatibility is important.

Could you, please, test something for me?

The line that considers $callers first is here:
https://github.com/elixir-tesla/tesla/pull/771/files#diff-2f20f5984a9eb33f1b56789990338dce99195b8e484e41124ad7c5e821d2e38fR240

Could you modify deps/tesla/lib/tesla/mock.ex locally in your project and replace:

    potential_mock_holder_pids = [self() | Enum.reverse(callers) ++ Enum.reverse(ancestors)]

with:

    potential_mock_holder_pids = [self() | Enum.reverse(ancestors) ++ Enum.reverse(callers)]

Then, run MIX_ENV=test mix deps.compile tesla and mix test.

I would like to understand if it still breaks your test suite or if that would solve the issue in a backwards compatible way.

[yordis]

btw, is there any reason that you aren't using the new Tesla.Test module?

@MzudemO do you mind sharing a test case setup to help @tomekowal with it?

[MzudemO]

@yordis Unfortunately the reason is that it's an old project that heavily uses Tesla.Mock in tests and we haven't had the resources to migrate fully to Mox.

@tomekowal

Is the expectation that global mocks are always considered before local ones

Not as such. One case we have is an API where the authentication flow is cached and executed inside Cachex.fetch, which is a separate process. All regular requests are synchronous in the same process as the caller of the API. The result of this is that we needed Tesla.Mock.mock_global for the authentication, and Tesla.Mock.mock was sufficient for all other requests. This implementation is older than my time on the project, I don't want to argue it is a good idea 😄

The global mock was called because $ancestors could never find the local mock - so essentially the global mock was only visible to separate processes.

Including $callers in any form will now find the local mock in the spawned process and the global mock can never be hit, so the order does not matter in this case. I'm not sure if it's possible to be backwards compatible while including $callers, since there would need to be some way to fall back to a global mock if no local mock clauses match - probably breaking in other places.

It seems that this isn't an issue for more people (at this moment?) so if you don't want to support this behaviour that's totally fine. Appreciate you taking a look at it!

[tomekowal]

@MzudemO Thanks for the detailed explanation. I would really like to preserve using $callers because it seems to me the correct thing to do.

However, I have a good news for you. Cachex in version 4.1 respects $callers too. That means, you will be able to use it with local mocks only 🎉.

Given that you've used mock_global, I assume your tests return the same value for the same cache key, so changing mock_global to mock could work out of the box. If you need to return different values for same keys, you'd need to start cachex under start_supervised in test setup.

[yordis]

@yordis Unfortunately the reason is that it's an old project that heavily uses Tesla.Mock in tests and we haven't had the resources to migrate fully to Mox.

@MzudemO if you share some code snippets, I could use it to tune some LLM prompting 🤔 I have been thinking about it lately, upload prompting guides as part of the docs type of thing.

Regardless, is the situation taken care off?