Make ipc_tests file structure a little saner and add an ipc_perftests target.

Make ipc_tests file structure a little saner and add an ipc_perftests target.

This means that the (one, semi-manual) IPC perf test that we have will build
without manual hackery (and do so separately from the ipc_tests target).


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=176341

[jam, 2013-01-09 21:56:52]

https://codereview.chromium.org/11819041/diff/1/ipc/ipc_channel_unittest.h
File ipc/ipc_channel_unittest.h (right):

https://codereview.chromium.org/11819041/diff/1/ipc/ipc_channel_unittest.h#ne...
ipc/ipc_channel_unittest.h:10: class IPCChannelTest : public IPCTestBase {
what's the point of this file? i.e. why can't whoever is deriving from this just
derive from IPCTestBase (other than IPCChannelTest cc file of course)

[viettrungluu, 2013-01-09 22:15:50]

https://codereview.chromium.org/11819041/diff/1/ipc/ipc_channel_unittest.h
File ipc/ipc_channel_unittest.h (right):

https://codereview.chromium.org/11819041/diff/1/ipc/ipc_channel_unittest.h#ne...
ipc/ipc_channel_unittest.h:10: class IPCChannelTest : public IPCTestBase {
On 2013/01/09 21:56:53, John Abd-El-Malek wrote:
> what's the point of this file? i.e. why can't whoever is deriving from this
just
> derive from IPCTestBase (other than IPCChannelTest cc file of course)

ipc_send_fds_test.cc adds (POSIX-only) tests to IPCChannelTest.

Possibly, these tests should go in ipc_channel_posix_unittest.cc instead. But
that file defines an IPCChannelPosixTest that doesn't derive from IPCTestBase
(formerly known as IPCChannelTest). Agh.

Or maybe we should just put the tests in ipc_send_fds_test.cc into their own
(differently-named) test.

All things being equal, I'd prefer not to rename tests (because it messes up
test histories, etc.), but maybe that's the price of sanity.

[viettrungluu, 2013-01-09 23:53:36]

On 2013/01/09 22:15:50, viettrungluu wrote:
> https://codereview.chromium.org/11819041/diff/1/ipc/ipc_channel_unittest.h
> File ipc/ipc_channel_unittest.h (right):
> 
>
https://codereview.chromium.org/11819041/diff/1/ipc/ipc_channel_unittest.h#ne...
> ipc/ipc_channel_unittest.h:10: class IPCChannelTest : public IPCTestBase {
> On 2013/01/09 21:56:53, John Abd-El-Malek wrote:
> > what's the point of this file? i.e. why can't whoever is deriving from this
> just
> > derive from IPCTestBase (other than IPCChannelTest cc file of course)
> 
> ipc_send_fds_test.cc adds (POSIX-only) tests to IPCChannelTest.
> 
> Possibly, these tests should go in ipc_channel_posix_unittest.cc instead. But
> that file defines an IPCChannelPosixTest that doesn't derive from IPCTestBase
> (formerly known as IPCChannelTest). Agh.
> 
> Or maybe we should just put the tests in ipc_send_fds_test.cc into their own
> (differently-named) test.

(Do you want me to do this? Or do you have another suggestion?)

> 
> All things being equal, I'd prefer not to rename tests (because it messes up
> test histories, etc.), but maybe that's the price of sanity.

[jam, 2013-01-10 17:14:24]

On 2013/01/09 23:53:36, viettrungluu wrote:
> On 2013/01/09 22:15:50, viettrungluu wrote:
> > https://codereview.chromium.org/11819041/diff/1/ipc/ipc_channel_unittest.h
> > File ipc/ipc_channel_unittest.h (right):
> > 
> >
>
https://codereview.chromium.org/11819041/diff/1/ipc/ipc_channel_unittest.h#ne...
> > ipc/ipc_channel_unittest.h:10: class IPCChannelTest : public IPCTestBase {
> > On 2013/01/09 21:56:53, John Abd-El-Malek wrote:
> > > what's the point of this file? i.e. why can't whoever is deriving from
this
> > just
> > > derive from IPCTestBase (other than IPCChannelTest cc file of course)
> > 
> > ipc_send_fds_test.cc adds (POSIX-only) tests to IPCChannelTest.
> > 
> > Possibly, these tests should go in ipc_channel_posix_unittest.cc instead.
But
> > that file defines an IPCChannelPosixTest that doesn't derive from
IPCTestBase
> > (formerly known as IPCChannelTest). Agh.
> > 
> > Or maybe we should just put the tests in ipc_send_fds_test.cc into their own
> > (differently-named) test.
> 
> (Do you want me to do this? Or do you have another suggestion?)

I think this seems the least confusing for someone reading the code. if tests
are in different cc files, it really seems they should have a different fixture
name
> 
> > 
> > All things being equal, I'd prefer not to rename tests (because it messes up
> > test histories, etc.), but maybe that's the price of sanity.

[viettrungluu, 2013-01-10 17:43:50]

On 2013/01/10 17:14:24, John Abd-El-Malek wrote:
> > > Or maybe we should just put the tests in ipc_send_fds_test.cc into their
own
> > > (differently-named) test.
> > 
> > (Do you want me to do this? Or do you have another suggestion?)
> 
> I think this seems the least confusing for someone reading the code. if tests
> are in different cc files, it really seems they should have a different
fixture
> name

Thanks, done. PTAL.

[jam, 2013-01-11 01:18:28]

lgtm

[commit-bot: I haz the power, 2013-01-11 03:05:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/viettrungluu@chromium.org/11819041/9014

[commit-bot: I haz the power, 2013-01-11 13:35:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/viettrungluu@chromium.org/11819041/9014

[commit-bot: I haz the power, 2013-01-11 13:43:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/viettrungluu@chromium.org/11819041/9014

[commit-bot: I haz the power, 2013-01-11 13:53:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/viettrungluu@chromium.org/11819041/9014

[commit-bot: I haz the power, 2013-01-11 15:13:38]

Change committed as 176341