fix: mocks for supervised tasks #750
Conversation
|
|
||
| pid_holder |> Process.info() |> Keyword.get(:dictionary) |> Keyword.get(__MODULE__) | ||
| Enum.find_value(potential_mock_holder_pids, nil, fn pid -> | ||
| {:dictionary, process_dictionary} = Process.info(pid, :dictionary) |
There was a problem hiding this comment.
Using Process.info(pid, :dictionary) instead of just Process.info(pid) reduces the amount data copied between processes.
|
|
||
| pid_holder |> Process.info() |> Keyword.get(:dictionary) |> Keyword.get(__MODULE__) | ||
| Enum.find_value(potential_mock_holder_pids, nil, fn pid -> |
There was a problem hiding this comment.
Using find_value instead of find returns the mock directly, so we don't have to call Process.info for a second time after we got the pid.
| pid when is_pid(pid) -> pid | ||
| name when is_atom(name) -> Process.whereis(name) | ||
| end | ||
| potential_mock_holder_pids = [self() | Enum.reverse(Process.get(:"$callers", []))] |
There was a problem hiding this comment.
We search for mocks hierarchically. Even though, it is unlikely that we've set mocks both in the parent of the test process and in the test process, I would be super confused if I defined the mock right before calling a function and it found a mock from somewhere else.
Hence, I start searching from mocks from the current process and then up the callers hierarchy.
In most situations $callers and $ancestors are the same. E.g. spawning a task, starting a GenServer puts the same pid in both.
The notable difference is starting a supervised task because the ancestors of a supervised task is its supervisor, while the caller is the process that asked to spawn it.
I believe that for mocking $callers is the correct one to use.
| @@ -113,6 +113,41 @@ defmodule Tesla.MockTest do | |||
| end | |||
| end | |||
|
|
|||
| describe "supervised task" do | |||
| test "allows mocking in the caller process" do | |||
| # in real apps, task supervisor will be part of the supervision tree | |||
There was a problem hiding this comment.
The problem with testing callers vs ancestors was that if I start supervisor from test process, the test process would still be a grand-child and it would be both caller and ancestor.
This test makes sure that mocking task is caller and not ancestor of the supervised task.
E.g. if you swap $callers back to $ancestors, it would fail.
|
|
||
| !is_nil(is_holder?) | ||
| end) | ||
| |> case do |
There was a problem hiding this comment.
That case statement is a leftover from the previous solution that assumed using non-existent Process.get(pid, value). Now, it is not necessary.
|
It makes sense to me, thanks for the improvements! |
|
btw, are you familiar with https://hexdocs.pm/tesla/Tesla.Test.html and https://hexdocs.pm/tesla/1-testing.html ideally you would migrate to use the module |
|
Thank you @tomekowal 🚀 |
|
Thanks @yordis I know Tesla.Mock is getting deprecated. We are in the process of upgrading from Tesla 1.4 to 1.13, so we have four years of work to migrate :) |
|
I dont mind keeping it around for a long time, so don't sweat over it. Let me know if you need any help with the migration! I wrote a guide for it https://hexdocs.pm/tesla/v1-macro-migration.html |
This PR is a follow up of this comment:
#746 (comment)
Thanks again @carrascoacd for the original fix. Using
Process.infoinspired me to improve it even further :)