GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — add parametrized tests to cpputest by the-real-orca · Pull Request #666 · cpputest/cpputest

Conversation

@the-real-orca

@the-real-orca the-real-orca commented Jun 6, 2015

Copy link
Copy Markdown

No description provided.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
the-real-orca added 5 commits May 27, 2015 12:15
@the-real-orca
add interface tests and parametrized mix-ins
e868b05
The basic idea is:
 - have so called mix-in tests that can be inserted to other test groups
 - use a second, non-executed test registry (reuse TestRegistry) -> mix-in registry
 - add tests and test groups to this mix-in registry (derived from Utest and UTestShell) -> mix-in tests, mix-in groups
 - have a normal test that injects the appropriate test from the second registry in the scope of the current test group -> apply mix-in group to test group
 - this injection test is self-looped until all tests from the mix-in group are executed
@the-real-orca
add interface tests and parametrized mix-ins
6105b43
The basic idea is:
 - have so called mix-in tests that can be inserted to other test groups
 - use a second, non-executed test registry (reuse TestRegistry) -> mix-in registry
 - add tests and test groups to this mix-in registry (derived from Utest and UTestShell) -> mix-in tests, mix-in groups
 - have a normal test that injects the appropriate test from the second registry in the scope of the current test group -> apply mix-in group to test group
 - this injection test is self-looped until all tests from the mix-in group are executed
@the-real-orca
Merge branch 'master' of https://github.com/the-real-orca/cpputest
e0a4737
Conflicts:
	src/CppUTest/Utest.cpp
@the-real-orca
add mix-in unit tests
48c5994
Conflicts:
	tests/MixInTest.cpp
	tests/MixInTest.h
@the-real-orca
add documentation for MIXINs / parametrized test
398c63a
@arstrube

arstrube commented Jun 7, 2015

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@the-real-orca
fix compiler warnings
1f59974
@arstrube

arstrube commented Jun 7, 2015

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@basvodde

basvodde commented Jun 8, 2015

Copy link
Copy Markdown
Member

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!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

the-real-orca added 2 commits June 8, 2015 17:49
@the-real-orca
fix virtual function
a734251
@the-real-orca
add system architecture
83ff5be
@the-real-orca

the-real-orca commented Jun 8, 2015

Copy link
Copy Markdown
Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@arstrube

arstrube commented Jun 8, 2015

Copy link
Copy Markdown
Contributor

@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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Arnd R. Strube
Fix char constant problem
32898c9
@arstrube

arstrube commented Jun 8, 2015

Copy link
Copy Markdown
Contributor

@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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Arnd R. Strube added 2 commits June 8, 2015 21:38
Arnd R. Strube
fix non-virtual destructor
d2e2a23
Arnd R. Strube
Remove extraneous semicolon
dba04c6
@arstrube

arstrube commented Jun 8, 2015

Copy link
Copy Markdown
Contributor

Also:

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

arstrube
arstrube reviewed Jun 8, 2015
View reviewed changes
Comment thread tests/MixInApplyTest.cpp Outdated

@arstrube arstrube Jun 8, 2015

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@arstrube

arstrube commented Jun 8, 2015

Copy link
Copy Markdown
Contributor

@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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@arstrube

arstrube commented Jun 9, 2015

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@basvodde

basvodde commented Jun 9, 2015

Copy link
Copy Markdown
Member

@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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@the-real-orca

the-real-orca commented Jun 9, 2015

Copy link
Copy Markdown
Author

@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)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@the-real-orca

the-real-orca commented Jun 9, 2015

Copy link
Copy Markdown
Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

the-real-orca and others added 2 commits June 9, 2015 12:56
@the-real-orca
refactor/simplify parameter setting
5735e6b
Arnd R. Strube
Add Mixin-Tests to Makefile.am
a371c41
@arstrube

arstrube commented Jun 9, 2015

Copy link
Copy Markdown
Contributor

@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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@arstrube

arstrube commented Jun 9, 2015

Copy link
Copy Markdown
Contributor

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@the-real-orca
Merge remote-tracking branch 'remotes/arstrube/orca'
ebc41a6
the-real-orca added 2 commits June 15, 2015 08:39
@the-real-orca
change mixin group + test name concatenation to underscore (so that s…
c571e3a
…ubsequent scripts have no problems with parsing the name)
@the-real-orca
separate formal CppUTest tests and example show-case tests for MixIns
2692462
@the-real-orca

the-real-orca commented Jun 15, 2015

Copy link
Copy Markdown
Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jakubmiernik

jakubmiernik commented Apr 23, 2020

Copy link
Copy Markdown
Contributor

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@basvodde

basvodde commented Apr 24, 2020

Copy link
Copy Markdown
Member

@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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@thetic thetic mentioned this pull request Sep 10, 2026
Add parameterized tests thetic/mu.tiny#109
Open
5 of 8 tasks

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@the-real-orca @arstrube @basvodde @jakubmiernik
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.