Test processes using handle_continue

[Fl4m3Ph03n1x]

@hauleth

Is the SUT your process?

I feel there is some clarification needed here. The Worker process is a GenServer, that does, some work.
As all GenServers, it’s code is divided into 3 parts:

1. The public API that clients can call
2. GenServer calls to the process itself
3. handle_x calls which do the real work

In the tests I am performing right now, I am testing the public API. Say I have the following code:

defmodule Clint.Worker do
  use GenServer

  defmodule State do
    defstruct conn_pid: nil,
      active_streams: [],
      worker_id: nil,
      opts: %{},
      deps: %{}
  end

  ###############
  # Public API  #
  ###############

  @spec start_link(map) :: Supervisor.on_start
  def start_link(args) do
    deps =
      args
      |> Map.get(:deps, %{})
      |> build_deps()

    worker_id = Map.fetch!(args, :worker_id)
    opts = Map.fetch!(args, :opts)

    Logger.debug("Starting worker #{inspect worker_id }")

    init_state = %State{worker_id: worker_id, opts: opts, deps: deps}
    pname = :bananas
    GenServer.start_link(__MODULE__, init_state, name: pname)
  end

  @spec request(atom, integer, charlist, map) :: {:ok, :received}
  def request(group_name, worker_id, url, injected_deps \\ %{}) do
    deps = build_deps(injected_deps)
    GenServer.cast(:bananas, {:fire, url})
    {:ok, :received}
  end

  @spec build_deps(map) :: map
  defp build_deps(injected_deps), do:
    Map.merge(@default_deps, injected_deps, fn _, a, b -> Map.merge(a, b) end)

  #############
  # Callbacks #
  #############

  @impl GenServer
  def init(state) do
    Process.flag(:trap_exit, true)
    {:ok, state, {:continue, :establish_conn}}
  end

  @impl GenServer
  def handle_continue(:establish_conn, state) do
    with  {:ok, conn_pid} <- Logic.establish_connection do
      new_state = %{state | conn_pid: conn_pid}
      {:noreply, new_state}
    else
      {:error, reason} -> {:stop, reason, state}
    end
  end

  @impl GenServer
  def handle_cast({:fire, url}, state) do
    stream_ref = state.deps.http.get.(state.conn_pid, url)

    new_state = %{state | active_streams: [stream_ref | state.active_streams]}
    {:noreply, new_state}
  end
end

Here the public API (what clients will call) are the start_link and request functions. I am testing the Public API of the worker, the face it shows to the world.

This fits nicely into the “Test the interface, not the implementation” rule of testing, but it is insufficient. For example, using this methodology I can’t test any handle_info calls, unless I create a worker and then send him the exact messages that trigger handle_info (which according to some community members, I should do. I am now trying out this strategy).

Is this Unit testing? Is this integration?
I argue that the Worker is my unit, so I can argue this is unit testing. Some of you will say “You are mixing your state tests with GenServer OTP and therefore this is integration testing”. I wouldn’t say you are wrong, but I will definitely say this is a very thin line, at least for me.

---

@chrismcg I have never used trace for testing before, and I fail to see how I could apply it here. I need to invest more time into this idea, as it looks really cool!

I am however not sure why I need to wait for the

:erlang.trace(:new, true, [:call, :return_to])

call.

---

@peerreynders

The last few topics that you have posted suggest to me that you need a new boundary - for example between the worker process and the http client library.

I usually (try to) make my questions and topics as small and isolated as possible, to make the load on the people reading smaller. Sometimes, this comes at the cost of clarity, which I believe is the case here.

My worker does not depend on any HTTP client. It depends on an HTTP contract, which can be implemented by any client I wish. The boundary is well defined, the way I see it

Perhaps, the trouble here is in making it clear it is a boundary. Perhaps you believe I am testing too much the details of my worker which leads you to think I have no boundaries defined. You are one of the most well educated people in testing I have seen and I find it curious how your opinions differ from mine in so many areas. All I can say to defend myself is that I am following a traditional London style TDD, while injecting the dependencies directly without creating Mock modules, because I believe functional injection is the best way of passing dependencies.

@peerreynders

Depending on the necessary sophistication of the test double “test first” may need to take a back seat for a while.

Test first is not always the solution, true. This is actually the refactor of a project that I made and that I consider is a complete disaster. No better time to fix it than now

In fact, one of the many problem is that this project has not tests at all

---

@hauleth

Especially for you I have prepared small library that I want to publish on Hex later:

Oh my God. Thank you so much. I don;t want to sound … ungrateful, but I am not well versed in erlang yet, so I have trouble understanding what this actually accomplishes. I can only hope this comes with great documentation for dummies like me

---

@sorentwo So you let the tests fail repeatedly until you hit a timeout that is slow enough? I understand you provide a maximum number of tries for the test to run, correct (50 times) ?

[hauleth]

@Fl4m3Ph03n1x

I have trouble understanding what this actually accomplishes

It simply provides module that you can use instead of GenServer.call/2 to make “calls” synchronous, ex. if you have:

{:ok, pid} = GenServer.start_link(Clint.Worker, args)

GenServer.cast(pid, {:fire, url})

You can replace it with:

{:ok, pidish} = :gen_local.start_link(Clint.Worker, args)

{:ok, pidish} = :gen_local.cast(pidish, {:fire, url}

To run whole thing synchronously. Unfortunately there is no support for named processes as this would make state handling pretty hard and slightly “automagical”. Of course this approach do not suite all gen_server use cases as for example it will not work with active gen_tcp/gen_udp connections (naturally).

[chrismcg]

@Fl4m3Ph03n1x

@chrismcg I have never used trace for testing before, and I fail to see how I could apply it here. I need to invest more time into this idea, as it looks really cool!

Back at the top you said that

@Fl4m3Ph03n1x

Is there a way to make ExUnit run the assertions without forcing the Worker process to send a message in handle_continue signaling it? (think of it as forcing the Worker to broadcast a message once handle_continue is done running).

The BEAM provides this through it’s built in tracing facilities. The code I prototyped showed how to get a message in your test process when handle_continue is finished.

@Fl4m3Ph03n1x

I am however not sure why I need to wait for the

:erlang.trace(:new, true, [:call, :return_to])

call.

This line says “I want to trace function calls and their returns in all new processes (and ports) created from now, please send me a message for each one”. The trace_pattern call in the next line says “Actually I only want a message if it’s a call to or return from handle_continue in a specific module” (:local is needed to make the return tracing work).

When that is setup the SUT process is started. BEAM will then send messages to the test process for calls and returns that match what’s been asked for. Those messages can be waited for and once the :return_to message has been received you know that handle_continue has finished.

If you just waited on the :call message there would still be a race condition as your Logic.establish_connection could still be running when the test process started running the asserts.

HTH. I haven’t actually used this technique though I can think of times that I might have now!

[chrismcg]

I have played with this a bit more and come up with a version that only gets a message when handle_continue is returned from:

  test "can know when handle_continue finished" do
    :erlang.trace(:new_processes, true, [:call])

    :erlang.trace_pattern(
      # interested in this module, function and arity
      {Tracetest.Server, :handle_continue, 2},
      # this match pattern doesn't care what the arguments are
      # {:message, false} means don't send the :call message
      # {:return_trace} means do send a :return_from message
      [{:_, [], [{:message, false}, {:return_trace}]}],
      # needed for local calls within modules to work
      [:local]
    )

    {:ok, pid} = Tracetest.Server.start_link(:foo)

    # this is triggered when finished by the :return_trace in the match spec above
    assert_receive {:trace, ^pid, :return_from, {Tracetest.Server, :handle_continue, 2},
                    {:noreply, :some_state}}

    assert true == true
  end

[LostKobrakai]

I just want to mention one big caveat with using tracing in that context: It’ll quite severely couple the test to the implementation.

[chrismcg]

It’s knows the name of the module, the :handle_continue function, and it’s arity. In my example it knows the return args as well but you don’t need those if you just care about whether the function returned or not. I personally would not rate that as “severely” coupled in this case because :handle_continue is part of OTP not some internal function someone could rename during a refactoring.

Why do you rate it severe?

[LostKobrakai]

Take an implementation before handle_continue became a thing using Process.send_after. You couldn’t change this to use handle_continue without breaking the test. Or maybe using gen_statem at some point makes more sense, which has different callbacks, you’d also need to change the test.

[chrismcg]

Sure but I’d be fine with that personally in this particular case which I think of as sort of a last resort. It’s a tradeoff between adding something to a public API just for the tests and knowing a small amount about the internal implementation.

I don’t think I’d use this technique a lot, I’d much prefer to structure the code not to need it or wait for something publicly observable like a registry entry. I do remember a couple of times where I’d have been happy to use this instead of changing the code though.

If you did make changes like you said then the fix shouldn’t be too hard to work out. The code to do this could easily be extracted to a “start_and_wait” function if it was used in multiple tests so there would only be one place to change.

If I was reviewing some code and I saw something like this I would definitely raise a flag though, especially if it was tied to application specific implementation details.

[peerreynders]

@Fl4m3Ph03n1x

I find it curious how your opinions differ from mine in so many areas.

Maybe I simply prefer Inside-Out testing in order to control test maintenance costs (giving up some defect localization).

Once one becomes diligent with testing it’s important to find ways to balance the value of testing with the burden it imposes - test obsessed can go too far.

Kent Beck’s (2008) opinion just recently surfaced here again.

There are times where “should I even be testing this”, “should I be testing this particular aspect” or “should I be testing it in this manner” are valid questions.

[swelham]

I have been dealing with this same issue recently of wanting to wait for handle_contiue to complete before I start testing my server.

I found a the simplest solution was to just put in a call to the server since the message won’t be processed until after the continue has completed. I didn’t want to wrote code in a handle_call just for testing so I used the erlang sys module for this since it provides convince debug functions for working with processes. I found calling :sys.get_state(server_pid) often did the trick. This way I don’t have to use some arbitrary sleep duration to try and guess how long it will take.

[peerreynders]

@swelham

I found a the simplest solution was to just put in a call to the server since the message won’t be processed until after the continue has completed.

That’s a good point. I would think nothing of adding a :stop/:shutdown message to the process even if production doesn’t use it. That way the test case could

• start the process
• send the :stop message
• wait for the process to terminate (either via :EXIT or :DOWN)
• then assert the observable actions

[sorentwo]

@Fl4m3Ph03n1x

@sorentwo So you let the tests fail repeatedly until you hit a timeout that is slow enough? I understand you provide a maximum number of tries for the test to run, correct (50 times) ?

That’s right. Usually it passes on the second or third attempt locally, but if the system is noisy or it is running in CI there is some breathing room.

This is essentially how assertions work in web testing frameworks like Capybara/Hound.