Making it possible to override the name suffix for parameterized test instances

Currently, when using INSTANTIATE_TEST_CASE_P the test instances are all named with a "/x" suffix, where x is the instance number.  

This change adds the possibility to override that behavior so that tests can construct more meaningful name suffixes base on the parameter value.

[junov1, 2011-10-19 18:56:42]

PTAL, someone from googletestframework?

[jgm, 2011-10-20 18:36:47]

Justin,

Thank you for this contribution. However, we discussed a similar issue long ago
and were opposed to it for the following reasons:

- Test names must be unique.  Using the printed value of the test
parameter as part of the test name may break the uniqueness, as two
different values may be printed as the same text (it's totally up to
the print function, which is out of gUnit's control).

- Test names are used in test filters (--gunit_filter), and thus
cannot contain meta characters ('*') or unprintable characters.  This
cannot be guaranteed if we make the printed test parameter part of the
name.

- In general, we're against bloating gUnit with "features" that only
benefit a small number of users, especially when they are just fixing
cosmetic issues.

We need to pass on this contribution for the same reasons. Thanks for your
support!

Greg

[junov1, 2011-10-20 19:11:31]

Fair enough. I guess the consolation is that I can override PrintTo
for displaying GetParam() in a human-readable way.  Right now, the
value of GetParam() is only printed to stdout for tests that fail.
Would it be generally useful to print it for all tests?  It would be
very useful to me.  That change, if should you find it acceptable
would be a simple one-liner consisting of removing an 'if' statement.

On Thu, Oct 20, 2011 at 2:36 PM,  <jgm@google.com> wrote:
> Justin,
>
> Thank you for this contribution. However, we discussed a similar issue
> long ago and were opposed to it for the following reasons:
>
> - Test names must be unique.  Using the printed value of the test
> parameter as part of the test name may break the uniqueness, as two
> different values may be printed as the same text (it's totally up to
> the print function, which is out of gUnit's control).
>
> - Test names are used in test filters (--gunit_filter), and thus
> cannot contain meta characters ('*') or unprintable characters.  This
> cannot be guaranteed if we make the printed test parameter part of the
> name.
>
> - In general, we're against bloating gUnit with "features" that only
> benefit a small number of users, especially when they are just fixing
> cosmetic issues.
>
> We need to pass on this contribution for the same reasons. Thanks for
> your support!
>
> Greg
>
> http://codereview.appspot.com/5305047/
>

[Vlad, 2011-10-21 17:03:47]

Hi Justin,

On Thu, Oct 20, 2011 at 12:11 PM, Justin Novosad <junov@chromium.org> wrote:

> Fair enough. I guess the consolation is that I can override PrintTo
> for displaying GetParam() in a human-readable way.  Right now, the
> value of GetParam() is only printed to stdout for tests that fail.
> Would it be generally useful to print it for all tests?  It would be
> very useful to me.  That change, if should you find it acceptable
> would be a simple one-liner consisting of removing an 'if' statement.
>
> Every change may bring benefits in some situations but be detrimental in
others. The downside of this particular one is that parameter values can be
quite long. In such situations, printing for every test will badly pollute
the test output, making the potential test failure messages harder to find.
So forcing it on everyone is not a good solution. Making new options expands
our test space; that is something we generally loathe to do.

One solution for you is to build your own console output listener. You can
copy the existing listener and change it in any way you want. The downside
is that a lot of infrastructure code used to build it is not part of the
public API. You could copy it, as weel, but that would be a lot of
copy-pasting. I think the most beneficial change would be to publish this
infrastructure. It is not a trivial change though, as one would have to
design a good API around it.


> On Thu, Oct 20, 2011 at 2:36 PM,  <jgm@google.com> wrote:
> > Justin,
> >
> > Thank you for this contribution. However, we discussed a similar issue
> > long ago and were opposed to it for the following reasons:
> >
> > - Test names must be unique.  Using the printed value of the test
> > parameter as part of the test name may break the uniqueness, as two
> > different values may be printed as the same text (it's totally up to
> > the print function, which is out of gUnit's control).
> >
> > - Test names are used in test filters (--gunit_filter), and thus
> > cannot contain meta characters ('*') or unprintable characters.  This
> > cannot be guaranteed if we make the printed test parameter part of the
> > name.
> >
> > - In general, we're against bloating gUnit with "features" that only
> > benefit a small number of users, especially when they are just fixing
> > cosmetic issues.
> >
> > We need to pass on this contribution for the same reasons. Thanks for
> > your support!
> >
> > Greg
> >
> > http://codereview.appspot.com/5305047/
> >
>

Regards,
Vlad

[junov1, 2011-10-24 15:00:19]

Thanks for your advice.

On Fri, Oct 21, 2011 at 1:03 PM, Vlad Losev <vladlosev@gmail.com> wrote:
> Hi Justin,
> On Thu, Oct 20, 2011 at 12:11 PM, Justin Novosad <junov@chromium.org> wrote:
>>
>> Fair enough. I guess the consolation is that I can override PrintTo
>> for displaying GetParam() in a human-readable way.  Right now, the
>> value of GetParam() is only printed to stdout for tests that fail.
>> Would it be generally useful to print it for all tests?  It would be
>> very useful to me.  That change, if should you find it acceptable
>> would be a simple one-liner consisting of removing an 'if' statement.
>>
> Every change may bring benefits in some situations but be detrimental in
> others. The downside of this particular one is that parameter values can be
> quite long. In such situations, printing for every test will badly pollute
> the test output, making the potential test failure messages harder to find.
> So forcing it on everyone is not a good solution. Making new options expands
> our test space; that is something we generally loathe to do.
> One solution for you is to build your own console output listener. You can
> copy the existing listener and change it in any way you want. The downside
> is that a lot of infrastructure code used to build it is not part of the
> public API. You could copy it, as weel, but that would be a lot of
> copy-pasting. I think the most beneficial change would be to publish this
> infrastructure. It is not a trivial change though, as one would have to
> design a good API around it.
>
>>
>> On Thu, Oct 20, 2011 at 2:36 PM,  <jgm@google.com> wrote:
>> > Justin,
>> >
>> > Thank you for this contribution. However, we discussed a similar issue
>> > long ago and were opposed to it for the following reasons:
>> >
>> > - Test names must be unique.  Using the printed value of the test
>> > parameter as part of the test name may break the uniqueness, as two
>> > different values may be printed as the same text (it's totally up to
>> > the print function, which is out of gUnit's control).
>> >
>> > - Test names are used in test filters (--gunit_filter), and thus
>> > cannot contain meta characters ('*') or unprintable characters.  This
>> > cannot be guaranteed if we make the printed test parameter part of the
>> > name.
>> >
>> > - In general, we're against bloating gUnit with "features" that only
>> > benefit a small number of users, especially when they are just fixing
>> > cosmetic issues.
>> >
>> > We need to pass on this contribution for the same reasons. Thanks for
>> > your support!
>> >
>> > Greg
>> >
>> > http://codereview.appspot.com/5305047/
>> >
>
> Regards,
> Vlad