[chromium-dev] Re: Unit tests and anonymous namespaces.
I just wanted to make sure I understood your proposal.
Right now, test classes want to be in the anonymous namespace so that
unit test files do not have to coordinate with each other in the
naming of test classes. With your proposal, the thing which is
exposed outside of the anonymous namespace (MyClassPeer) is based on a
thing already known to be unique (MyClass), so coordinate is not an
issue unless you have multiple unit test files testing the same class.
There are still just as many items exposed outside of the anonymous
namespace, but they are named in a way which prevents collisions.
Is that about right?
Thanks,
scott
On Tue, Mar 3, 2009 at 4:49 PM, William Chan (陈智昌)
willc...@chromium.org wrote:
My old team never really used FRIEND_TEST. We found it ugly that our
production code depended on test code. We typically used friended
Peer classes defined in the unittest file, but not in the anonymous
namespace. These are simple shims that provide access to the private
section. It also saves on having to FRIEND_TEST each individual test
as you add them. It looks like almost every time FRIEND_TEST is used,
it's used for multiple tests, not just a single one. I'm not sure how
much of a problem chrome has with build dependencies leading to
rebuilds, but it was very annoying in google projects to add a
FRIEND_TEST to a widely used .h file, thus forcing everyone to
rebuild, even though you're only adding a new test.
On Tue, Mar 3, 2009 at 4:30 PM, Darin Fisher da...@chromium.org wrote:
On Tue, Mar 3, 2009 at 3:55 PM, Scott Hess sh...@chromium.org wrote:
On the Mac, code like this:
namespace {
class MyTest : public testing::Test {
};
} // namespace
TEST_F(MyTest, ATest) {
}
generates errors like this:
warning: 'MyTest_ATest_Test' has a field
'MyTest_ATest_Test::anonymous' whose type uses the anonymous
namespace
warning: 'MyTest_ATest_Test' has a base 'unnamed::MyTest' whose type
uses the anonymous namespace
Removing the namespace fixes it, which is poor because we seem to want
to move towards more anonymous namespace use. Putting the test case
inside the namespace also fixes it, but is incompatible with
FRIEND_TEST(). This seems to be a gcc 4.2 addition, per:
http://gcc.gnu.org/gcc-4.2/changes.html
Members of the anonymous namespace are now local to a particular
translation unit, along with any other declarations which use them,
though they are still treated as having external linkage for language
semantics.
At this point, I'm sort of at the bleeding edge of my knowledge. For
FRIEND_TEST() cases, it seems like the anonymous namespace needs to
go, but elsewhere it can be changed to enclose the entire file. Does
that seem reasonable for now?
-scott
Assuming that you really need to use TEST_F, then it would be unfortunate to
lose the anonymous namespace. The anonymous namespace has allowed us to
have short names for helper classes used by tests without fear of colliding
with other short names used by other tests. (We have had those kinds of
conflicts in the past.)
We don't use FRIEND_TEST that much. Maybe we can just use #ifdef UNIT_TEST
to expose class APIs to unit tests?
-Darin
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Unit tests and anonymous namespaces.
2009/3/6 Scott Hess sh...@chromium.org:
I just wanted to make sure I understood your proposal.
Right now, test classes want to be in the anonymous namespace so that
unit test files do not have to coordinate with each other in the
naming of test classes. With your proposal, the thing which is
exposed outside of the anonymous namespace (MyClassPeer) is based on a
thing already known to be unique (MyClass), so coordinate is not an
issue unless you have multiple unit test files testing the same class.
There are still just as many items exposed outside of the anonymous
namespace, but they are named in a way which prevents collisions.
Is that about right?
Mostly right. It's fewer things exposed outside of the anonymous
namespace, since previously you needed each TEST/TEST_F class to exist
outside the anonymous namespace in order for you to FRIEND_TEST it.
In this way, you only have one Peer per class you're testing. I don't
know enough about Chrome yet, but if there were some very commonly
used classes, then it might make sense to put the Peer class in a
header file for multiple tests to share. My gut instinct is this use
case doesn't happen enough in Chrome to make it worthwhile, but I have
no clue. It made more sense in the Google code base for various
reasons.
Also, I'm a big fan of testing via the public interface methods as
much as possible. Hopefully we don't need to friend most classes in
order to tell them well.
Thanks,
scott
On Tue, Mar 3, 2009 at 4:49 PM, William Chan (陈智昌)
willc...@chromium.org wrote:
My old team never really used FRIEND_TEST. We found it ugly that our
production code depended on test code. We typically used friended
Peer classes defined in the unittest file, but not in the anonymous
namespace. These are simple shims that provide access to the private
section. It also saves on having to FRIEND_TEST each individual test
as you add them. It looks like almost every time FRIEND_TEST is used,
it's used for multiple tests, not just a single one. I'm not sure how
much of a problem chrome has with build dependencies leading to
rebuilds, but it was very annoying in google projects to add a
FRIEND_TEST to a widely used .h file, thus forcing everyone to
rebuild, even though you're only adding a new test.
On Tue, Mar 3, 2009 at 4:30 PM, Darin Fisher da...@chromium.org wrote:
On Tue, Mar 3, 2009 at 3:55 PM, Scott Hess sh...@chromium.org wrote:
On the Mac, code like this:
namespace {
class MyTest : public testing::Test {
};
} // namespace
TEST_F(MyTest, ATest) {
}
generates errors like this:
warning: 'MyTest_ATest_Test' has a field
'MyTest_ATest_Test::anonymous' whose type uses the anonymous
namespace
warning: 'MyTest_ATest_Test' has a base 'unnamed::MyTest' whose type
uses the anonymous namespace
Removing the namespace fixes it, which is poor because we seem to want
to move towards more anonymous namespace use. Putting the test case
inside the namespace also fixes it, but is incompatible with
FRIEND_TEST(). This seems to be a gcc 4.2 addition, per:
http://gcc.gnu.org/gcc-4.2/changes.html
Members of the anonymous namespace are now local to a particular
translation unit, along with any other declarations which use them,
though they are still treated as having external linkage for language
semantics.
At this point, I'm sort of at the bleeding edge of my knowledge. For
FRIEND_TEST() cases, it seems like the anonymous namespace needs to
go, but elsewhere it can be changed to enclose the entire file. Does
that seem reasonable for now?
-scott
Assuming that you really need to use TEST_F, then it would be unfortunate to
lose the anonymous namespace. The anonymous namespace has allowed us to
have short names for helper classes used by tests without fear of colliding
with other short names used by other tests. (We have had those kinds of
conflicts in the past.)
We don't use FRIEND_TEST that much. Maybe we can just use #ifdef UNIT_TEST
to expose class APIs to unit tests?
-Darin
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Unit tests and anonymous namespaces.
2009/3/6 William Chan (陈智昌) willc...@chromium.org: 2009/3/6 Scott Hess sh...@chromium.org: I just wanted to make sure I understood your proposal. Right now, test classes want to be in the anonymous namespace so that unit test files do not have to coordinate with each other in the naming of test classes. With your proposal, the thing which is exposed outside of the anonymous namespace (MyClassPeer) is based on a thing already known to be unique (MyClass), so coordinate is not an issue unless you have multiple unit test files testing the same class. There are still just as many items exposed outside of the anonymous namespace, but they are named in a way which prevents collisions. Is that about right? Mostly right. It's fewer things exposed outside of the anonymous namespace, since previously you needed each TEST/TEST_F class to exist outside the anonymous namespace in order for you to FRIEND_TEST it. In this way, you only have one Peer per class you're testing. I don't know enough about Chrome yet, but if there were some very commonly used classes, then it might make sense to put the Peer class in a header file for multiple tests to share. My gut instinct is this use case doesn't happen enough in Chrome to make it worthwhile, but I have no clue. It made more sense in the Google code base for various reasons. Also, I'm a big fan of testing via the public interface methods as much as possible. Hopefully we don't need to friend most classes in order to tell them well. The complaint I got about putting the test class outside the anonymous namespace is that there was another completely unrelated test far away which used the same name (and was also outside the anonymous namespace, but could be put inside). Having files implementing unittests around a single class needing to coordinate with each other seems much more reasonable. Thanks, scott --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
[chromium-dev] Re: Unit tests and anonymous namespaces.
On Tue, Mar 3, 2009 at 3:55 PM, Scott Hess sh...@chromium.org wrote:
On the Mac, code like this:
namespace {
class MyTest : public testing::Test {
};
} // namespace
TEST_F(MyTest, ATest) {
}
generates errors like this:
warning: ‘MyTest_ATest_Test’ has a field
‘MyTest_ATest_Test::anonymous’ whose type uses the anonymous
namespace
warning: ‘MyTest_ATest_Test’ has a base ‘unnamed::MyTest’ whose type
uses the anonymous namespace
Removing the namespace fixes it, which is poor because we seem to want
to move towards more anonymous namespace use. Putting the test case
inside the namespace also fixes it, but is incompatible with
FRIEND_TEST(). This seems to be a gcc 4.2 addition, per:
http://gcc.gnu.org/gcc-4.2/changes.html
Members of the anonymous namespace are now local to a particular
translation unit, along with any other declarations which use them,
though they are still treated as having external linkage for language
semantics.
At this point, I'm sort of at the bleeding edge of my knowledge. For
FRIEND_TEST() cases, it seems like the anonymous namespace needs to
go, but elsewhere it can be changed to enclose the entire file. Does
that seem reasonable for now?
In this case the solution is easy. Erase the class MyTest definition
and use TEST instead of TEST_F.
Brett
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Unit tests and anonymous namespaces.
On Tue, Mar 3, 2009 at 3:55 PM, Scott Hess sh...@chromium.org wrote:
On the Mac, code like this:
namespace {
class MyTest : public testing::Test {
};
} // namespace
TEST_F(MyTest, ATest) {
}
generates errors like this:
warning: ‘MyTest_ATest_Test’ has a field
‘MyTest_ATest_Test::anonymous’ whose type uses the anonymous
namespace
warning: ‘MyTest_ATest_Test’ has a base ‘unnamed::MyTest’ whose type
uses the anonymous namespace
Removing the namespace fixes it, which is poor because we seem to want
to move towards more anonymous namespace use. Putting the test case
inside the namespace also fixes it, but is incompatible with
FRIEND_TEST(). This seems to be a gcc 4.2 addition, per:
http://gcc.gnu.org/gcc-4.2/changes.html
Members of the anonymous namespace are now local to a particular
translation unit, along with any other declarations which use them,
though they are still treated as having external linkage for language
semantics.
At this point, I'm sort of at the bleeding edge of my knowledge. For
FRIEND_TEST() cases, it seems like the anonymous namespace needs to
go, but elsewhere it can be changed to enclose the entire file. Does
that seem reasonable for now?
-scott
Assuming that you really need to use TEST_F, then it would be unfortunate to
lose the anonymous namespace. The anonymous namespace has allowed us to
have short names for helper classes used by tests without fear of colliding
with other short names used by other tests. (We have had those kinds of
conflicts in the past.)
We don't use FRIEND_TEST that much. Maybe we can just use #ifdef UNIT_TEST
to expose class APIs to unit tests?
-Darin
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Unit tests and anonymous namespaces.
My old team never really used FRIEND_TEST. We found it ugly that our
production code depended on test code. We typically used friended
Peer classes defined in the unittest file, but not in the anonymous
namespace. These are simple shims that provide access to the private
section. It also saves on having to FRIEND_TEST each individual test
as you add them. It looks like almost every time FRIEND_TEST is used,
it's used for multiple tests, not just a single one. I'm not sure how
much of a problem chrome has with build dependencies leading to
rebuilds, but it was very annoying in google projects to add a
FRIEND_TEST to a widely used .h file, thus forcing everyone to
rebuild, even though you're only adding a new test.
On Tue, Mar 3, 2009 at 4:30 PM, Darin Fisher da...@chromium.org wrote:
On Tue, Mar 3, 2009 at 3:55 PM, Scott Hess sh...@chromium.org wrote:
On the Mac, code like this:
namespace {
class MyTest : public testing::Test {
};
} // namespace
TEST_F(MyTest, ATest) {
}
generates errors like this:
warning: ‘MyTest_ATest_Test’ has a field
‘MyTest_ATest_Test::anonymous’ whose type uses the anonymous
namespace
warning: ‘MyTest_ATest_Test’ has a base ‘unnamed::MyTest’ whose type
uses the anonymous namespace
Removing the namespace fixes it, which is poor because we seem to want
to move towards more anonymous namespace use. Putting the test case
inside the namespace also fixes it, but is incompatible with
FRIEND_TEST(). This seems to be a gcc 4.2 addition, per:
http://gcc.gnu.org/gcc-4.2/changes.html
Members of the anonymous namespace are now local to a particular
translation unit, along with any other declarations which use them,
though they are still treated as having external linkage for language
semantics.
At this point, I'm sort of at the bleeding edge of my knowledge. For
FRIEND_TEST() cases, it seems like the anonymous namespace needs to
go, but elsewhere it can be changed to enclose the entire file. Does
that seem reasonable for now?
-scott
Assuming that you really need to use TEST_F, then it would be unfortunate to
lose the anonymous namespace. The anonymous namespace has allowed us to
have short names for helper classes used by tests without fear of colliding
with other short names used by other tests. (We have had those kinds of
conflicts in the past.)
We don't use FRIEND_TEST that much. Maybe we can just use #ifdef UNIT_TEST
to expose class APIs to unit tests?
-Darin
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Unit tests and anonymous namespaces.
That sounds like an even better solution, thanks!-Darin
On Tue, Mar 3, 2009 at 4:49 PM, William Chan (陈智昌) willc...@chromium.orgwrote:
My old team never really used FRIEND_TEST. We found it ugly that our
production code depended on test code. We typically used friended
Peer classes defined in the unittest file, but not in the anonymous
namespace. These are simple shims that provide access to the private
section. It also saves on having to FRIEND_TEST each individual test
as you add them. It looks like almost every time FRIEND_TEST is used,
it's used for multiple tests, not just a single one. I'm not sure how
much of a problem chrome has with build dependencies leading to
rebuilds, but it was very annoying in google projects to add a
FRIEND_TEST to a widely used .h file, thus forcing everyone to
rebuild, even though you're only adding a new test.
On Tue, Mar 3, 2009 at 4:30 PM, Darin Fisher da...@chromium.org wrote:
On Tue, Mar 3, 2009 at 3:55 PM, Scott Hess sh...@chromium.org wrote:
On the Mac, code like this:
namespace {
class MyTest : public testing::Test {
};
} // namespace
TEST_F(MyTest, ATest) {
}
generates errors like this:
warning: ‘MyTest_ATest_Test’ has a field
‘MyTest_ATest_Test::anonymous’ whose type uses the anonymous
namespace
warning: ‘MyTest_ATest_Test’ has a base ‘unnamed::MyTest’ whose type
uses the anonymous namespace
Removing the namespace fixes it, which is poor because we seem to want
to move towards more anonymous namespace use. Putting the test case
inside the namespace also fixes it, but is incompatible with
FRIEND_TEST(). This seems to be a gcc 4.2 addition, per:
http://gcc.gnu.org/gcc-4.2/changes.html
Members of the anonymous namespace are now local to a particular
translation unit, along with any other declarations which use them,
though they are still treated as having external linkage for language
semantics.
At this point, I'm sort of at the bleeding edge of my knowledge. For
FRIEND_TEST() cases, it seems like the anonymous namespace needs to
go, but elsewhere it can be changed to enclose the entire file. Does
that seem reasonable for now?
-scott
Assuming that you really need to use TEST_F, then it would be unfortunate
to
lose the anonymous namespace. The anonymous namespace has allowed us to
have short names for helper classes used by tests without fear of
colliding
with other short names used by other tests. (We have had those kinds of
conflicts in the past.)
We don't use FRIEND_TEST that much. Maybe we can just use #ifdef
UNIT_TEST
to expose class APIs to unit tests?
-Darin
--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com
View archives, change email options, or unsubscribe:
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---
[chromium-dev] Re: Unit tests and anonymous namespaces.
On Tue, Mar 3, 2009 at 4:49 PM, William Chan (陈智昌) willc...@chromium.org wrote: My old team never really used FRIEND_TEST. We found it ugly that our production code depended on test code. We typically used friended Peer classes defined in the unittest file, but not in the anonymous namespace. These are simple shims that provide access to the private section. It also saves on having to FRIEND_TEST each individual test as you add them. It looks like almost every time FRIEND_TEST is used, it's used for multiple tests, not just a single one. I'm not sure how much of a problem chrome has with build dependencies leading to rebuilds, but it was very annoying in google projects to add a FRIEND_TEST to a widely used .h file, thus forcing everyone to rebuild, even though you're only adding a new test. In some cases we make a class that derives from testing::Test a friend, and write pass-throughs there. I agree it's a good idea if we have many tests that need this for the same tested class. We don't have a whole lot of FRIEND_TESTs in the first place, so it probably isn't a big deal for us. Brett --~--~-~--~~~---~--~~ Chromium Developers mailing list: chromium-dev@googlegroups.com View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~--~~~~--~~--~--~---
