[Mojo Video Capture] Separate testing from production code

[Mojo Video Capture] Separate testing from production code

The goal of this CL is to get a clear separation of testing-only code
from production code in services/video_capture.
* Removed testing-only APIs and code from production service "video_capture"
* Created new service "testable_video_capture" which is a superset of
  "video_capture" that additionally exposes testing-only APIs and links
  testing-only code.
* Moved testing-only code to subfolder test.

This CL is part of the Mojo Video Capture work. For the bigger picture,
see [1] CL1.9.5b

BUG=584797
TEST=video_capture_unittests, content_unittests, capture_unittests

[1] https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxsxWslJ_X-zpOhijvI8/edit?usp=sharing

Committed: https://crrev.com/4c0567125d08240ce3512a13ede21e21e7b8a269
Cr-Commit-Position: refs/heads/master@{#433798}

[chfremer, 2016-11-16 22:03:36]

Description was changed from

==========
presubmit


Merge branch '1.9.5a' into 1.9.5b


Merge branch '1.9.5a' into 1.9.5b


Merge branch '1.9.5a' into 1.9.5b


Merge branch '1.9.5a' into 1.9.5b


Merge branch '1.9.5a' into 1.9.5b


Finished separation of test code.


Mostly done, but mock_device_test unable to add mock device.


WIP


WIP


WIP separate testing from production

BUG=
==========

to

==========
[Mojo Video Capture] Separate testing from production code

The goal of this CL is to get a clear separation of testing-only code 
from production code in services/video_capture.
* Removed testing-only APIs and code from production service "video_capture"
* Created new service "testable_video_capture" which is a superset of 
  "video_capture" that additionally exposes testing-only APIs and links
  testing-only code.
* Moved testing-only code to subfolder test.

This CL is part of the Mojo Video Capture work. For the bigger picture, 
see [1] CL1.9.5b

BUG=584797
TEST=video_capture_unittests, content_unittests, capture_unittests

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...
==========

[chfremer, 2016-11-16 22:05:39]

chfremer@chromium.org changed reviewers:
+ emircan@chromium.org, mcasas@chromium.org, rockot@chromium.org

[chfremer, 2016-11-16 22:05:40]

mcasas@: PTAL
rockot@: Please give feedback on approach from a Mojo perspective
emircan@: FYI

[chfremer, 2016-11-17 22:09:06]

On 2016/11/16 22:05:40, chfremer wrote:
> mcasas@: PTAL
> rockot@: Please give feedback on approach from a Mojo perspective
> emircan@: FYI

ping

[Ken Rockot(use gerrit already), 2016-11-17 22:27:17]

I am a bit confused by this approach. Why define a new service just for tests?
Why not just unit-test the individual components of the service?

And if you need integration testing, it doesn't seem like a separate test
service actually provides that coverage.

In general the pattern of having a testable_foo_service for every foo_service
seems highly undesirable and I would like to avoid it.

[chfremer, 2016-11-17 22:50:22]

On 2016/11/17 22:27:17, Ken Rockot wrote:
> I am a bit confused by this approach. Why define a new service just for tests?
> Why not just unit-test the individual components of the service?
> 
> And if you need integration testing, it doesn't seem like a separate test
> service actually provides that coverage.
> 
> In general the pattern of having a testable_foo_service for every foo_service
> seems highly undesirable and I would like to avoid it.

Thanks, Ken. That is very useful feedback.
I guess my idea was to have a component test that tests the service "from the
outside",
such that test coverage includes both the implementation and the "wrapping"
service shell.
But you are right, that using a separate service for testing makes the coverage
of the 
"wrapping" service shell useless. 

Some of the tests still exercise the production service, i.e. ServiceTest, 
FakeDeviceDescriptorTest, and FakeDeviceTest.

Let me change the MockDeviceTest so that it still tests the component "as a
whole" 
but does so "from the inside", i.e. not going through a service boundary. This
should 
eliminate the need for the separate service.

Will post back as a PatchSet.

[chfremer, 2016-11-18 00:23:36]

PTAL

I modified MockDeviceTest to exercise the service implementation direction
instead of going through the service shell. This gives us the same test
coverage, but eliminates a lot of overhead. In particular, we no longer have to
wrap mock devices to control them across a Mojo boundary.

[chfremer, 2016-11-18 22:15:33]

On 2016/11/18 00:23:36, chfremer wrote:
> PTAL
> 
> I modified MockDeviceTest to exercise the service implementation direction
> instead of going through the service shell. This gives us the same test
> coverage, but eliminates a lot of overhead. In particular, we no longer have
to
> wrap mock devices to control them across a Mojo boundary.

ping

[Ken Rockot(use gerrit already), 2016-11-21 23:22:46]

lgtm

[mcasas, 2016-11-21 23:29:41]

lgtm

[chfremer, 2016-11-21 23:31:04]

chfremer@chromium.org changed reviewers:
+ tsepez@chromium.org

[chfremer, 2016-11-21 23:31:05]

tsepez@: Please RS *.mojom

[Tom Sepez, 2016-11-21 23:45:39]

RS LGTM

[chfremer, 2016-11-21 23:55:52]

The CQ bit was checked by chfremer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 23:56:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-22 00:05:08]

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

[commit-bot: I haz the power, 2016-11-22 00:05:09]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[chfremer, 2016-11-22 01:26:06]

The CQ bit was checked by chfremer@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-22 01:26:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-22 03:03:17]

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

[commit-bot: I haz the power, 2016-11-22 03:03:18]

Dry run: This issue passed the CQ dry run.

[chfremer, 2016-11-22 05:49:56]

The CQ bit was checked by chfremer@chromium.org

[chfremer, 2016-11-22 05:49:56]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
rockot@chromium.org, mcasas@chromium.org
Link to the patchset: https://codereview.chromium.org/2505143002/#ps40001
(title: "Removed #include of removed header file.")

[commit-bot: I haz the power, 2016-11-22 05:50:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-22 05:59:28]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1479793796060060,
"parent_rev": "e0ae7aa6e7a054ee274a4102f9df2d64dfb62b20", "commit_rev":
"ca3df55255066e49bc920465937a6150dfe867dc"}

[commit-bot: I haz the power, 2016-11-22 05:59:51]

Description was changed from

==========
[Mojo Video Capture] Separate testing from production code

The goal of this CL is to get a clear separation of testing-only code 
from production code in services/video_capture.
* Removed testing-only APIs and code from production service "video_capture"
* Created new service "testable_video_capture" which is a superset of 
  "video_capture" that additionally exposes testing-only APIs and links
  testing-only code.
* Moved testing-only code to subfolder test.

This CL is part of the Mojo Video Capture work. For the bigger picture, 
see [1] CL1.9.5b

BUG=584797
TEST=video_capture_unittests, content_unittests, capture_unittests

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...
==========

to

==========
[Mojo Video Capture] Separate testing from production code

The goal of this CL is to get a clear separation of testing-only code 
from production code in services/video_capture.
* Removed testing-only APIs and code from production service "video_capture"
* Created new service "testable_video_capture" which is a superset of 
  "video_capture" that additionally exposes testing-only APIs and links
  testing-only code.
* Moved testing-only code to subfolder test.

This CL is part of the Mojo Video Capture work. For the bigger picture, 
see [1] CL1.9.5b

BUG=584797
TEST=video_capture_unittests, content_unittests, capture_unittests

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...
==========

[commit-bot: I haz the power, 2016-11-22 05:59:52]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-22 06:02:14]

Description was changed from

==========
[Mojo Video Capture] Separate testing from production code

The goal of this CL is to get a clear separation of testing-only code 
from production code in services/video_capture.
* Removed testing-only APIs and code from production service "video_capture"
* Created new service "testable_video_capture" which is a superset of 
  "video_capture" that additionally exposes testing-only APIs and links
  testing-only code.
* Moved testing-only code to subfolder test.

This CL is part of the Mojo Video Capture work. For the bigger picture, 
see [1] CL1.9.5b

BUG=584797
TEST=video_capture_unittests, content_unittests, capture_unittests

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...
==========

to

==========
[Mojo Video Capture] Separate testing from production code

The goal of this CL is to get a clear separation of testing-only code
from production code in services/video_capture.
* Removed testing-only APIs and code from production service "video_capture"
* Created new service "testable_video_capture" which is a superset of
  "video_capture" that additionally exposes testing-only APIs and links
  testing-only code.
* Moved testing-only code to subfolder test.

This CL is part of the Mojo Video Capture work. For the bigger picture,
see [1] CL1.9.5b

BUG=584797
TEST=video_capture_unittests, content_unittests, capture_unittests

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...

Committed: https://crrev.com/4c0567125d08240ce3512a13ede21e21e7b8a269
Cr-Commit-Position: refs/heads/master@{#433798}
==========

[commit-bot: I haz the power, 2016-11-22 06:02:16]

Patchset 3 (id:??) landed as
https://crrev.com/4c0567125d08240ce3512a13ede21e21e7b8a269
Cr-Commit-Position: refs/heads/master@{#433798}