Using macros to generate ExUnit test cases - very slow and scaling poorly?

[Dan Swain]

I'm using a macro that looks something like this to generate test cases:

defmodule RequiredFields do
  @required_fields [:foo, :bar, ... etc]
  @valid_value %{foo: 1, bar: 1, ...}

  defmacro generate_test_cases(type) do
    Enum.map(@required_fields, fn(field) ->
      quote do
        test "#{unquote(field)} is required (#{unquote(type)})" do
          without = Map.put(@valid_value, unquote(field), nil)
          expected_error = String.to_atom("invalid_" <> Atom.to_string(unquote(field)))
          expected = {:error, expected_error}
          assert expected == validate(without, unquote(type))
        end
      end
    end)
  end
end

...
require RequiredFields
RequiredFields.generate_test_cases(:first_type)
RequiredFields.generate_test_cases(:second_type)

My list of required fields is around 25 elements long and 4 different variations, and two different "types".  All told I'm generating about 200 test cases.  It's taking over 50 seconds to generate, load, and run the tests.  The validations themselves are all very simple, so I'm 99% sure it's not the actual code execution that's taking a long time.  Another thing that's interesting is that If I remove half of the test cases (i.e., only the `:first_type`) cases, it cuts the time down to around 5 seconds for around 100 cases.

Hopefully I'm just doing something wrong?

[Eric Meadows-Jönsson]

There is no need for generating multiple test cases. The beauty of assert in elixir is that it's macro so we can display nice error messages. For example the following test case:

```

test "foo" do

  x = 1

  y = 2

  assert x == y

end

```

Will produce the following error message that shows both the expected and actual value.

```

  1) test foo (Bar.Foo)

     test/bar/foo_test.exs:12

     Assertion with == failed

     code: x == y

     lhs:  1

     rhs:  2

     stacktrace:

       test/bar/foo_test.exs:15

```

--
You received this message because you are subscribed to the Google Groups "elixir-lang-talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-ta...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-talk/a7f129f2-b011-4428-a6b0-1a776ae8f61d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Eric Meadows-Jönsson

[Dan Swain]

Hi Eric,

The reason I'm generating multiple test cases is to assert on each field independently.  With 25 fields, it gets hard to look at test output (even though assert in ExUnit does a great job of formatting) and see which field failed.  It's also good for isolating what parts of code are having problems.

I'm going for a "one assertion per test" approach.  I realize the merits of that are debatable, but that's tangential to the fact that i went from 5 to 50 seconds by doubling the number of test cases, which is my real concern.

 - Dan

--
You received this message because you are subscribed to a topic in the Google Groups "elixir-lang-talk" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/elixir-lang-talk/DLyopG-rfa4/unsubscribe.
To unsubscribe from this group and all its topics, send an email to elixir-lang-ta...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-talk/CAM_eaphcUWqrH4gg45ashbw-%3DP2RDvakDbZwCmCux6Q4ZzPxGg%40mail.gmail.com.

[José Valim]

Eric's answer is the correct answer. You are moving things from runtime to compile time for no benefit at all. If what assert shows is not enough, you can still give it a message that it will be shown on error reports:

    assert foo != bar, message: "expected foo and bar, this and that"

(or something of sorts)

In fact, somewhat ironically, you are adding a lot of indirection so your test suite can print a readable "name is required field" but the test itself, the thing you will surely look at the most and maintain, becomes unreadable and full of indirection.

Is the price of unreadable tests really worth paying for "one assertion per test"? Is it justified enough to move code from runtime, the area all developers are most comfortable with, to compile time?

The answer to me is certainly no.

In any case, let's answer the question: why is it slow?

Remember that macros manipulate quoted expressions, which are ASTs, and you are injecting many branches in the module tree by defining tests dynamically. Not only that, this tree is quite repetitive because the test structure is the same over and over.

One of the advices with macros is to keep what is inside the quote to the bare minimum exactly because it is repeated over and over and over again. Not only that, your code will be much cleaner if you keep it separate. This should improve things:

        test "#{unquote(field)} is required (#{unquote(type)})" do

          RequiredFields.assert_required(@valid_value, unquote(field))
        end

Not only that, parts of the compiler will have linear performance according to the number of functions/variables. Or even worse. This would explain the drastic slow down. There are known workarounds but you should get better performance with Erlang 18 as many folks have worked to remove many of those cases from the compiler.

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

On Mon, Sep 21, 2015 at 8:20 PM, Dan Swain <dan.t...@gmail.com> wrote:

--

[Dan Swain]

Thanks, José, I appreciate the answer.

For the sake of discussion, how would you propose I handle this situation?  I haven't seen an alternative that makes sense to me.  I have a struct that, due to legacy reasons, has around 25 fields that I need to validate.  20 or so of them have about the same validation logic, and the rest have various logic.  There is a bit of overlap in the validation rules as well - i.e., 18 of them are required, 15 of them (not a subset of the 18 required fields) must be positive, 3 of them can be negative.

The way I approached it was to have lists of fields for which a particular rule applies; i.e., iterate over required_fields and assert that validation fails when that field is nil; iterate over positive_fields and assert that validation fails when the value is 0.  I had a hard time thinking of a way to test this without macros that didn't get very repetitive.  In my experience, repetitive code - even in tests, if not especially in tests - can be just as dangerous as indirection due to metaprogramming (which I usually try to use very judiciously if at all).

I guess one idea would be to set all of the required fields to nil and then assert that the error set contains each error that I expect; though this requires that I return a specific error for each condition in order to be able to test that the test failed where it should have.

```

test "required fields are present" do

  invalid = %{@valid | foo: nil, bar: nil, ..... }  # really do this with enum, but just to keep this short..

  expected = %{invalid | errors: [:invalid_foo, invalid_bar]}

  assert {:error, expected} == validate(invalid)

end
```

vs

```

test "required fields are present" do

  invalid = %{@valid | foo: nil, bar: nil, .... }

  assert {:error, invalid} == validate(invalid)  # how do I know that it failed because `bar` is nil?

end

```

--
You received this message because you are subscribed to a topic in the Google Groups "elixir-lang-talk" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/elixir-lang-talk/DLyopG-rfa4/unsubscribe.
To unsubscribe from this group and all its topics, send an email to elixir-lang-ta...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-talk/CAGnRm4JVtApmd88xunUoqACxEXjZx45X6RJciYB%2Be8Efi3nwzQ%40mail.gmail.com.

[Dan Swain]

Wow.  Just to follow up; I made each macro only a single line as José recommended and it shaved maybe 5 seconds off the total time.  Then I upgraded to Erlang 18 and the tests now run in 4 seconds.

Also I think maybe I misunderstood the recommendations before; One way to get multiple assertions would be to iterate over each validated field _within a single test case_ - maybe that's what Eric was trying to suggest?  I still prefer one assertion per test, but it makes sense as an optimization.

[José Valim]

I would go with something like this:

@required_fields [:foo, :bar, ...]

test "required fields are present" do

  for field <- @required_fields do

    ...

    assert ... = ..., "expected field :#{field} to be a required field but validator yada yada yada"

  end

end

It can't get simpler than this. And technically speaking, it is still just a single assertion. ;)

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-talk/CANbzxuUodD53MyR14Wob%3DxU4NRnSEbTR9fDnkdbi%2B0N9wHs6jg%40mail.gmail.com.

[Alex Shneyderman]

On Monday, September 21, 2015 at 4:09:10 PM UTC-4, José Valim wrote:

I would go with something like this:

@required_fields [:foo, :bar, ...]

test "required fields are present" do

  for field <- @required_fields do

    ...

    assert ... = ..., "expected field :#{field} to be a required field but validator yada yada yada"

  end

end

It can't get simpler than this. And technically speaking, it is still just a single assertion. ;)

It is not the same thing though. On the first failure all consecutive assertions will not run. 

Also, ex_unit does not report the number of assertions (I find them useful), so if you stick 

to one assertion per test you at least know the volume of testing you are doing (assuming 

those assertions are quality assertions this indicator could be of value).

Cheers,

Alex.

[José Valim]

Then you can do:

test "required fields are present" do

  invalid =

    Enum.filter(@required_fields, fn field ->

      ... != ... 

    end
   assert invalid == [], "expected following fields to be required: #{Enum.join(invalid, ",")}"

end

The point still stands. There is no reason to do it at compilation time.

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D