Increase the number of collections in `verifyAllCombinations` beyond 9 about approvaltests.cpp HOT 15 CLOSED

Thanks for the quick reply!

If I have some time before October I will give it a go, but with CppCon and everything, that's not sure.

Also, by trying to implement the combination of 10 collections as a user, I didn't manage to do it without changing the code of the generated header itself: since CombinationApprovals is a class and not a namespace, I wasn't sure how to extend it.

from approvaltests.cpp.

Thanks for logging this, Jonathan.

Yes, I'd love to make this use variadic templates... I think you showed me enough previously that I could probably do it, although not until October at the earliest.

I'd be really happy if anyone else was inclined to implement it sooner...

from approvaltests.cpp.

Ah, that's a really good point. If it changed from a class to a namespace, it would be fine, as user code wouldn't change, so it wouldn't be a breaking change...

(We are not opposed to breaking changes - just trying to avoid unnecessary ones)

from approvaltests.cpp.

In case that could help, here is some stand-alone code that performs a cartesian product.
It's in C++14 and I haven't come round to adapt it to C++11.
But perhaps that could be a starting point.

from approvaltests.cpp.

Thanks Jonathan!

from approvaltests.cpp.

I played around a bit with this an came up with this, which is a bit simplified and works with C++11:
http://coliru.stacked-crooked.com/a/f60f3a1ea1a04d8e

Feel free to use!

from approvaltests.cpp.

I created a pull request for this, let me know what you think.

from approvaltests.cpp.

Hi @mika-fischer - thanks again - we've accepted the PR...

we've also removed the old backwards-compatibility functions, on the grounds that it is likely incredibly rare that anyone would have committed code that used a custom reporter with combinations....

Having seen the new tests, we would prefer the reporter to be the first arg, rather than the middle one.

We tried to do that and couldn't get it to compile - could you advise please?

Thanks

Llewellyn and Clare

from approvaltests.cpp.

Re-opening, as a reminder until we decide what to do about the parameter order....

from approvaltests.cpp.

The current version of this was released this evening in v6.0.0

from approvaltests.cpp.

See #47

I think what tripped you up is the missing std::decay in the std::enable_if. It's needed because the reporter is passed by const reference and not by value (and should have been there also in the previous version, in case the functor was passed by reference...)

See https://godbolt.org/z/Iblrl8 for a demonstration.

It's probably a good idea to also use std::decay in other places where std::is_base_of is used.

from approvaltests.cpp.

It's probably a good idea to also use std::decay in other places where std::is_base_of is used.

Interesting - we've now got

• EnableIfNotDerivedFromReporter in ApprovalTests/CombinationApprovals.h
• IsNotDerivedFromReporter in ApprovalTests/core/Reporter.h

template<class T, class R = void>
using EnableIfNotDerivedFromReporter = typename std::enable_if<!std::is_base_of<Reporter, typename std::decay<T>::type>::value, R>::type;

template<typename T>
using IsNotDerivedFromReporter = typename std::enable_if<!std::is_base_of<Reporter, T>::value, int>::type;

I was going to move the first to the second, but didn't understand what the class R = void was all about...

Perhaps if the template types had longer names that conveyed their role?

I don't suppose you might be available to remote-pair on this at some point? It might be quicker than me sending questions via GitHub?

from approvaltests.cpp.

PS Thank you!

from approvaltests.cpp.

Hi Clare, it's a bit difficult for me at the moment to take time out of my day. But this is simple to answer.

You use std::enable_if to constrain a function or class template. And the way you use it is that you put it in a place where normally a type would go. The way it works is that in case the boolean expression in the first argument is false, it causes a template instantiation error (which is not a compile error, but it will cause the template not to be used for the current instantiation).

If the boolean expression is true, then std::enable_if effectively evaporates, meaning it returns the type that it replaced in the first place and everything is as it was before.

So, for instance, if you have a function template:

template <class T>
T func() {
...
}

and you want to constrain the parameter T so that if fulfills some predicate predicate_v, you could write:

template <class T>
std::enable_if_t<predicate_v<T>, T>
func() {
...
}

As you can see, the return type of the function template was replaced by the enable_if. And
now if you keep in mind that in the success case the enable_if should effectively just disappear, then it becomes clear, that enable_if must know which type to return, and that is exactly the purpose of the second template parameter to enable_if.

The IsNotDerivedFromReporter had int hard-coded, so I couldn't use it and it was not in a Detail namespace so I didn't want to change it. But I agree that it would be nicer to have only one of them.

from approvaltests.cpp.

This was really helpful - thank you @mika-fischer

from approvaltests.cpp.