Allow matching on ExUnit setup/setup_all blocks

[josevalim]

For example:

setup %{login_as: username} do
  # ...
end

will only be invoked if the context as the given key. We should also support guard clauses:

setup %{login_as: username} when is_binary(username) do
  # ...
end

/cc @chrismccord

[danhper]

I would like to give this one a try if nobody is on it yet!

[lexmag]

Please go ahead.

[ericmj]

This could be confusing since multiple setups does not generate clauses, instead they just create new functions. Notice the difference in behaviour between functions and setups:

def foo(%{login_as: username}), do: ...
def foo(context), do: ...

setup %{login_as: username}, do: ...
setup context, do: ...

For the function only one clause will run if the argument matches %{login_as: ...}, for the setup both blocks will run.

[josevalim]

I thought about it but I thought it was still worth it because today there is a lot of code like this:

setup context do
  if username = context[:login_as] do
    ...
  else
    :ok
  end
end

Maybe we could have something like setup_matching but I am not convinced. Thoughtss?

[josevalim]

Btw, the reason why it may not be a problem is because we know that

setup context do
setup context do

will run both setup blocks.

[lexmag]

Hmm, as for me, today's if way feels good enough. I have nightmares about deeply nested before-callback hell in RSpec where global state mutates. :bowtie:

Thankfully we don't do nesting, thus in my rating if >= setup > setup_matching.

[josevalim]

I hate the if way because we have about 4 lines of boilerplate compared to today's solution. Sometimes it is more boilerplate than actual code.

[michalmuskala]

You could use with to simplify:

setup context do
  with {:ok, %{login_as: username}} <- {:ok, context},
   do: {:ok, login(context)}
end

Maybe we could have something to reduce this {:ok, value} wrapping, like:

setup with %{login_as: username} <- context,
       do: login(context)

[josevalim]

I find the with proposal quite hard to read, to be honest. :(

[whatyouhide]

I'm leaning towards a :-1: on the with proposal as I find it hard to read as well. I'm inclined to agree with @lexmag here, a simple if may be more verbose but it's clear and people know what to expect from it. If we go with the proposed solution, we introduce some kind of complexity (for the reasons @ericmj mentioned as well) that we will have to document (intensively :smile:) for people to be confident with.

[danhper]

We can already write

setup_all do
  {:ok, %{foo: "bar"}}
end

setup do
  IO.puts("this will run anyway")
  :ok
end

setup %{foo: v} do
  IO.puts("foo = #{v} but I die if foo is not here")
  :ok
end

so the behavior of running all the clauses already exists. Therefore, I do not think that it would really increase the complexity or make things harder to understand if we skip the clause instead of just failing with a FunctionClauseError.

[danhper]

I just opened a PR, so maybe we can continue the discussion there?

[josevalim]

My counter-proposal is to add a new set of macros: setup_if, setup_all_if and test_if that will only run if the context matches. There may be better options than setup_if but I prefer that to setup_matching. Thoughts?

[whatyouhide]

The idea could work, but I don't like the name setup_if too much; I find it hard to link it to the idea of "matching". Maybe setup_if_matching? It's long, yeah, but I tend not to care too much if variable/function names are long if they convey the meaning in a good way.

[josevalim]

I thought about setup_case and setup_when but the latter on doesn't look good when using guards:

setup_when %{age: age} when age >= 18 do

[whatyouhide]

setup_if_matching %{age: age} when age >= 18 do
  ...
end

looks good to me though. Or maybe setup_if_matches, may work as well.

[lexmag]

What about setup_only? :bowtie:

[whatyouhide]

@lexmag looks a bit alien to me; what would "only" mean?

[josevalim]

What about setup_only? :bowtie:

I think we should go with setup_matching then. :P

[lexmag]

Also I think it would be valuable to come up with a 3-setup use case and see how it will be transformed with new macros.

[josevalim]

We discussed a bit about this during ElixirConf EU. I will post a new report later.

[josevalim]

Here is the new proposal: https://groups.google.com/forum/#!topic/elixir-lang-core/pDI1zvYCj_k

[josevalim]

In master!