Explicit vs DRY tests

Hmm, this is pretty subjective, but I’d personally go with a variant on Version 2 that swaps Enum.each for a for comprehension. I find it just reads better since it’s so close to spoken language i.e. “for each of these numbers, assert this condition”

  test "1, 2, 4, 7, 8 returns number as a string" do
    for n <- [1, 2, 4, 7, 8], do: assert fizz_buzz(n) == "#{n}"
  end

Since it’s generally good practice to avoid “magic strings” – or in this case numbers – when writing tests, we could generate the multiples. It’s probably a bit much to test something like FizzBuzz, but since it’s an exercise… might as well!

  test "multiple of fifteen aka three and five returns FizzBuzz" do
    for n <- 1..10, do: assert fizz_buzz(15 * n) == "FizzBuzz"
  end

Or maybe even throw in an Enum.random instead.

  test "multiple of fifteen aka three and five returns FizzBuzz" do
    assert fizz_buzz(15 * Enum.random(1..100)) == "FizzBuzz"
  end

To me the test labels also stand out the most. They should at best specify what needs to happen and not just repeat the examples.

At that point property testing (e.g. using stream_data) might become the more appropriate tool to use.

I took all of your feedback into account; except the use of a random because having non predictable tests fears me

This is how I would write it now:

defmodule Kata.FizzBuzz.Test do
  use ExUnit.Case

  import Kata.FizzBuzz

  test "non multiples of 3 and 5 returns the number as a string" do
    for n <- 1..100, rem(n, 3) != 0, rem(n, 5) != 0 do
      assert fizz_buzz(n) == Integer.to_string(n)
    end
  end

  test "multiples of 3 but not of 5 returns Fizz" do
    for n <- 1..100, rem(n, 3) == 0, rem(n, 5) != 0 do
      assert fizz_buzz(n) == "Fizz"
    end
  end

  test "multiples of 5 but not of 3 returns Fizz" do
    for n <- 1..100, rem(n, 3) != 0, rem(n, 5) == 0 do
      assert fizz_buzz(n) == "Buzz"
    end
  end

  test "multiples of 3 and 5 returns Fizz" do
    for n <- 1..100, rem(n, 3 * 5) == 0 do
      assert fizz_buzz(n) == "FizzBuzz"
    end
  end
end

Seems nice to me!

However, this testing strategy seems to be a bit CPU intensive for a trivial test? Maybe that’s why you suggest the use of a random?

I think I am an outlier when it comes to tests, but I would not include any logic to generate assertions in unit tests. I would just choose one type of input for each type of output, with a test for each. If this logic was extra critical or issues keep popping up with unexpected inputs, I guess I would consider adding some sort of property based test to supplement the unit tests, but I would consider carefully before doing it.

I agree here. It’s nice having very explicit, clear failures and not having to guess which one caused a failure. The assert message may provide a clear failure in this case. I think one should default to minimum abstraction in your tests, if there are no other pressing concerns like complexity of function under test.

I usually look at the failure message from my tests to decide whether I like the style of the test.

ExUnit outputs me this in an error case:

  1) test &fizz_buzz/1: multiples of 3 and 5 returns Fizz (Kata.FizzBuzz.Test)
     test/kata/fizz_buzz_test.exs:27
     Assertion with == failed
     code:  assert fizz_buzz(n) == "FizzBuzz"
     left:  "FizzBug"
     right: "FizzBuzz"

It point the problem: we would liked to know the value of n in this case

I’m surprised ExUnit does not already print out the values of the variables used in the failing code.

My approach, as your is IMHO too repeating the implementation:

defmodule Kata.FizzBuzzTest do
  use ExUnit.Case
  use ExUnitProperties

  property "multiples of 3 contains `Fizz`" do
    check all n <- integer() do
      result = fizz_buzz(n * 3)
      assert String.contains?(result, "Fizz")
    end
  end

  property "multiples of 5 contains `Buzz`" do
    check all n <- integer() do
      result = fizz_buzz(n * 5)
      assert String.contains?(result, "Buzz")
    end
  end

  property "returned value is a binary" do
    check all n <- integer() do
      assert is_binary(fizz_buzz(n))
    end
  end

  property "result for 'plain' values is stringified integer" do
    check all n <- integer(), rem(n, 3) * rem(n, 5) != 0 do
      assert fizz_buzz(n) == Integer.to_string(n)
    end
  end
end

There probably could be some additional properties like the above

True! Maybe because it tests all the case from 1 to 100 (or more easily), which is maybe not the purpose of a readable (as a spec) unit test.

I think it’s a bit too much to have a dependency to stream_data for a trivial unit test, I will avoid to use it for now

I will start to apply a combination of the version 1 and 3

After all, I really consider it as a nice feature proposal for ExUnit

For me stream_data is a way to go from day one. It is “must have” just like credo and ex_doc.