Telemetry: Mock out subprocess in VTune profiler unit test.

Telemetry: Mock out subprocess in VTune profiler unit test.

Instead of using simple_mock.MockObject for mocking out subprocess, this CL introduces a MockSubprocess class which is more flexible in handling calls. This way we can avoid the problem of having to know in advance how many calls the profiler will make to subprocess.

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

[Dominik Grewe, 2014-05-20 15:46:43]

Ptal. This is different from what I had originally suggested on the bug, but it
seems a lot cleaner.

[tonyg, 2014-05-21 08:45:00]

Rather than mock out the code under test, can we also fix this by instead making
the code under test more robust? ie. handling the ProcessGoneException? It's
also possible I don't fully understand the issue.

[Dominik Grewe, 2014-05-21 08:50:49]

On 2014/05/21 08:45:00, tonyg wrote:
> Rather than mock out the code under test, can we also fix this by instead
making
> the code under test more robust? ie. handling the ProcessGoneException? It's
> also possible I don't fully understand the issue.

Yes, we should make the code more robust. I'll do that in a separate patch.

In the test we still have the potential problem though that (at least
theoretically) a process can disappear between the test code querying the number
of processes and the profiler querying the number of processes. This could lead
to a mismatch in the calls we're expecting and the calls we actually get,
causing the test to fail. I just wanted to make sure we never run into that
problem.

[tonyg, 2014-05-21 09:30:41]

lgtm

I think I understand the rationale now.

[Dominik Grewe, 2014-05-21 10:57:29]

Having discussed this with Sami we came up with a cleaner implementation: by
having a MockSubprocess class rather than using simple_mock.MockObject we can
avoid the problem of having to know in advance what calls to subprocess the
profiler will make. Ptal.

[Sami, 2014-05-21 11:15:47]

Thanks, lgtm! Looks pretty straightforward now.

[tonyg, 2014-05-21 11:15:56]

lgtm

Much better!

[Dominik Grewe, 2014-05-21 11:23:24]

The CQ bit was checked by dominikg@chromium.org

[commit-bot: I haz the power, 2014-05-21 11:25:38]

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

[commit-bot: I haz the power, 2014-05-21 23:52:42]

Change committed as 272023