llvm dev - Mar 2018 - [RFC] Updating googletest to non-release tagged version

Hi all,

I'm currently writing some unit tests for some debug line error handling
code I'm working on (see e.g. https://reviews.llvm.org/D44382), and I just
ran into an annoying disabled feature in gtest, specifically the
"Combine"
feature for use in combinatorially generating parameterised tests. A FIXME
comment in ProfileData\CoverageMappingTest.cpp suggests that I'm not the
only one to have tried and discovered that they cannot use this feature.
The problem is that the version of googletest (v 1.8.0, released in Aug
2016) in the LLVM tree requires TR1 tuple support for this feature, which
is not really supported in recent compilers, and has been explicitly
disabled in our googletest CMakeLists.txt, thus disabling "Combine".

I did a bit of looking around, and v 1.8.0 is indeed the last officially
tagged release of googletest. However, there has been a lot of development
on the framework since that point, including a fix to enable use of Combine
with std::tuple-supporting compilers. There have been a number of issue
raised on the googletest issue tracker (see e.g.
https://github.com/google/googletest/issues/1467 or
https://github.com/google/googletest/issues/1079) asking about a 1.9.0
release, and there has been zero response from anybody answering the query
of when/if it will happen. In the meantime, the last release gets older and
crustier...

I'd therefore like to propose something that might be seen as slightly
controversial: update to use ToT googletest (or at least some reasonably
recent version of master), at least until a new release is created.

Thoughts?

James
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180312/45de75de/attachment-0001.html>

+Chandler who might have some thoughts on this.

Could you provide an example here of the motivation for the feature you're
missing? Might help motivate the discussion (and/or we'll end up nitpicking
how it could be done differently without that feature... - which is sort of
where I'm going with this. Combinatorial test case expansion does seem a
bit suspicious to me - I'd hope we could pick a few examples from the
various equivalence classes & that would suffice?)

On Mon, Mar 12, 2018 at 9:01 AM James Henderson via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi all,
>
> I'm currently writing some unit tests for some debug line error
handling
> code I'm working on (see e.g. https://reviews.llvm.org/D44382), and I
> just ran into an annoying disabled feature in gtest, specifically the
> "Combine" feature for use in combinatorially generating
parameterised
> tests. A FIXME comment in ProfileData\CoverageMappingTest.cpp suggests that
> I'm not the only one to have tried and discovered that they cannot use
this
> feature. The problem is that the version of googletest (v 1.8.0, released
> in Aug 2016) in the LLVM tree requires TR1 tuple support for this feature,
> which is not really supported in recent compilers, and has been explicitly
> disabled in our googletest CMakeLists.txt, thus disabling
"Combine".
>
> I did a bit of looking around, and v 1.8.0 is indeed the last officially
> tagged release of googletest. However, there has been a lot of development
> on the framework since that point, including a fix to enable use of Combine
> with std::tuple-supporting compilers. There have been a number of issue
> raised on the googletest issue tracker (see e.g.
> https://github.com/google/googletest/issues/1467 or
> https://github.com/google/googletest/issues/1079) asking about a 1.9.0
> release, and there has been zero response from anybody answering the query
> of when/if it will happen. In the meantime, the last release gets older and
> crustier...
>
> I'd therefore like to propose something that might be seen as slightly
> controversial: update to use ToT googletest (or at least some reasonably
> recent version of master), at least until a new release is created.
>
> Thoughts?
>
> James
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180315/52b5b057/attachment.html>

On Thu, Mar 15, 2018 at 9:09 PM, David Blaikie via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> +Chandler who might have some thoughts on this.
>
> Could you provide an example here of the motivation for the feature
you're
> missing? Might help motivate the discussion (and/or we'll end up
nitpicking
> how it could be done differently without that feature... - which is sort of
> where I'm going with this. Combinatorial test case expansion does seem
a bit
> suspicious to me - I'd hope we could pick a few examples from the
various
> equivalence classes & that would suffice?)
>
> On Mon, Mar 12, 2018 at 9:01 AM James Henderson via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>>
>> Hi all,
>>
>> I'm currently writing some unit tests for some debug line error
handling
>> code I'm working on (see e.g. https://reviews.llvm.org/D44382), and
I just
>> ran into an annoying disabled feature in gtest, specifically the
"Combine"
>> feature for use in combinatorially generating parameterised tests. A
FIXME
>> comment in ProfileData\CoverageMappingTest.cpp suggests that I'm
not the
>> only one to have tried and discovered that they cannot use this
feature. The
>> problem is that the version of googletest (v 1.8.0, released in Aug
2016) in
>> the LLVM tree requires TR1 tuple support for this feature, which is not
>> really supported in recent compilers, and has been explicitly disabled
in
>> our googletest CMakeLists.txt, thus disabling "Combine".
>>
>> I did a bit of looking around, and v 1.8.0 is indeed the last
officially
>> tagged release of googletest. However, there has been a lot of
development
>> on the framework since that point, including a fix to enable use of
Combine
>> with std::tuple-supporting compilers. There have been a number of issue
>> raised on the googletest issue tracker (see e.g.
>> https://github.com/google/googletest/issues/1467 or
>> https://github.com/google/googletest/issues/1079) asking about a 1.9.0
>> release, and there has been zero response from anybody answering the
query
>> of when/if it will happen. In the meantime, the last release gets older
and
>> crustier...
>>
>> I'd therefore like to propose something that might be seen as
slightly
>> controversial: update to use ToT googletest (or at least some
reasonably
>> recent version of master), at least until a new release is created.
>>
>> Thoughts?
This sounds strange.

I've been using googletest-1.8.0 in my project, with fresh compilers
(clang, gcc - https://godbolt.org/g/jg4tFG - tr1/tuple works)
and not once did i have any issues with unavailability of Combine,
even though i do use it.

Debian sid:
$ dpkg -S tr1/tuple
libstdc++-7-dev:amd64: /usr/include/c++/7/tr1/tuple
libstdc++-5-dev:amd64: /usr/include/c++/5/tr1/tuple
libstdc++-6-dev:amd64: /usr/include/c++/6/tr1/tuple

So "which is not really supported in recent compilers" part is at
least too broad.

Roman.
>> James
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

On Thu, Mar 15, 2018 at 11:10 AM David Blaikie via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> +Chandler who might have some thoughts on this.
>
FWIW, I have no concerns about updating to a modern googletest. More modern
the better IMO if someone is willing to do the work to make sure it works
on all our platforms, etc.

However:
> Could you provide an example here of the motivation for the feature
you're
> missing?
>
I definitely think that the whole space of combinatorial testing should be
carefully discussed. Seems like a high risk of long-running tests w/o
providing much incremental value.

> Might help motivate the discussion (and/or we'll end up nitpicking how
it
> could be done differently without that feature... - which is sort of where
> I'm going with this. Combinatorial test case expansion does seem a bit
> suspicious to me - I'd hope we could pick a few examples from the
various
> equivalence classes & that would suffice?)
>
>
> On Mon, Mar 12, 2018 at 9:01 AM James Henderson via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Hi all,
>>
>> I'm currently writing some unit tests for some debug line error
handling
>> code I'm working on (see e.g. https://reviews.llvm.org/D44382), and
I
>> just ran into an annoying disabled feature in gtest, specifically the
>> "Combine" feature for use in combinatorially generating
parameterised
>> tests. A FIXME comment in ProfileData\CoverageMappingTest.cpp suggests
that
>> I'm not the only one to have tried and discovered that they cannot
use this
>> feature. The problem is that the version of googletest (v 1.8.0,
released
>> in Aug 2016) in the LLVM tree requires TR1 tuple support for this
feature,
>> which is not really supported in recent compilers, and has been
explicitly
>> disabled in our googletest CMakeLists.txt, thus disabling
"Combine".
>>
>> I did a bit of looking around, and v 1.8.0 is indeed the last
officially
>> tagged release of googletest. However, there has been a lot of
development
>> on the framework since that point, including a fix to enable use of
Combine
>> with std::tuple-supporting compilers. There have been a number of issue
>> raised on the googletest issue tracker (see e.g.
>> https://github.com/google/googletest/issues/1467 or
>> https://github.com/google/googletest/issues/1079) asking about a 1.9.0
>> release, and there has been zero response from anybody answering the
query
>> of when/if it will happen. In the meantime, the last release gets older
and
>> crustier...
>>
>> I'd therefore like to propose something that might be seen as
slightly
>> controversial: update to use ToT googletest (or at least some
reasonably
>> recent version of master), at least until a new release is created.
>>
>> Thoughts?
>>
>> James
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180319/3633c17e/attachment.html>