Test processes using handle_continue

The obvious “solution” is to pause the test process for X amount of milliseconds and give a time for the process to initialize… this will slow down the tests a bit but if you don’t want to add any other code to signal that server initialized already I am not sure how you can do that.

I’m not sure this is a really worthwhile intent to have: Never change production for testing. I’d rather phrase it as creating flexible enough production code so you don’t need to add anything extra for testing the code.

In your example the worker could receive a callback, which defines the actual work.
This way you can test the runtime behaviour of the worker without any of your prod code needing to know about the callback it receives from the test, and it’s even more flexible because you can now use the worker to do all kinds of tasks and not just one specific one.

# Worker
def start_link(callback), do: GenServer.start_link(__MODULE__, callback)
def init(callback), do: {:ok, callback, {:continue, :run_callback}}
[…]

# Business Code
Worker.start_link(&fancy_callback_doing_business_logic/1)

# Test
test = self()
callback = fn -> send(test, :msg) end)
Worker.start_link(callback)
assert_receive :msg

Edit:

This also allows you to test fancy_callback_doing_business_logic without a process around it.

100% agree.

It still doesn’t fix the fact that I am changing my code to be more complex, all in the name of a feature I will likely never use

I do admit it is a valid solution though!

I’m not sure it’s more change to your code than trying to make atomic counters increment. Both wouldn’t be needed without you trying to test something running async to the process starting the computation.

When I add atomic counters, I do that on my test code only.

If I pass a callback, I have to change my production code to receive a parameter on startup (the said callback), I have to change my supervisor creating the workers, and then I have to change the worker itself to be more generic. All this when the purpose of the worker is very specific and when it wouldn’t benefit from the inherent complexity of having a dynamic handle_continue. All these are changes in production code, which don’t bring any real design benefits to the Worker, because this GenServer is riddled with handle_info callbacks specific to it’s purpose.

Can you understand my point of view?

I agree your approach is perfect for a generic worker. But in this specific case, the worker is anything but generic, so it makes me reluctant to pay the price to have a more dynamic handle_continue.

test "saves base_url to dumper and metrics if it cannot connect to it" do
	Process.flag :trap_exit, true

	test = self()

	args = %{
		deps: %{
			dumper: %{
				save_failed_request: fn url ->
					send(test, {:dumper, :save_failed_request, [url]})
					{:ok, :saved}
				end
			},
			metrics: %{
				inc: fn id ->
					send(test, {:metrics, :inc, [id]})
					{:ok, 1}
				end
			}
		}
	}

	{:ok, _pid} = Worker.start_link(args)
	assert_receive {:dumper, :save_failed_request, args}, 600
	# Check args
	assert_receive {:metrics, :inc, args}, 600
	# Check args
	assert_receive {:EXIT, _, :timeout}, 600
	Process.flag :trap_exit, false
end

This is the test of your other topic rewritten to not use atomic counters, but callbacks sending messages. I’d say it’s even better than before because you can also send e.g. the args back to the test for assertion. You did already use the option of “passing in callbacks”, just that you used it to increment counters instead of sending messages.

Aside:

What is that rationale for this structure?

			dumper: %{
				save_failed_request: fn url ->
					send(test, {:dumper, :save_failed_request, [url]})
					{:ok, :saved}
				end
			},
			metrics: %{
				inc: fn id ->
					send(test, {:metrics, :inc, [id]})
					{:ok, 1}
				end
			}

It seems a bit JavaScript-y/Object-y. If these callbacks only exist for test monitoring purposes, it should be possible to unify them into one single function using pattern matching and putting the arguments into a keyword list. (Then it should be possible to wrap it in a macro to remove the function calls from production code (similar to :compile_time_purge_matching ) - though I’ve never done it).

This is how I inject the functions I am stubbing/spying/faking into the SUT. Why maps?
Because I like the syntax of my_map.deps.http.get.('blabla') rather than my_keyword[:deps][:http][:get].('blabal').

I have used keyword lists extensively by now, and I don’t really see the added value when compared to maps (you can also pattern match with them). Even if pattern matching with maps is more limited, I still prefer their less verbose usage any time of the day.

And no, I was not influenced by JavaScript

Quite a brutal idea, but I still go for the first rule of macros: Don’t use macros.
I am not alone in my team, so I am not convinced by the price I would have to pay just because it would be fun. I would have to convince others, teach them and then maintain this position of mine. A price I am not going to pay alone without being convinced this truly has a gigantic benefit.

The testing you are doing is invasive which is typical of Behaviour verification and therefore brutal to start with.

In this case the macros aren’t for fun but to ensure that production code isn’t burdened with testing overhead.

In the absence of macros keyword lists are cheaper to create than maps. That way the production code pays the minimum of overhead while the cost is moved to the testing code.

How about testing handle_continue/2 by calling handle_continue/2? If init/1 does nothing then this would be as simple as:

test "hadle_continue/2 does it job" do
  {:noreply, state} = MyServer.handle_continue(:foo, [])

  assert expected_state == state
end