naming typed tests?

[Matthew Woehlke]

Is it possible to give more meaningful names to typed tests than "/0",
"/1", etc.? (And no, I don't want prefixes, I want to get rid of the
useless index value, e.g. "MyTest/int-Works".)

--
Matthew

[Matthew Woehlke]

(I set reply-to for a reason. Please keep discussion on the list.)

On 2013-02-11 18:42, Joey Oravec wrote:
> It may be difficult if you're relying on the default printer.

Huh? I don't see where "the default printer" has anything to do with my
question. I'm talking about the test names, which are generated by
TypeParameterizedTest::Register passed to MakeAndRegisterTestInfo.

In fact, from my reading of the applicable code, it is not possible to
provide a name using the existing mechanisms, since the parameter suffix
comes from String::Format("%d", index) and cannot be changed once the
test object is constructed.

> The challenge you'd face in convincing people to change the naming is that
> macros are used to auto generate functions, objects, and variables from
> that name so there are constraints (just like on your test name and class).

I don't see that the parameter participates in an identifier name
anywhere. (I can imagine that would be very difficult, since currently I
see nothing preventing a type parameter from being something crazy like
'typename Foo<Bar, 5>'.)

> It would also impose a lot of changes to how you specify the test in a
> commandline arg for filtering.

Again, I don't see the problem... so you might pass "MyTest/int" instead
of "MyTest/0" as the filter argument. The filtering itself doesn't
change. This is only a problem if you are relying on some scripted
external mechanism for invoking the tests... in which case, just don't
use the feature.

>> Is it possible to give more meaningful names to typed tests than "/0",
>> "/1", etc.? (And no, I don't want prefixes, I want to get rid of the
>> useless index value, e.g. "MyTest/int-Works".)

So, I managed to achieve what I want by tinkering with gtest's guts (and
in the process, made typed tests more convenient). Please see
http://github.com/mwoehlke/gtest/tree/typed-tests-with-named-parameters.
Using this, I can do:

TYPED_TEST_CASE_WITH_NAMES(Matrix, float, double);
TYPED_TEST(Matrix, Identity) { /* ... */ }

...and get tests with names like:

Matrix/float.Identity
Matrix/double.Identity

Even better, it should have no effect on users of TYPED_TEST_CASE.

There are caveats, of course... the type names cannot contain ','s (use
typedefs to work around), and TYPED_TEST_CASE_WITH_NAMES must be used in
the global namespace (no anonymous namespaces). The latter is because I
was not able to figure out how to correctly declare the template
specialization otherwise. (Although I didn't spend a huge lot of time;
nesting it in an extra class that can be typedef'd into scope might work.

This may need to be conditionally disabled if support for ancient
compilers is needed, but that should be okay. (Only the new macro itself
should need to be hidden.)

--
Matthew

[Matthew Woehlke]

Ping?

I added this feature to my local gtest (see
http://github.com/mwoehlke/gtest/tree/typed-tests-with-named-parameters). Would
this be acceptable for inclusion upstream?

Matthew

[Zhanyong Wan (λx.x x)]

Thanks, Matthew.

I looked at your proposal and implementation.  I have some concerns on the hidden gotchas when using this new macro:

- it's inconsistent with TYPED_TEST: your macro takes the types as separate macro arguments; TYPED_TEST takes a Types<>.

- (correct me if I'm wrong) if someone uses a type that contains ',', instead of a compiler error, he'll get a wrong behavior silently at run time.

These can be fixed, but I'm more worried about having characters like '<', '>', '(', ')', '*', '&', ' ' in test names, as they have special meanings on the command line and will make --gtest_filter harder to use.

gtest already includes the type names in the text output and XML output:

[----------] 2 tests from CommonTest/0, where TypeParam = char

[ RUN      ] CommonTest/0.ValuesAreCorrect

[       OK ] CommonTest/0.ValuesAreCorrect (0 ms)

[ RUN      ] CommonTest/0.ValuesAreStillCorrect

[       OK ] CommonTest/0.ValuesAreStillCorrect (0 ms)

[----------] 2 tests from CommonTest/0 (0 ms total)

[----------] 2 tests from CommonTest/1, where TypeParam = int

[ RUN      ] CommonTest/1.ValuesAreCorrect

[       OK ] CommonTest/1.ValuesAreCorrect (0 ms)

[ RUN      ] CommonTest/1.ValuesAreStillCorrect

[       OK ] CommonTest/1.ValuesAreStillCorrect (0 ms)

[----------] 2 tests from CommonTest/1 (0 ms total)

Therefore I'm reluctant to incorporate this patch.  Sorry,

--
Matthew

--

--- You received this message because you are subscribed to the Google Groups "Google C++ Testing Framework" group.
To unsubscribe from this group and stop receiving emails from it, send an email to googletestframework+unsub...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--
Zhanyong

[Matthew Woehlke]

On 2013-04-04 19:55, Zhanyong Wan (λx.x x) wrote:
> Thanks, Matthew.
>
> I looked at your proposal and implementation. I have some concerns on the
> hidden gotchas when using this new macro:
>
> - it's inconsistent with TYPED_TEST: your macro takes the types as separate
> macro arguments; TYPED_TEST takes a Types<>.

Is the type name (as a printable string) available to Types<>? If yes,
then this can be done differently, but I didn't see where it was.

> - (correct me if I'm wrong) if someone uses a type that contains ',',
> instead of a compiler error, he'll get a wrong behavior silently at run
> time.

Actually I am surprised this compiles; I would have expected
preprocessor argument splitting to split such a type at the comma,
resulting in invalid types.

This could be addressed the same as the next point.

> These can be fixed, but I'm more worried about having characters like '<',
> '>', '(', ')', '*', '&', ' ' in test names, as they have special meanings
> on the command line and will make --gtest_filter harder to use.

We could simply not support these. This would be easy to enforce by
declaring a dummy variable like 'dummy_#typename', which would
artificially enforce that only type names which are by themselves valid
identifiers are allowed; typedefs can be used as needed to test types
that would otherwise not meet this restriction.

> gtest already includes the type names in the text output and XML output:
>

> [----------] 2 tests from CommonTest/0, *where TypeParam = char*

> [ RUN ] CommonTest/0.ValuesAreCorrect
> [ OK ] CommonTest/0.ValuesAreCorrect (0 ms)
> [ RUN ] CommonTest/0.ValuesAreStillCorrect
> [ OK ] CommonTest/0.ValuesAreStillCorrect (0 ms)
> [----------] 2 tests from CommonTest/0 (0 ms total)

This, unfortunately, is useless for my purposes. The whole point of this
exercise was for the *test names* to be reasonable. In particular, I
require the type name in --gtest_list_tests so that test discovery knows
something more useful than '0' when enumerating tests. (I'm wrapping
gtest with CTest.)

If there were an option to list test names like:

Range/int.
Construct
Grow
Range/float.
Construct
Grow

...instead of:

Range/0.
Construct
Grow
Range/1.
Construct
Grow

...then this could solve my problem.

> Therefore I'm reluctant to incorporate this patch. Sorry,

If you have a better suggestion for addressing the above problem, I'm
open. I (also) got no reply when I originally asked if there was already
a way to do this...

--
Matthew

[Zhanyong Wan (λx.x x)]

On Fri, Apr 5, 2013 at 8:12 AM, Matthew Woehlke <matthew...@kitware.com> wrote:

On 2013-04-04 19:55, Zhanyong Wan (λx.x x) wrote:

Thanks, Matthew.

I looked at your proposal and implementation.  I have some concerns on the
hidden gotchas when using this new macro:

- it's inconsistent with TYPED_TEST: your macro takes the types as separate
macro arguments; TYPED_TEST takes a Types<>.

Is the type name (as a printable string) available to Types<>? If yes, then this can be done differently, but I didn't see where it was.

gtest has code to use RTTI to find the string name for an arbitrary type.  That's how we are printing the type parameters today.

 

- (correct me if I'm wrong) if someone uses a type that contains ',',
instead of a compiler error, he'll get a wrong behavior silently at run
time.

Actually I am surprised this compiles; I would have expected preprocessor argument splitting to split such a type at the comma, resulting in invalid types.

This could be addressed the same as the next point.

These can be fixed, but I'm more worried about having characters like '<',
'>', '(', ')', '*', '&', ' ' in test names, as they have special meanings
on the command line and will make --gtest_filter harder to use.

We could simply not support these. This would be easy to enforce by declaring a dummy variable like 'dummy_#typename', which would artificially enforce that only type names which are by themselves valid identifiers are allowed; typedefs can be used as needed to test types that would otherwise not meet this restriction.

It's true this can be enforced, but that makes a bad user experience.  We want to reduce the gotchas in the framework, not add to them.

gtest already includes the type names in the text output and XML output:

[----------] 2 tests from CommonTest/0, *where TypeParam = char*

[ RUN      ] CommonTest/0.ValuesAreCorrect
[       OK ] CommonTest/0.ValuesAreCorrect (0 ms)
[ RUN      ] CommonTest/0.ValuesAreStillCorrect
[       OK ] CommonTest/0.ValuesAreStillCorrect (0 ms)
[----------] 2 tests from CommonTest/0 (0 ms total)

This, unfortunately, is useless for my purposes. The whole point of this exercise was for the *test names* to be reasonable. In particular, I require the type name in --gtest_list_tests so that test discovery knows something more useful than '0' when enumerating tests. (I'm wrapping gtest with CTest.) 

If there were an option to list test names like:

Range/int.
  Construct
  Grow
Range/float.
  Construct
  Grow

...instead of:

Range/0.
  Construct
  Grow
Range/1.
  Construct
  Grow

...then this could solve my problem.

Got it.  Why didn't you say what you really want earlier? :)

I think it's a good idea to extend the output of --gtest_list_tests to include the values of type parameters and value parameters of the tests, e.g.

Range/0. for type int

  Construct

  Grow

Range/1. for type float

  Construct

  Grow

Therefore I'm reluctant to incorporate this patch.  Sorry,

If you have a better suggestion for addressing the above problem, I'm open. I (also) got no reply when I originally asked if there was already a way to do this...

--
Matthew

--
Zhanyong

[Matthew Woehlke]

On 2013-04-05 17:39, Zhanyong Wan (λx.x x) wrote:
>> If there were an option to list test names like:
>>
>> Range/int.
>> Construct
>> Grow
>> Range/float.
>> Construct
>> Grow
>>
>> ...instead of:
>>
>> Range/0.
>> Construct
>> Grow
>> Range/1.
>> Construct
>> Grow
>>
>> ...then this could solve my problem.
>

> Got it. Why didn't you say what you *really* want earlier? :)

See original message? ;-) (Well, I am guessing you did and are poking
fun at the divergence from the same...)

> I think it's a good idea to extend the output of --gtest_list_tests to
> include the values of type parameters and value parameters of the tests,
> e.g.
>
> Range/0. for type int
> Construct
> Grow
> Range/1. for type float
> Construct
> Grow

Is it safe to do that unconditionally? I would be worried it isn't (e.g.
it would probably break my existing discovery process), which makes me
inclined to add a new option.

Maybe --gtest_list_tests_with_type?

Do you think that is something you would do soon, or is it worth my
taking a look when/if I get a chance?

--
Matthew

[Zhanyong Wan (λx.x x)]

No, it isn't.  It may break scripts that assume the current output format of --gtest_list_tests.  However, sometimes a breaking change is the right thing to do -- otherwise the system will keep growing into a complex mess.

 

I would be worried it isn't (e.g. it would probably break my existing discovery process), which makes me inclined to add a new option.

Maybe --gtest_list_tests_with_type?

Every flag we add increases the complexity of the framework.  Not just for the maintainers, but for the users as well.  I'd rather extend the behavior of the existing --gtest_list_tests.

Do you think that is something you would do soon, or is it worth my taking a look when/if I get a chance?

I need to fix all scripts used within Google that may break as the result of this change, so it's best done by me than you.  I'll try to work on it soon, but it really hard to tell as I have many things on my plate right now.

I'll start a separate thread to propose this change.  Thanks,

--
Zhanyong