Skip to content

Test supervisor has :"$callers" populated with the test pid, but the child process does not #13666

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

Closed
InAbsentia opened this issue Jun 14, 2024 · 7 comments

Comments

@InAbsentia
Copy link

InAbsentia commented Jun 14, 2024

Elixir and Erlang/OTP versions

$ elixir --version
Erlang/OTP 27 [erts-15.0] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit]

Elixir 1.17.0 (compiled with Erlang/OTP 27)

Operating system

MacOS 14.5

Current behavior

When starting a GenServer in a test with start_supervised/1, the supervisor has :"$callers" set with the test pid, but the child process does not:

defmodule MyGenServer do
  use GenServer

  def init(_) do
    Process.get()
    |> Keyword.get(:"$ancestors")
    |> List.first()
    |> Process.info(:dictionary)
    |> dbg()

    {:ok, %{}}
  end
end

defmodule MyGenServerTest do
  use ExUnit.Case

  test "fetching data on start" do
    dbg(System.version())
    dbg(self())

    start_supervised!({MyGenServer, []})
  end
end

[test/my_gen_server_test.exs:29: MyGenServerTest."test fetching data on start"/1]
System.version() #=> "1.17.0"

[test/my_gen_server_test.exs:30: MyGenServerTest."test fetching data on start"/1]
self() #=> #PID<0.356.0>

[lib/my_gen_server.ex:44: MyGenServer.init/1]
Process.get() #=> [
  # Child process does *not* have `:"$callers"`
  "$ancestors": [#PID<0.362.0>, #PID<0.356.0>],
  "$initial_call": {MyGenServer, :init, 1}
]
|> Keyword.get(:"$ancestors") #=> [#PID<0.362.0>, #PID<0.356.0>]
|> List.first() #=> #PID<0.362.0>
|> Process.info(:dictionary) #=> {:dictionary,
 [
   # Supervisor does have `:"$callers"` with test pid
   "$callers": [#PID<0.356.0>],
   "$ancestors": [#PID<0.356.0>],
   "$initial_call": {:supervisor, ExUnit.OnExitHandler.Supervisor, 1}
 ]}

Expected behavior

From the discussion in #13252 and the documentation added in #13253, I expected that the child process would have :"$callers" populated with the test process's pid.

@InAbsentia InAbsentia changed the title :"$callers" is populated with the test pid on test supervisor, but not the child process Test supervisor has :"$callers" populated with the test pid, but the child process does not Jun 14, 2024
@josevalim
Copy link
Member

We should probably revert that commit. I think the approach there only works if you spawn tasks, but even then I am not sure, and I don't think it makes sense to spawn tasks directly in the test supervisor anyway.

cc @LostKobrakai @ityonemo

@InAbsentia
Copy link
Author

Thanks for the quick response, @josevalim!

Would you be interested in a different approach that works for all processes with the test supervisor or do you think the additional complexity required wouldn't be worth it?

For context, I have a GenServer that would ideally authenticate with a remote service at startup, saving the auth token for future requests. That way, we can know right away if authentication fails instead of waiting to find out when making a request. I previously enabled testing this process using Bypass, but that requires adding complexity and overhead to both the tests and the production code. I recently removed Bypass from our test suite in favor of Req.Test's stub functionality, but that meant that this particular GenServer can no longer authenticate at startup because it doesn't own the stub or have :"$callers" populated with the test process and the process has already made the request by the time the tests call Req.Test.allow/3 with the process's pid. There are various workarounds for this so it's not the end of the world, but it would be nice if they weren't necessary. 🙂

I've also been told by some of my teammates that they've run into similar ownership issues using Mox, but I don't know the details.

@josevalim
Copy link
Member

I am not sure it can be made to work. The issue is that we require each child process to set the callers themselves. So it ends up being the same as calling allow.

@InAbsentia
Copy link
Author

Ok, I thought that might be the case. Thanks again!

@LostKobrakai
Copy link
Contributor

I think there are two different parts to discuss here:

  1. Having $callers populated in a process.
    This has afaik always required an processes API to collaborate in picking up $caller in the process, which starts the new process, and pushing that value through the startup logic into the newly started process. Integrate start_supervised supervisor with :$caller #13253 had no intention to change anything around that. In elixir itself Task is the only module to do so, but I've certainly build similar ones for my projects.

  2. start_supervised not standing in the way of the above (when present), as it's a separate process between the test process and a started process.
    That part was changed in Integrate start_supervised supervisor with :$caller #13253. By setting $callers on the test supervisor any process, which has an API to deal with $callers would do so now. Before the changes those processes would only work when started directly (e.g. using start_link apis), but not when started with start_supervised. The goal was to remove the difference in behaviour between manual start_link being called and the same process being started with start_supervised.

So given 1. I'm with @josevalim – I don't think there is a way to make this work for any kind of process. But I wouldn't want to see 2. removed as a result of that.

josevalim added a commit that referenced this issue Jun 15, 2024
@josevalim
Copy link
Member

Apologies for the bad commit message, $callers was interpreted by the terminal. :)

@InAbsentia
Copy link
Author

Thanks for the clarification! Now that I understand the situation better, I was able to come up with a pretty clean test helper setup using the new functionality.

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

No branches or pull requests

3 participants