Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typed Tests #357

Closed
ecorm opened this issue Dec 5, 2014 · 24 comments
Closed

Typed Tests #357

ecorm opened this issue Dec 5, 2014 · 24 comments

Comments

@ecorm
Copy link

ecorm commented Dec 5, 2014

When performing the same tests over different types, it would be useful to have a feature similar to Google's Typed Tests: https://code.google.com/p/googletest/wiki/AdvancedGuide#Typed_Tests

@philsquared
Copy link
Collaborator

This comes up from time to time. Thanks for bringing it up again - it helps to get an idea of how many would find it useful.
Although I don't have immediate plans to add type parameterisation it is definitely on the wish-list (although value parameterisation, or even property-based testing, which should be enabled by the in-progress "generators" feature, is probably higher up the list at the moment).

@garethsb-ghost
Copy link

+1
This is something I'm missing when migrating from Boost.Test. See comments + first cut with explicit list of types (rather than type-lists used by Google Test and Boost.Test) in #46

@fsolenberg
Copy link

+1
Like the lightweighted-ness of Catch, but this is a necessity for a test framework IMO - I often make an implementation which is stupid, slow, straightforward, write an exhaustive test for it, then re-implement in an efficient, less-obivous way.

@wichtounet
Copy link

+1
I just found the need for this in one of my library that is tested with Catch, that'd be great to have in Catch directly :)

@garethsb-ghost
Copy link

@fsolenberg @wichtounet Are you able to report whether the CATCH_TEMPLATE_TEST_CASE_n macros I proposed an implementation of on 12 Jan (in #46) work for your use cases?

@philsquared hasn't commented on whether he'd accept that code as a patch yet, but more votes (either way!) might help...

Best regards,

@wichtounet
Copy link

@garethsb This works like a charm :) I add to change CATCH_TEST_CASE to TEST_CASE and CATCH_SECTION to SECTION though, this may come from different version of Catch. Thanks a lot. I will definitely use it in my template math library.

@garethsb-ghost
Copy link

@wichtounet That's great!

By the way, Catch allows you to choose whether it provides short names (TEST_CASE, SECTION, etc.) or ones with the CATCH_ prefix, by defining CATCH_CONFIG_PREFIX_ALL or not.

I guess I should work up my sketch into a proper PR that supports both forms...

@fsolenberg
Copy link

One thing that bugs me with this proposal is that it doesn't allow different ways of constructing the object to be tested - they would all have to be constructed the same way. In that sense, I think it would become a framework type solution, which I would have to work around for some scenarios.

I like how straightforward it is to use SECTIONs in Catch. Another way to approach typed tests would be to do something similar for the setup/teardown as well? Such as:

TEST_CASE("foo") {
  INIT_SECTION("reference") {
    reference_impl foo("bin/coeffs_ref");
  }
  INIT_SECTION("optimized") {
    optimized_impl foo;
  }

  SECTION("bar") {
    REQUIRE(foo.size() == 32);
    REQUIRE(foo.bar({1, 2, 3, 4}) == 76);
    ...

So the actual set of tests to run would be the cartesian product of INIT_SECTIONs and SECTIONs (there's likely a DEINIT_SECTION as well, and sure, the naming can be discussed). I haven't looked at the Catch source so I have no idea whether this is feasible or not.

Then TEMPLATE_TEST_CASE_2, referred to above, would become:

TEST_CASE("test a vector for all supported types") {
  INIT_SECTION("int") {
    std::vector<int> vec;
  }
  INIT_SECTION("float") {
    std::vector<float> vec;
  }
  REQUIRE(vec.size() == 0);
}

Yes, a little bit more verbose, but nothing that can't be folded into another macro. :) e.g.:

TEST_CASE("test a vector for all supported types") {
  INIT_SECTION_TYPES(int, float) {
    std::vector<T> vec;
  }
  REQUIRE(vec.size() == 0);
}

That's one idea.

@wichtounet
Copy link

@fsolenberg Could you check your formatting ? All the angle brackets have gone away.

@fsolenberg
Copy link

@wichtounet Thanks for pointing that out.

@garethsb-ghost
Copy link

This is good input and I think I understand the motivation of @fsolenberg to argue for INIT_SECTION.

However I believe that these template test cases should work like C++ templates; the types under test must meet all the requirements of the concept being tested. If the types have a few different behaviours, those parts can be tested independently by other test cases specific to that type or subset of types.

In the INIT_SECTION example I think this should be achieved by moving construction out of the test case i.e.

template <typename T> T foo_construct();
template <>
reference_impl foo_construct<reference_impl>() { return reference_impl("bin/coeffs_ref"); }
template <>
optimized_impl foo_construct<optimized_impl>() { return optimized_impl(); }

TEMPLATE_TEST_CASE_2("foo", "", Impl, reference_impl, optimized_impl )
{
  Impl foo = foo_construct<Impl>();
  SECTION("bar") {
    REQUIRE(foo.size() == 32);
    REQUIRE(foo.bar({1, 2, 3, 4}) == 76);
    ...

(I'm not actually sure in @fsolenberg pseudo-code how the foo variable declared in the INIT_SECTION scope is intended to be available in the later code?)

Best regards,

@fsolenberg
Copy link

Yes, you're right, looking through the code now I can see my suggested approach would require quite heavy-handed restructuring, and when I think about it, wonder if it is at all possible?

While the template specialization would get the job done, this is exactly the type of work around I was referring to - the framework dictates how the code must be written, it's not me in charge anymore, and readability suffers. Ho-humm...

@garethsb-ghost
Copy link

Your example use case is testing constructed instances of types, and doesn't use the type-name itself at all. To be honest I'd write these kind of tests using existing Catch infrastructure:

template <typename T > void foo_test(T foo);
TEST_CASE("foo")
{
  foo_test(reference_impl("bin/coeffs_ref"));
  foo_test(optimized_impl());
}
template <typename T > void foo_test(T foo) {
  SECTION("bar") {
    REQUIRE(foo.size() == 32);
    REQUIRE(foo.bar({1, 2, 3, 4}) == 76);
    ...

Actually we can wrap that up in a macro to allow you to write:

TEMPLATE_FUNCTION_TEST_CASE_2("foo", foo, reference_impl("bin/coeffs_ref"), optimized_impl())
{
  SECTION("bar") {
    REQUIRE(foo.size() == 32);
    REQUIRE(foo.bar({1, 2, 3, 4}) == 76);
    ...

However, the template test cases that I need are more general - I need to be able to test other properties of the type (does it have a certain nested type, can I construct it from two ints, etc.).

Best regards,

@wichtounet
Copy link

@garethsb Finally, there seems to be something wrong in your implementation. Now that I have many templated test cases, some of them are not always launched :s I have added some prints into some of them and I don't see the prints. I've also add REQUIRE(false) into some tests, but that does not show anything on the console rather than all tests passed... I didn't have these kinds of issue before using template_test_case_2. Looking at the proprocessed code, I cannot find anything wrong, but I'm nowhere near a Catch pro. I have about 500 test cases and almost all of them are templated, could I be hitting some limits ?

@garethsb-ghost
Copy link

I suspect it's a problem with using INTERNAL_CATCH_UNIQUE_NAME in the global scope. As far as I remember it's implemented with __LINE__ so it's likely you're now hitting duplicates between translation units.

We need to get it into a file scope to solve this, e.g. using anonymous namespace.

@wichtounet
Copy link

I also came to the same conclusion :) Simply adding static to the template function fixed the issue :)

@Trass3r
Copy link

Trass3r commented Apr 26, 2015

@philsquared is there any feature ticket or something like that for value parameterization so one can keep track of the work on that?

@philsquared
Copy link
Collaborator

Sorry. This dropped off my radar until #565 referenced it recently.
I've just had a quick browse back through and I tend to agree with @garethsb's March 17th suggestion. That's pretty much how I do it and it seems to work pretty well without the need for any special handling in Catch itself.
There is one other issue with @garethsb's proposal as written: that SECTION within the templated function will only get executed once! What you need is some way of disambiguating it for each invocation. What I usually do is pass an additional string that gets concatenated to the SECTION name (or used as the name entirely).

@wichtounet
Copy link

There is one other issue with @garethsb's proposal as written: that SECTION within the templated function will only get executed once! What you need is some way of disambiguating it for each invocation. What I usually do is pass an additional string that gets concatenated to the SECTION name (or used as the name entirely).

I don't get the issue here. We only need each section to be executed once since there is a section for each template type and each section simply calls the templated test function with the correct type. No ?

@philsquared
Copy link
Collaborator

@wichtounet:

TEST_CASE("foo") calls foo_test twice. foo_test contains a SECTION. Although they are different instantiations of the foo_test template the only way SECTIONs are disambiguated within a test case in Catch is via file and line number - which will be the same for both entries to the SECTION.

It's one of the areas that simplicity of the SECTION abstraction breaks down, unfortunately. You have to know a little of the implementation details for it to make sense.

@wichtounet
Copy link

@philsquared Sorry, I think we are not talking about the same thing :s I was referring to the TEMPLATE_TEST_CASE_n and you were referring to the simpler example for March 17, sorry.

@philsquared
Copy link
Collaborator

Ah, the perils of (me) dropping in on the thread so late - sorry! I did try and qualify it with "March 17th proposal" :-)

@Leandros
Copy link

Leandros commented Mar 8, 2018

👍

@horenmar
Copy link
Member

This is in as of 450dd05

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants