proposal: a flag for enabling disabled tests

[Zhanyong Wan (λx.x x)]

Hi,

Google Test allows you to temporarily disable a test function by
prepending DISABLED_ to its name:
http://code.google.com/p/googletest/wiki/GoogleTestAdvancedGuide#Temporarily_Disabling_Tests.
Sometimes you may want to run these disabled tests. Currently you'll
need to remove the DISABLED_ prefix from their names and recompile.
Obviously that's a lot of hassle.

Eric Roman (cc-ed) proposed to add a new command line flag to
re-enable disabled tests. The name I'm thinking is
--gtest_include_disabled_tests.

Please share your comments on the feature. Thanks,

--
Zhanyong

[Chandler Carruth]

On Tue, Dec 30, 2008 at 11:52 AM, Zhanyong Wan (λx.x x) <w...@google.com> wrote:
>
>
> Hi,
>
> Google Test allows you to temporarily disable a test function by
> prepending DISABLED_ to its name:
> http://code.google.com/p/googletest/wiki/GoogleTestAdvancedGuide#Temporarily_Disabling_Tests.
> Sometimes you may want to run these disabled tests. Currently you'll
> need to remove the DISABLED_ prefix from their names and recompile.
> Obviously that's a lot of hassle.
>
> Eric Roman (cc-ed) proposed to add a new command line flag to
> re-enable disabled tests. The name I'm thinking is
> --gtest_include_disabled_tests.

I think this still suffers confusions. Are we including disabled tests
as failures? As a special "disabled" entry in the output? I think you
already gave a really good name in describing this flag:

--gtest_reenable_disabled_tests

Alternatively, this might be a good place to leverage double negatives
to describe the transitions taking place:

--gtest_undisable_tests or --gtest_undisable_disabled_tests

A finaly thought, using a term like "force" may help people
immediately see that this is an override of the selected behavior:

--gtest_force_run_disabled_tests or --gtest_force_enable_disabled_tests

I think "allow" and "include" are the problems ultimately -- they can
be misconstrued to still not actually execute the tests, and override
the disable setting.

-Chandler

[Zhanyong Wan (λx.x x)]

On Sat, Jan 3, 2009 at 3:00 PM, Chandler Carruth <chan...@google.com> wrote:
> On Tue, Dec 30, 2008 at 11:52 AM, Zhanyong Wan (λx.x x) <w...@google.com> wrote:
>>
>>
>> Hi,
>>
>> Google Test allows you to temporarily disable a test function by
>> prepending DISABLED_ to its name:
>> http://code.google.com/p/googletest/wiki/GoogleTestAdvancedGuide#Temporarily_Disabling_Tests.
>> Sometimes you may want to run these disabled tests. Currently you'll
>> need to remove the DISABLED_ prefix from their names and recompile.
>> Obviously that's a lot of hassle.
>>
>> Eric Roman (cc-ed) proposed to add a new command line flag to
>> re-enable disabled tests. The name I'm thinking is
>> --gtest_include_disabled_tests.
>
> I think this still suffers confusions. Are we including disabled tests
> as failures? As a special "disabled" entry in the output? I think you

Good point.

> already gave a really good name in describing this flag:
>
> --gtest_reenable_disabled_tests
>
> Alternatively, this might be a good place to leverage double negatives
> to describe the transitions taking place:
>
> --gtest_undisable_tests or --gtest_undisable_disabled_tests

Google finds ~2,260,000 occurrences of "reenable" vs ~11,500
occurrences of "undisable", so I think the former is far more natural.

> A finaly thought, using a term like "force" may help people
> immediately see that this is an override of the selected behavior:
>
> --gtest_force_run_disabled_tests or --gtest_force_enable_disabled_tests

"force" sounds clear to me too. With "force", I'd go with "run"
instead of "enable" as the former is already clear and shorter.

I have two favorite entries now:

--gtest_reenable_disabled_tests
--gtest_force_run_disabled_tests

I'm slightly inclined to the latter, as the double "e" in "reenable"
may give people trouble spelling the word. What do you think?

> I think "allow" and "include" are the problems ultimately -- they can
> be misconstrued to still not actually execute the tests, and override
> the disable setting.
>
> -Chandler
>
>>
>> Please share your comments on the feature. Thanks,
>>
>> --
>> Zhanyong
>>
>

--
Zhanyong

[Vlad Losev]

A while ago we had a proposition to disable tests at runtime with DisableThisTest method. While this has been postponed, it is a useful feature. I personally needed it on a few occasions and feel we should implement it. How do you see this flag interacting with the runtime disabling of tests?

Cheers,
Vlad.

[Zhanyong Wan (λx.x x)]

On Mon, Jan 5, 2009 at 10:41 AM, Vlad Losev <vl...@google.com> wrote:
> A while ago we had a proposition to disable tests at runtime with
> DisableThisTest method. While this has been postponed, it is a useful
> feature. I personally needed it on a few occasions and feel we should
> implement it. How do you see this flag interacting with the runtime
> disabling of tests?

Thanks for bringing this up, Vlad.

Since DisableThisTest() is issued from individual test functions, it
should be allowed to override the --gtest_force_run_disabled_tests
flag. To avoid confusion, I think we should rename DisableThisTest()
to something like IgnoreThisTestResult(). The difference between
"disable" and "ignore" is that:

- A disabled test is not run at all (of course, unless it has been
re-enabled).

- An ignored test is run, but the result is discarded.

With this terminology, the rules are:

- A test is skipped (i.e. not run at all) iff it's DISABLED_ and not
re-enabled via --gtest_force_run_disabled_tests.

- The result of a test is ignored iff IgnoreThisTestResult() has been
called during the life of the test.

Sounds good?

--
Zhanyong

[Chandler Carruth]

On Mon, Jan 5, 2009 at 11:49 AM, Zhanyong Wan (λx.x x) <w...@google.com> wrote:
> On Mon, Jan 5, 2009 at 10:41 AM, Vlad Losev <vl...@google.com> wrote:
>> A while ago we had a proposition to disable tests at runtime with
>> DisableThisTest method. While this has been postponed, it is a useful
>> feature. I personally needed it on a few occasions and feel we should
>> implement it. How do you see this flag interacting with the runtime
>> disabling of tests?
>
> Thanks for bringing this up, Vlad.
>
> Since DisableThisTest() is issued from individual test functions, it
> should be allowed to override the --gtest_force_run_disabled_tests
> flag. To avoid confusion, I think we should rename DisableThisTest()
> to something like IgnoreThisTestResult(). The difference between
> "disable" and "ignore" is that:
>
> - A disabled test is not run at all (of course, unless it has been
> re-enabled).
>
> - An ignored test is run, but the result is discarded.
>
> With this terminology, the rules are:
>
> - A test is skipped (i.e. not run at all) iff it's DISABLED_ and not
> re-enabled via --gtest_force_run_disabled_tests.
>
> - The result of a test is ignored iff IgnoreThisTestResult() has been
> called during the life of the test.

What if the test crashes immediately after that? Is the crash ignored?
Assuming that the crash isn't ignored, a bunch of other questions
arise: what constructs are safe given an ignore, ASSERT*? assert()?
exceptions?

I'd say that the execution must succeed, and instead that only the
EXPECT*() and ASSERT*() results are ignored, is that sufficient?

(Sorry of these questions were asked when this was previously
discussed, I don't remember the discussion so I probably missed it
entirely.)

-Chandler

[Vlad Losev]

On Mon, Jan 5, 2009 at 11:49 AM, Zhanyong Wan (λx.x x) <w...@google.com> wrote:

On Mon, Jan 5, 2009 at 10:41 AM, Vlad Losev <vl...@google.com> wrote:
> A while ago we had a proposition to disable tests at runtime with
> DisableThisTest method. While this has been postponed, it is a useful
> feature. I personally needed it on a few occasions and feel we should
> implement it. How do you see this flag interacting with the runtime
> disabling of tests?

Thanks for bringing this up, Vlad.

Since DisableThisTest() is issued from individual test functions, it
should be allowed to override the --gtest_force_run_disabled_tests
flag.  To avoid confusion, I think we should rename DisableThisTest()
to something like IgnoreThisTestResult().  The difference between
"disable" and "ignore" is that:

- A disabled test is not run at all (of course, unless it has been
 re-enabled).

- An ignored test is run, but the result is discarded.

With this terminology, the rules are:

- A test is skipped (i.e. not run at all) iff it's DISABLED_ and not
 re-enabled via --gtest_force_run_disabled_tests.

- The result of a test is ignored iff IgnoreThisTestResult() has been
 called during the life of the test.

Sounds good?

Yes, with a slight cringe upon the fact that we will be introducing yet another concept. We will have to report ignored tests separately, too:

You have 5 disabled tests.
You have 8 ignored tests.
 

[Keir Mierle]

On Sat, Jan 3, 2009 at 3:00 PM, Chandler Carruth <chan...@google.com> wrote:

Bikeshedding!

--gtest_run_disabled_tests_anyway

[Zhanyong Wan (λx.x x)]

2009/1/5 Keir Mierle <mie...@gmail.com>:

To help us judge the options, could you explain why you prefer this
over gtest_force_run_disabled_tests? Thanks,

>
>>
>>
>> -Chandler
>>
>> >
>> > Please share your comments on the feature. Thanks,
>> >
>> > --
>> > Zhanyong
>> >
>
>

--
Zhanyong

[Keir Mierle]

Now that I think about it, --gtest_run_disabled_tests seems best.  It is clear, short, direct. The force doesn't really make much difference. Also, I am confident I would guess --gtest_run_disabled_tests but not so much the others.

Keir

[Zhanyong Wan (λx.x x)]

This is one of the earlier candidates I considered. I rejected it as
it could be interpreted as "run the disabled tests only" (i.e.
skipping the tests that are not marked as DISABLED_). I think "force"
makes this interpretation less likely. However, English is not my
native language and my hunch could be wrong.

How about gtest_also_run_disabled_tests?

> Keir
>>
>>
>> >
>> >>
>> >>
>> >> -Chandler
>> >>
>> >> >
>> >> > Please share your comments on the feature. Thanks,
>> >> >
>> >> > --
>> >> > Zhanyong
>> >> >
>> >
>> >
>>
>>
>>
>> --
>> Zhanyong
>
>

--
Zhanyong

[Zhanyong Wan (λx.x x)]

Or a shorter version: gtest_run_disabled_tests_too.

>
>
>> Keir
>>>
>>>
>>> >
>>> >>
>>> >>
>>> >> -Chandler
>>> >>
>>> >> >
>>> >> > Please share your comments on the feature. Thanks,
>>> >> >
>>> >> > --
>>> >> > Zhanyong
>>> >> >
>>> >
>>> >
>>>
>>>
>>>
>>> --
>>> Zhanyong
>>
>>
>
>
>
> --
> Zhanyong
>

--
Zhanyong

[Keir Mierle]

I like this one the most.

[Zhanyong Wan (λx.x x)]

Thanks for the vote, Keir. It would help if you could shed some light
on why you prefer this over gtest_run_disabled_tests_too.

>
>>
>> Or a shorter version: gtest_run_disabled_tests_too.
>>
>> >
>> >
>> >> Keir
>> >>>
>> >>>
>> >>> >
>> >>> >>
>> >>> >>
>> >>> >> -Chandler
>> >>> >>
>> >>> >> >
>> >>> >> > Please share your comments on the feature. Thanks,
>> >>> >> >
>> >>> >> > --
>> >>> >> > Zhanyong
>> >>> >> >
>> >>> >
>> >>> >
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> Zhanyong
>> >>
>> >>
>> >
>> >
>> >
>> > --
>> > Zhanyong
>> >
>>
>>
>>
>> --
>> Zhanyong
>
>

--
Zhanyong

[Zhanyong Wan (λx.x x)]

2009/1/5 Keir Mierle <ke...@google.com>:
>
>
> On Mon, Jan 5, 2009 at 2:15 PM, Zhanyong Wan (λx.x x) <w...@google.com>

> If someone read this to me I would think they meant
> gtest_run_disabled_tests2. This is probably just me, because I do a
> reasonable amount of numeric computing where squared values are common
> (variables ending in 2, i.e. norm2).

Thanks for clarifying. It really helps.

This makes sense. It's probably a good idea to avoid "too" in an
identifier due to its pronunciation. Therefore
gtest_also_run_disabled_tests sounds better to me now. What do others
think?

> Pretty minor though. I do not have a strong opinion either way.
> Keir

>>
>> >
>> >>
>> >> Or a shorter version: gtest_run_disabled_tests_too.
>> >>
>> >> >
>> >> >
>> >> >> Keir
>> >> >>>
>> >> >>>
>> >> >>> >
>> >> >>> >>
>> >> >>> >>
>> >> >>> >> -Chandler
>> >> >>> >>
>> >> >>> >> >
>> >> >>> >> > Please share your comments on the feature. Thanks,
>> >> >>> >> >
>> >> >>> >> > --
>> >> >>> >> > Zhanyong
>> >> >>> >> >
>> >> >>> >
>> >> >>> >
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>> --
>> >> >>> Zhanyong
>> >> >>
>> >> >>
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Zhanyong
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Zhanyong
>> >
>> >
>>
>>
>>
>> --
>> Zhanyong
>
>

--
Zhanyong

[Matt Frantz]

+1 for "also_run..."

There are ample precedents, e.g. --alsologtostderr.

Matt

[Chandler Carruth]

2009/1/5 Matt Frantz <matt...@google.com>:
> +1 for "also_run..."

-1 ;]

My problem is that this implies duplication, which isn't occurring. I
don't think its adding information. Just "run" is fine with me, but I
do prefer "force_run" to clarify that this is a deviation from
standard behavior.

-Chandler

[Zhanyong Wan (λx.x x)]

On Mon, Jan 5, 2009 at 4:47 PM, Chandler Carruth <chan...@google.com> wrote:
> 2009/1/5 Matt Frantz <matt...@google.com>:
>> +1 for "also_run..."
>
> -1 ;]
>
> My problem is that this implies duplication, which isn't occurring. I
> don't think its adding information. Just "run" is fine with me, but I
> do prefer "force_run" to clarify that this is a deviation from
> standard behavior.

I'm fine with "force_run", but would like to understand why "also" implies duplication.  In:

"Please buy some milk on your way home; also, deposit the check John sent you last week."

"also" is used to connect two tasks; yet I can see no duplication here.

The following reads fine to me:

"By default gtest runs active tests only; when --gtest_also_run_disabled_tests is specified, it also runs disabled tests."

Therefore I don't quite understand your concern.  Thanks,

--
Zhanyong

[Chandler Carruth]

On Tue, Jan 6, 2009 at 12:24 AM, Zhanyong Wan (λx.x x) <w...@google.com> wrote:

On Mon, Jan 5, 2009 at 4:47 PM, Chandler Carruth <chan...@google.com> wrote:
> 2009/1/5 Matt Frantz <matt...@google.com>:
>> +1 for "also_run..."
>
> -1 ;]
>
> My problem is that this implies duplication, which isn't occurring. I
> don't think its adding information. Just "run" is fine with me, but I
> do prefer "force_run" to clarify that this is a deviation from
> standard behavior.

I'm fine with "force_run", but would like to understand why "also" implies duplication.  In:

"Please buy some milk on your way home; also, deposit the check John sent you last week."

"also" is used to connect two tasks; yet I can see no duplication here.

The following reads fine to me:

"By default gtest runs active tests only; when --gtest_also_run_disabled_tests is specified, it also runs disabled tests."

Therefore I don't quite understand your concern.  Thanks,

Interesting, you're quite correct. Thinking back to why I got stuck going the other way, I think it was only because of the specific example given, which isn't quite the same: --alsologtostderr, the "to" being the critical difference. That implies duplication of logs, and I mis-read the suggestion along the same lines, a good example of what I was thinking would be "--gtest_also_run_under=valgrind" or something. I retract my objection, and call it a bikeshed. Pick you favorite. =]

-Chandler

[Zhanyong Wan (λx.x x)]

OK, I'll pick gtest_also_run_disabled_tests. Eric, please move
forward with the implementation. Thanks,

--
Zhanyong