check- vs. assert-

[housel]

When I was first exposed to the xUnit frameworks (SUnit, jUnit, nUnit, etc., see http://xunitpatterns.com/xUnit.html), I noted that

• The xUnit frameworks use a semantics of aborting the entire test method when an assertion fails.
  This is considered best practice for test frameworks nowadays; it eliminates meaningless cascades
  of assertion failures, and encourages a no-broken-windows policy more than a
  note-failure-and-continue policy would.
• Naming the assertion methods assertWhatever is more suggestive of that semantics,
  by comparison with assert statements and assert macros in various programming languages.
  (Not that this is a universal choice; CppUTest uses CHECK_WHATEVER and Google Test
  uses EXPECT_WHATEVER.)
• For testworks, we could adopt the new abort-test-on-failure semantics for macros named assert-,
  and keep the existing semantics on the check- macros in existing checks.

Unfortunately I never communicated that observation to anyone. And so, when the current assert-* macros were added in 2013, I considered it a missed opportunity.

However, after five years I still think it would be a good idea. I suggest that we adopt abort-test-unit-on-failure semantics for the assert-* macros, and keep (but discourage) check-* semantics for existing tests.

[cgay]

Are you okay with the check-* macros taking the same form as the assert-* macros? i.e., description optional and auto-generated?

[housel]

I think that would cause unnecessary code churn. I'd prefer to leave the existing tests as they are, and when touching things gradually move them to assert-* with the new argument order.

[cgay]

In the case where the "check" semantics are desired, changing them to "assert" won't work. In the case where assert-* need to be changed to check-* to get the new semantics I don't want to have to specify description strings where it's not helpful.

Giving them the same signature seems important.

A possible way around this is to create additional macros:
check-true and assert-true (et al) require descriptions.
check-true* and assert-true* (et al) don't require descriptions.
I'm not a big fan of this approach.

If assertion failures had line numbers associated with them the descriptions would be even less useful. I imagine that's hard though.

[edit: fixed formatting]

[housel]

I guess my point is that people shouldn't be desiring check-* semantics, and that the value of check-* is not in providing a different semantic, but a compatible one.

[cgay]

I see, so the plan would be to deprecate the check-* macros. I think that's reasonable.

[cgay]

I agree that fatal semantics should generally be preferred, but I think there are legitimate uses for non-fatal semantics. The main one being looping over a set of cases. Using fatal semantics means having to build/run/fix n times instead of once (or at least < n).

I propose to add new expect-* macros for non-fatal-with-optional-description semantics and start using that when we really want non-fatal semantics. I'll go over the existing assert-*calls and change them to expect-* where appropriate.

[cgay]

A couple of comments for when I eventually work on this...

1. The Go test framework allows subtests, so a common idiom is to, within one test, loop over some data and call t.Run("subtest name", f(data)). Then you effectively have n variants of the test, each of which can have "fatal" errors, but they are only fatal for the subtest. The subtests are reported individually. This is a long way of saying that I'm not sure I agree that people shouldn't want the current "check" semantics. Maybe it would be better to think of it as not being the default. (Maybe this is what with-test-unit was trying to be? I never really understood it.) Consider a parser test where you want to iterate over a list of potential inputs and note a non-fatal failure if one can't be parsed...why should a failure in one cause a failure in all the others?

2. We could provide expect-* macros that have the check-* semantics but the assert-* parameter structure. I'm a little wary of introducing yet another set of macros, but perhaps it's okay as long as we only mention the check-* macros in passing as "obsolete" in the documentation.

3. Essentially unrelated to this bug, but I'm on a roll now and this is in the same area of the code... It should be possible to fix the assert-* macros to accept a format string and format args instead of just a "description", which requires calling format-to-string in all but the most trivial circumstances.

[cgay]

What is described in the initial comment was done:

testworks/tests/testworks-test-suite.dylan

Line 484 in 51b577b

define test test-assertion-failure-terminates ()

I'm implementing the expect-* macros discussed here, and will close this bug when that's done.