add parametrized tests to cpputest

[the-real-orca]

No description provided.

[arstrube]

Looks interesting. Could you perhaps explain the use case? Which problem do parametrized mix-ins solve?

[arstrube]

About some of the CI errors -
virtual void setParams(void* p) {} remove the parameter name here

[basvodde]

Interesting!

I looked at the code and it does need some work and would like to have a bit of discussion on some parts, but this looks definitively useful.

I do think we ought to be able to make this change smaller. I'm not fully understanding a couple of things. One thing I already mentioned in the mailing list, which is... why does it need a separate TestRegistry?

Oh, the doc part will probably need to go in the doc repository on the website, right? If so, perhaps remove it from here.

Also, a small additional change. Could you put the example tests in one file? And could you try to make the tests a bit more descriptive. I think it'll need more tests still also, but let me first understand it better! :)

Again, thanks for the contribution!

[the-real-orca]

thanks for your feedback. I know that is’s a first proposal and that there may be some changes necessary. I tried to follow the cpputest philosophy and style as much as possible and reuse / mimic existing classes / macros.

First to the motivation / problem to be solved:
Parametrized tests are useful when you have a couple of very similar tests but with different test objects or slightly different settings.
The mixin approach is just a way that accomplish this task and does fit to cpputest.

A (simplified) real world example (actually my main motivation).
I have a generic interface (abstract class) for a storage with a write() and read() function. The actual implementation of the storage can be an in-memory buffer, file, DB, etc. (so no common base class, just the shared interface). However I have a couple of tests working on interface level: e.g. when an object is written to the storage and read from it, the read data should match the written one (independently from the actual implementation). Currently I have individual tests (test code) for each implementation doing the same test. I’ve tried to pack most of the common code in shared helper functions, but nevertheless, this approach is very error prone when it comes to changes or adding new tests.

With parametrized tests, all the interface tests can be grouped in a mixing group (similar to a test group) and added to one or more actual test groups. So when adding a new test to the mixin group, the new test is automatically applied to all test groups importing this mixin group.

Mixin test registry:
One of the design goals was to separate the mixin test definition from the normal test definition, and also avoid including mixin tests as headers to actual test files (this would lead to multiple definition errors). So the mixin tests and mixin test groups have to be stored in somewhere. Since cpputest already has a test registry, this seems to be the right place. However the mixin tests cannot be executed on their own, they have to be parametrized and added to an actual test group. First experiments with adding some kind of invalid test type to the registry were not very promising, so the easy solution is to simply use a second, not executed mixin test registry instance.

Example tests:
The idea of splitting the tests into multiple files was to use them as example showcase. Of course they can be merged to a single file, but then they do not reflect a real use case any more. For now they are really basic and minimalistic just as show case.

My first intention was to check if you like the basic concept and iteratively adapt it so that it does fit to the cpputest implementation.

[arstrube]

@the-real-orca Thank you for taking the trouble to explain. I'll definitely take a look at the tests now that I have a clearer picture.

[arstrube]

@the-real-orca to fix the Travis build:

MIXIN_PARAMS(DemoMixInGroup) // MIXIN_GROUP name
{
    SUT* obj;
    char const* expectedName;
};

@basvodde I think it's thanks to the new Cmake features that this was included in the Travis build at all -- the new files haven't been added to Automake yet.

[arstrube]

Also:

class SUT
{
public:
    virtual const char* className() = 0;
    virtual ~SUT() {} // needs virtual destructor
};

[arstrube]

Also, for Travis CI to pass, the final semicolon needs to go.

[arstrube]

@basvodde why is it that the new tests don't run when I build locally? I added them to my CodeBlocks project in order to test-run them, but they don't get built by automake.

[arstrube]

Successful Travis build see https://travis-ci.org/arstrube/cpputest/builds/65948028

[basvodde]

@the-real-orca related to the TestRegistry. Why not in the MIXIN_APPLY(ImplBTestGroup, DemoMixInGroup, ImplB_test) macro create one of the MixinTest objects and then store it within the test? Why would you need to have a global storage of them?

[the-real-orca]

@arstrube: thanks for taking the time to fix the code
Concerning the missing tests:
I’m developing on Windows (Visual Studio) and have added the new test files to the VC project (MixinTest.h, MixinTest.cpp, MixinApplyTest.cpp but as Bas mentioned, maybe these file should be combined into one cpp) unfortunately I have no experience with Linux autotools or cmake.
On Windows I get a correct result, so I guess it has something to do with missing files in the project.

TEST(ImplATestGroup, ImplA_test: name) - 0 ms
OK (807 tests, 1 ran, 2 checks, 0 ignored, 806 filtered out, 10 ms)

[the-real-orca]

I’ve added documentation to the wiki, maybe this helps to clarify things a bit.
https://github.com/the-real-orca/cpputest/wiki/Parametrized-Tests:-Implementation-Documentation

@basvodde: The Mixin Tests are not in the scope where MIXIN_APPLY(ImplBTestGroup, DemoMixInGroup, ImplB_test) is used. MIXIN_APPLY() will typically be used in TestMyImplementationXXX.cpp, whereas the MIXIN_TESTs are defined in a different Cpp file. So they don’t know from each other until they are linked together and being executed. The only thing MIXIN_APPLY() knows about is the mixin group name and the according MIXIN_PARAMS() definition has to be visible for MIXIN_APPLY().

Without a global storage, all Mixin Test implementation code would need to be included to the TestMyImplementationXXX.cpp file. But then I would still need some kind of local storage at file scope. Another alternative would be some kind of preprocessor loops, but I’m not aware of a useful construction.

[arstrube]

@the-real-orca here is what needs to be done for Automake so the tests will actually be compiled and run: a371c41.
Courious to see whether Travis will be happy with it - so far it's looking good.

[arstrube]

About the discussion regarding one test file vs three - perhaps we could distinguish between

• formal CppUTest tests and
• Example test

The former could be terse and ideally even live in one file (if possible). The latter should be a little more verbose than currently, and have individual tests against the implementations as well as the mix-in test. Also, the name of the mixin should then be a bit more descriptive than "name"...
Which reminds me, a colon and a space in the "test name" might perhaps confuse any scripts that parse CppUTest output?

[the-real-orca]

Thanks for your suggestions. I’ve split the tests into tests testing that the mixin is working -> tests/MixInTest.cpp and a show-case example -> examples/AllTests/MixIn… (I’ve updated the makefile, hope it compiles under Linux)

You are absolutely right, having a colon and space name can confuse subsequent scripts. I’ve changed it to underscores, so there is a clear separation between prefix and test name, but still composes to a valid class name.

[jakubmiernik]

Hi everyone,
What is status of this PR?
It's seems that everyone was happy about it, but it's not merged and still open. Can someone update?

[basvodde]

@jakubmiernik I'm not happy with the code. The size of the PR makes it hard to work with, which is why it keeps getting postpones. Now I'm planning for a release and only adding items related to stability. Thus after the release, I hope to look at it.

Quick scan on the code, it needs a lot of work still.