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

Mon Mar 19 09:41:43 PDT 2018

On Fri, Mar 16, 2018 at 2:41 AM James Henderson <
jh7370.2008 at my.bristol.ac.uk> wrote:

> Thanks. The motivating example can be seen in this review:
> https://reviews.llvm.org/D44382.
>
> In that review, I am unit testing .debug_line parsing, specifically, the
> behaviour when the parser is fed a malformed section. Most of the code
> under test goes through some slight variations in the code path, depending
> on a) the DWARF version (interesting cases are 3, 4 and 5), and b) whether
> the DWARF is 32-bit DWARF or 64-bit DWARF. So I have two axes, with
> multiple values on each. The end "symptoms" being tested aren't much
> different in each case, so it makes sense to share the test code as much as
> possible. Without Combine, I have to do the following:
>
> INSTANTIATE_TEST_CASE_P(
>     LineTableTestParams, DebugLineParameterisedFixture,
>     Values(std::make_pair(2, DWARF32), std::make_pair(3, DWARF32),
>            std::make_pair(4, DWARF32), std::make_pair(5, DWARF32),
>            std::make_pair(2, DWARF64), std::make_pair(3, DWARF64),
>            std::make_pair(4, DWARF64), std::make_pair(5, DWARF64)));
>
> I realised whilst typing this out, that I don't need a distinction between
> v2 and v3. However, there's still 6 different combinations, which I have to
> explicitly list. Combine would, by my understanding, allow me to do
> something like "Combine(Values(3, 4, 5), Values(DWARF32, DWARF64))".
>

Is it especially valuable to test all 6? Or would sufficient
coverage/confidence be achieved by testing, say, 3+32, 4+64, and 5+64? (ie:
there's at least one test for each of the values, but not for each
combination) - or is there interesting logic in the implementation that's
different for each combination/pair?

>
> As to why it doesn't work in 1.8, it's because LLVM has explicitly
> overridden the GTEST_HAS_TR1_TUPLE define (see
> utils\unittest\CMakeLists.txt and r316798). The short-story is that it's
> because recent MSVC compilers (sorry, I guess I was too broad in my
> original statement) have started issuing TR1 deprecation warnings, combined
> with C++11 not being detected properly.
>
> On 15 March 2018 at 18:31, Roman Lebedev <lebedev.ri at gmail.com> wrote:
>
>> 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
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180319/8f8f5d8a/attachment.html>