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

Deduplicate test names in parameterised tests #656

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tobiasleibner
Copy link

Without this change, there are lots of warnings about tests that have already been registered. Also, counting the number of asserts does not seem to work properly, see #581.

Problem:
When using the operator| syntax for parameterized tests, the test name is not adapted for each parameter and thus is not unique, which results in warnings (see #640) and problems with counting asserts (see #581). For steps to reproduce the warnings, see #640.

Solution:
Deduplicate test names by adding information about the current parameter in parentheses after the test name. See tests/ut.cpp for examples.

Issue:
#640, #581

Without this change, there are lots of warnings about tests that have
already been registered. Also, counting the number of asserts does not
seem to work properly, see boost-ext#581.
@tobiasleibner tobiasleibner force-pushed the deduplicate_parameterised_test_names branch 3 times, most recently from 18822db to b4cb115 Compare January 31, 2025 14:21
@tobiasleibner tobiasleibner force-pushed the deduplicate_parameterised_test_names branch from b4cb115 to a40c780 Compare January 31, 2025 15:54
@@ -1025,7 +1025,12 @@ struct eq_ : op {

if constexpr (type_traits::has_static_member_object_value_v<TLhs> and
type_traits::has_static_member_object_value_v<TRhs>) {
#if defined(_MSC_VER) && !defined(__clang__)
// for some reason, accessing the static member via :: does not compile on MSVC
Copy link
Contributor

Choose a reason for hiding this comment

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

instead #if/#else why not simply switch to lhs.value @ rhs.value?

Copy link
Author

@tobiasleibner tobiasleibner Jan 31, 2025

Choose a reason for hiding this comment

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

That was my first attempt, but then the less-than operator does not compile on clang anymore. It interprets the < in lhs.value < rhs.value as opening brace for a template. For some reason, this code seems to be really hard to parse for compilers. We could probably use the #if/else# only for the less-than operator and switch to lhs.value op rhs.value for the other operators.

apply(
[f, name](const auto&... args) {
[f, name, unique_name, &counter](const auto&... args) {
(detail::on<F>(events::test<F, Ts>{.type = "test",
Copy link
Contributor

Choose a reason for hiding this comment

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

The solution here makes sense, wonder whether maybe printing actual value if possible wouldn't be nicer?

"foo"_test = [](auto arg) {} | std::vector{1, 2, 3};

// foo/1
// foo/2
// foo/3

or

// foo(1)
// foo(2)
// foo(3)

which is simiar just with values intead of (1st parameter) etc.. which can be a backup if value is not printable.
any thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

For values which give a small string printing them is nicer than just enumerating the parameters, but these strings could potentially be very large. For example, when testing a linear algebra library, we could have something like

"foo"_test = [](auto arg) {} | create_test_matrices();

and then get

foo ([1 2 3 4
5   6   7   8
9 10 11 12])

already for a small matrix, and for large matrices (100x100, 1000x1000, ...) the output will be huge.

We could first stringify the values, then check their size (and maybe whether they contain newlines) and use the enumeration instead if they are too large. This would give the nicest output, but I am unsure about the performance implications. In the matrices example, converting a large matrix to a string might take quite some time (and a lot of memory for very large matrices).

We could restrict printing to certain types (e,g., integral and floating point types?) and use the enumeration for other types. We could also allow users to make other types printable by providing overloads/specializations of the functions/classes we are using to decide if we want to print or enumerate.

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

Successfully merging this pull request may close these issues.

2 participants