Use user-defined test name in report

[erlanger]

• Shows the user-defined "Name" in reports
• Name is specified with eunit format (tuple "Name" ......)
• Works with fixtures and deftestcase
• This is better than simply repeating the fixture function name
  for every test
• Does not work for plain deftest because of an eunit bug which
  does not send the name to the listener for plain tests

[yurrriq]

Please remove the unnecessary parent if form.

[erlanger]

@yurrriq do you mean the printable_list(...) test? It is needed to differentiante from tuples like
{spawn, Tests} or {timeout, Tests} etc.

[yurrriq]

Standalone comments (i.e. not inline with code) should start with ;; instead of ;. Other than that and my other comments, this looks good to me. Thanks for getting it sorted.

N.B. I don't have merge permissions, but @oubiwann does and maybe @rvirding.

[yurrriq]

io_lib:printable_list/1 already returns a boolean, so there's no need to wrap it in (if ...).

[yurrriq]

You can think of a defun body as having an implicit progn so it's unnecessary.

[yurrriq]

LGTM /cc @oubiwann

[oubiwann]

This looks very cool -- thanks for working on this feature!

I have some thoughts/requests for clarification:

1. this feature should be optional -- if users don't want to use it, they should have access to the previous behaviour (this may be the case? I haven't dug into the code to see, just looking at diff and running the tests locally off your branch)

2. Given point 1, the old tests should not be replaced, but rather added to.

If this change set keeps the old behaviour too, could you address point 2?

Looking forward to merging this!

[erlanger]

It is not an additional feature really, it simply honors the eunit behaviour which was being ignored before whenever you had (tuple "Title" Tests}.... to summarize:

• behavior before: ignore named tuples (but eunit honors them in a fixture)
• behavior w/patch: use name only if you have a named tuple, the rest continues to be the same.
  so the user already has an option not to use the feature (simply don't use named tuples in a fixture).

I can keep the old tests and add new ones for the patch.

What do you think?

[yurrriq]

👍 for keeping the old tests and adding new ones. I think point 1 is already addressed.

[oubiwann]

@erlanger Fantastic -- thanks for sharing more details.

+1 for the test updates -- that should do it (ah, as I typed this, it seems that a new change is being processed by Travis, so you may have just done that ...)

[erlanger]

hehe, I was pushing the code as you were typing :)

[oubiwann]

You may have already noted this and be debugging it, but it seems that the fixtures tests (ltest-fixture-tests) aren't running right now (on my laptop there should be 150+ tests, but right now only 146 are getting run).

You might not have seen this since there is currently some missing error/state checking in ltest ... but running rebar3 eunit will err out, making the issue above more obvious.

Incidentally, I think I've got a ticket open for this somewhere ... I'll look it up.

Update: Well, I couldn't find the old ticket (maybe it never got created?) so I created a new one: #49.

[erlanger]

Just posted a fix, it was a missing comma when I restored the old tests.

[oubiwann]

Sweet, thanks!