Add multiprocess test helper functions.

Add multiprocess test helper functions.

Don't force people who want to write multiprocess tests to derive their
test fixture from MultiProcessTest.

TBR=phajdan.jr@chromium.org

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

[Paweł Hajdan Jr., 2014-03-07 02:08:00]

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test.cc
File base/test/multiprocess_test.cc (right):

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test....
base/test/multiprocess_test.cc:27: #endif
nit:   // !defined(OS_ANDROID)

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test....
base/test/multiprocess_test.cc:57: CommandLine
MultiProcessTest::MakeCmdLine(const std::string& /*procname*/,
nit: AFAIK we don't comment out parameter names.

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test.h
File base/test/multiprocess_test.h (right):

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test....
base/test/multiprocess_test.h:43: //    
CHECK(base::WaitForExitCodeWithTimeout(test_child_handle, &rv,
Please change this to ASSERT_TRUE.

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test....
base/test/multiprocess_test.h:44: //                                           
base::TimeDelta::FromSeconds(5)));
This should be using base/test/test_timeouts. Ideally this would be just another
helper.

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test....
base/test/multiprocess_test.h:65: bool debug_on_start);
Is the debug_on_start parameter useful/needed.

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test_...
File base/test/multiprocess_test_android.cc (right):

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test_...
base/test/multiprocess_test_android.cc:23: const CommandLine&
/*base_command_line*/,
nit: Why comment out parameter names?

[viettrungluu, 2014-03-07 04:42:44]

Thanks, ptal.

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test.cc
File base/test/multiprocess_test.cc (right):

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test....
base/test/multiprocess_test.cc:27: #endif
On 2014/03/07 02:08:01, Paweł Hajdan Jr. wrote:
> nit:   // !defined(OS_ANDROID)

Done, but note that these are generally considered optional if the #if-#endif
block is small and there's no risk of confusion.

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test....
base/test/multiprocess_test.cc:57: CommandLine
MultiProcessTest::MakeCmdLine(const std::string& /*procname*/,
On 2014/03/07 02:08:01, Paweł Hajdan Jr. wrote:
> nit: AFAIK we don't comment out parameter names.

Removed. <shrug>

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test.h
File base/test/multiprocess_test.h (right):

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test....
base/test/multiprocess_test.h:43: //    
CHECK(base::WaitForExitCodeWithTimeout(test_child_handle, &rv,
On 2014/03/07 02:08:01, Paweł Hajdan Jr. wrote:
> Please change this to ASSERT_TRUE.

Done, though note that it's debatable whether it should be a CHECK() or an
ASSERT_TRUE(), and is dependent on context. In particular, ASSERT_x() generally
implies that a failure would have something to do with the unit being tested;
if, e.g., the child startup/shutdown code is well-established and tested
elsewhere, then this would logically be a CHECK(), not an ASSERT_TRUE().

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test....
base/test/multiprocess_test.h:44: //                                           
base::TimeDelta::FromSeconds(5)));
On 2014/03/07 02:08:01, Paweł Hajdan Jr. wrote:
> This should be using base/test/test_timeouts.

Done.

> Ideally this would be just another
> helper.

Yes. I'll do that separately, where I'll actually use it. (Or maybe I'll add a
test for it. Our test harness code is ironically woefully undertested.)

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test....
base/test/multiprocess_test.h:65: bool debug_on_start);
On 2014/03/07 02:08:01, Paweł Hajdan Jr. wrote:
> Is the debug_on_start parameter useful/needed.

I'll investigate and remove it separately if appropriate. (I suspect I'll end up
removing it.)

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test_...
File base/test/multiprocess_test_android.cc (right):

https://codereview.chromium.org/189373002/diff/1/base/test/multiprocess_test_...
base/test/multiprocess_test_android.cc:23: const CommandLine&
/*base_command_line*/,
On 2014/03/07 02:08:01, Paweł Hajdan Jr. wrote:
> nit: Why comment out parameter names?

Removed, though I believe the style guide permits these.

There are two reasons:
- signal intent (i.e., that it's intentionally not used), while retaining
documentary value (of the parameter name)
- prevent warnings, if the code is ever compiled with the unused-parameter
warning enabled

E.g., here it makes it clear that this implementation is total b.s. and ignores
lots of things that it shouldn't.

[viettrungluu, 2014-03-07 22:49:51]

Friendly ping.

[viettrungluu, 2014-03-08 02:47:26]

TBRing.

[viettrungluu, 2014-03-08 02:47:30]

The CQ bit was checked by viettrungluu@chromium.org

[commit-bot: I haz the power, 2014-03-08 11:17:24]

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

[commit-bot: I haz the power, 2014-03-08 11:45:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-08 11:45:06]

Try jobs failed on following builders: mac_chromium_compile_dbg,
mac_chromium_rel

[viettrungluu, 2014-03-08 16:14:53]

The CQ bit was checked by viettrungluu@chromium.org

[commit-bot: I haz the power, 2014-03-08 16:15:20]

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

[commit-bot: I haz the power, 2014-03-08 16:55:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-08 16:55:27]

Retried try job too often on mac_rel for step(s) telemetry_perf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[viettrungluu, 2014-03-08 17:09:07]

The CQ bit was checked by viettrungluu@chromium.org

[commit-bot: I haz the power, 2014-03-08 17:09:14]

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

[commit-bot: I haz the power, 2014-03-08 18:30:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-08 18:30:17]

Retried try job too often on mac_rel for step(s) telemetry_perf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[viettrungluu, 2014-03-08 21:01:35]

Committed patchset #3 manually as r255796 (presubmit successful).