search

chronos-tachyon / googletest-bazel

Fork of googletest that uses bazel.build
https://github.com/google/googletest
BSD 3-Clause "New" or "Revised" License
4 stars 4 forks source link

Implement testing::internal::GetTypeName in terms of a class template #415

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
I needed a way to customize the printing of a type name to accomodate my needs.

Currently, testing::internal::GetTypeName is a plain old template function.
However, you can't partially specialize function templates.

The solution is to implement testing::internal::GetTypeName in terms of a class 
template.

This is necessary to allow people to customize output, by partially 
specializing that implementation class template.

See the patch attached.

Gregory

Original issue reported on code.google.com by gregory....@gmail.com on 19 Jun 2012 at 10:09

Attachments:

GoogleCodeExporter commented 9 years ago 
It's not a good idea for gtest to support extension by having users 
use/specialize code in gtest's "internal" namespace. Code in "internal" can 
change and may break your code if you depend on it.

Can you please clarify what you want GetTypeName() to do in your case and why 
the current typeid(T).name() solution doesn't work?

Original comment by j...@google.com on 19 Jun 2012 at 1:31

GoogleCodeExporter commented 9 years ago 
Then maybe it has to be moved out of "internal" as it comes from a
practical need to customize google test output. Using a fused gtest, I
don't mind applying the patch in my codebase, however I felt like other
people out there might want to customize the output.

The whole situation is the following (with some shortcuts):

I'm testing a C API that provides the following entry points:
  - FooObject* foo_create(int type);
  - void foo_destroy(FooObject* o);
  - bool foo_getProperty(FooObject* o, int id, void* value, size_t
sizeOfValue);
  - const char* foo_type2str(int type); // returns "Bar" for FOO_BAR_TYPE
(coming from the FOO_TYPES enum);

I'm writing type parametrized tests for testing the retrieval of the
FOO_PROP_1 property:

TYPED_TEST_P(FOO_PROP_1_Test, test_foo_getProperty)
{
  FooObject* o = TypeParam::getInstance();

  uint32_t value;
  EXPECT_TRUE(foo_getProperty(o, FOO_PROP_1, &value, sizeof(value));
  EXPECT_EQ(1, value);

  foo_destroy(o); // In my real code I use a RAII helper
}

REGISTER_TYPED_TEST_CASE_P(FOO_PROP_1_Test, test_foo_getProperty);

Now I have:

template<int type>
class TypeWrapper
{
  static FooObject* getInstance()
  {
    return foo_create(type);
  }
};

which I use along with INSTANTIATE_TYPED_TEST_CASE_P:

INSTANTIATE_TYPED_TEST_CASE_P(FooTest, FOO_PROP_1_Test,
::testing::types<TypeWrapper<BAR_TYPE_ID> >); // where BAR_TYPE_ID comes
from an enum

And google test somewhat prints out "1 test from FooTest/FOO_PROP_1_Test/0,
where TypeParam = ::some:namespace::TypeWrapper<1234>"

However, If I can specialize GetTypeNameImpl the following way:

namespace testing {
namespace internal {

  template<int type>
  class GetTypeNameImpl<TypeWrapper<type> >
  {
    public:
    static String  GetTypeName()
    {
      return foo_type2str(type);
    }
  };

} // end of namespace internal
} // end of namespace testing

Now google test prints out "1 test from FooTest/FOO_PROP_1_Test/0, where
TypeParam = Bar" which is prettier.

Gregory

Original comment by gregory....@gmail.com on 19 Jun 2012 at 2:02

GoogleCodeExporter commented 9 years ago 
Thank you. That is an interesting use case, though I'm not sure how widely it 
applies since I believe this is the first request we've received for something 
like this. Would something like the following help you?

struct Bar : TypeWrapper<BAR_TYPE_ID> {};  // <-- Now Bar is a real type.
INSTANTIATE_TYPED_TEST_CASE_P(FooTest, FOO_PROP_1_Test, ::testing::types<Bar>);

Original comment by j...@google.com on 19 Jun 2012 at 2:25

GoogleCodeExporter commented 9 years ago 
That works more or less. It prints "where TypeParam = struct
namespace::from:which::i::am:testing::Bar".

Still, it gives less control over the output.

The change I propose isn't a big one and gives hackers a pinch of freedom.
From a user point of view, specialize nothing and you're as good as before.
And you know you're on your own as soon as you start specializing
implementation details in the testing::internal namespace anyway.

Gregory

Original comment by gregory....@gmail.com on 19 Jun 2012 at 4:15

GoogleCodeExporter commented 9 years ago 
Hi there.

Did you think about my proposal of adding a level of indirection to
that GetTypeName
function?

Original comment by gregory....@gmail.com on 27 Sep 2012 at 8:22

GoogleCodeExporter commented 9 years ago 
Stuff in the internal namespace is not for external use, and I don't want to 
set a precedent of making changes to internal abstractions to support external 
needs. If this is a real need by a number of users, then we should look into an 
official and supported solution; not a solution of specializing an internal 
class. But as far as I can tell, this is not a widely needed/requested feature.

From rereading the comments in this bug, it seems there are two alternative 
solutions:

1. You said you're happy to continue patching your change into your own gtest 
code. That's fine.
2. You can use the supported solution that I proposed in comment 3 above.

Thanks for your interest in googletest.

Original comment by j...@google.com on 28 Sep 2012 at 1:41

GoogleCodeExporter commented 9 years ago 
I know this has been classified as WontFix.

Yet, would you eventually move GetTypeName<T>() out of the internal namespace? 
Considering it didn't change in two years?

Original comment by gregory....@gmail.com on 28 Jul 2014 at 9:46