Add support for unittest_setup and unittest_teardown functions

[hlovdal]

Having setup (and teardown) functions makes it possible to set up dependencies that are unrelated to the functionality being tested, but due to the way code is written is still required.

Let's say you have a calculator class that in addition to performing calculations also print its operation on a LCD.

class Calculator {
  public:
  int add(int a, int b) {
    int result = a + b;
    char buf[40];
    sprintf(buf, "%d + %d = %d", a, b, result);
    Lcd_p->print(buf);
    return result;
  }
};

When testing the operations, the test should be focused on that, and bringing in additional setup that just happens to be required is poor test code, e.g.

unittest(add)
{
  Lcd_p = new MockLcd();   // <--- Does not belong here.
  Calculator c;
  int result = c.add(11, 22);
  assertEqual(33, result);
}

This belongs in a common setup function for all the tests in the file.

unittest_setup()
{
  Lcd_p = new MockLcd();
}

Does this sound reasonable? I will add some documentation later unless you object.

[ianfixes]

I love this idea.

I'm going to give it some more thought though. Off the top of my head, here are the things I'm curious about:

• Should we add a "task type" field to the Test class, with an enum for the possible options (setup task, teardown task, actual testing task)? This would replace the special "s e t u p" and "t e a r d o w n" names (which seem brittle to me)
• How should the setup/teardown be reported in the test output? What happens if someone puts an assertion in there?
• How do we even handle failures in setup/teardown? Like, do we make the function just return a bool, and handle it as a line item in the test?
• What if someone defines multiple setup / teardown entries? Is that a bug or a feature?

Overall though, great work on this. I really like the way you approached it.

[hlovdal]

Piggybacking on the Test class was just in order to get easy access to the sRoot list, which was an existing, ready to use storage. But it was really just an ugly hack, requiring the use of "magic" string constants.

Working further with it today it became clear that the correct approach is to create separate classes for setup and teardown, and that these can then hold their own static instance references. This makes the changes required in the Test class the absolute minimum it can to be, one optional call to each of the setup/teardown implementations and that's it. I am very pleased with the result.

This also provides the following constraints that will be enforced by the compiler:

• There can be at most one of each setup/teardown macro. Declaring multiple unittest_setup for instance will result in redefinition compilation errors.
• You cannot use asserts inside, attempts to do so will result in 'assertion’ was not declared in this scope compilation errors.

For now the functions returns void and failures are not handled specifically. Returning boolean and checking the result is not an unreasonable behaviour (one possible failure scenario could for instance be reading in test data from a file which does not exist any longer), however I absolutely do not want to force on a mandatory return true; statement for all cases where errors do not have to be handled, so I want the default to be void.

If there is demand for error handling, it is possible to add an additional TestSetupChecked class with a run method that returns boolean, add a corresponding unittest_setup_checked macro and enforce that only one of unittest_setup and unittest_setup_checked is present. That way people could choose if they wanted error handling or not.

But for now only void is supported. I tried making templates out of the classes but got stuck on declaring the static instance members, and I have no plans on spending more effort on that.

[ianfixes]

Thanks for all your time and thought on this. "There can be at most one of each setup/teardown macro" and "You cannot use asserts inside" are exactly what this feature needed.

I absolutely do not want to force on a mandatory return true; statement for all cases where errors do not have to be handled, so I want the default to be void.

Very good point. I'll create a separate issue as a placeholder in case there is demand for some exception to be raised. For now, I'll put into the documentation that the setup function is just a "silent" (no test output) helper.

This is a big step forward for a test system, thank you again for adding it.

[ianfixes]

Whoops, this isn't running before each test...

[ianfixes]

#107