Merge video_capture tests into service_unittests

Merge video_capture tests into service_unittests

BUG=686329
Review-Url: https://codereview.chromium.org/2660743002
Cr-Commit-Position: refs/heads/master@{#447176}
Committed: https://chromium.googlesource.com/chromium/src/+/942b85c7245a77a477c276a9bfc51d1507ff484f

[Ken Rockot(use gerrit already), 2017-01-28 01:40:01]

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

[commit-bot: I haz the power, 2017-01-28 01:40:47]

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

[Ken Rockot(use gerrit already), 2017-01-28 01:42:06]

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

[commit-bot: I haz the power, 2017-01-28 01:42:21]

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

[Ken Rockot(use gerrit already), 2017-01-28 01:47:29]

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

[commit-bot: I haz the power, 2017-01-28 01:47:53]

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, 2017-01-28 02:18:20]

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

[commit-bot: I haz the power, 2017-01-28 02:18:20]

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

[Ken Rockot(use gerrit already), 2017-01-30 17:29:15]

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

[commit-bot: I haz the power, 2017-01-30 17:29:29]

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, 2017-01-30 18:47:38]

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

[commit-bot: I haz the power, 2017-01-30 18:47:39]

Dry run: This issue passed the CQ dry run.

[Ken Rockot(use gerrit already), 2017-01-30 18:51:18]

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

[Ken Rockot(use gerrit already), 2017-01-30 18:51:32]

Description was changed from

==========
Merge video_capture tests into service_unittests

Also re-enables a few video_capture tests.

BUG=686329
==========

to

==========
Merge video_capture tests into service_unittests

Also renames a "ServiceTest" fixture to "VideoCaptureServiceTest"
so it's easier to distinguish from other services' tests in the
test suite output.

BUG=686329
==========

[commit-bot: I haz the power, 2017-01-30 18:51:49]

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

[Ken Rockot(use gerrit already), 2017-01-30 18:53:01]

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

[Ken Rockot(use gerrit already), 2017-01-30 18:53:02]

chfremer@ can you take a look as author of video_capture stuff?

[commit-bot: I haz the power, 2017-01-30 20:03:33]

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

[commit-bot: I haz the power, 2017-01-30 20:03:33]

Dry run: This issue passed the CQ dry run.

[chfremer, 2017-01-30 20:05:00]

Thanks, Ken. I think this is very useful.
My only concern is regarding the redundancy of the VideoCapture* prefix (see
code comment).

Other than that, lgtm.

https://codereview.chromium.org/2660743002/diff/80001/services/video_capture/...
File services/video_capture/test/video_capture_service_test.h (right):

https://codereview.chromium.org/2660743002/diff/80001/services/video_capture/...
services/video_capture/test/video_capture_service_test.h:16: class
VideoCaptureServiceTest : public service_manager::test::ServiceTest {
I understand that this is for being able to identify the test fixture the output
of test runs. 

I originally had all classes, including these tests, use the VideoCapture*
prefix and then decided to drop it, because it seemed awfully redundant. If we
now need the prefix for disambiguating tests from output of test runs, should we
add the prefix back in not just here, but for all the tests, e.g. the names
FakeDeviceTest and FakeDeviceDescriptorTest may otherwise also be ambiguous.

Alternatively, would it be possible to have the test runner output the
namespaces? This way we could avoid a lot of redundancy in the names.

[Ken Rockot(use gerrit already), 2017-01-30 20:08:08]

On Mon, Jan 30, 2017 at 12:05 PM, <chfremer@chromium.org> wrote:

> Thanks, Ken. I think this is very useful.
> My only concern is regarding the redundancy of the VideoCapture* prefix
> (see
> code comment).
>
> Other than that, lgtm.
>
>
> https://codereview.chromium.org/2660743002/diff/80001/
> services/video_capture/test/video_capture_service_test.h
> File services/video_capture/test/video_capture_service_test.h (right):
>
> https://codereview.chromium.org/2660743002/diff/80001/
> services/video_capture/test/video_capture_service_test.h#newcode16
> services/video_capture/test/video_capture_service_test.h:16: class
> VideoCaptureServiceTest : public service_manager::test::ServiceTest {
> I understand that this is for being able to identify the test fixture
> the output of test runs.
>
> I originally had all classes, including these tests, use the
> VideoCapture* prefix and then decided to drop it, because it seemed
> awfully redundant. If we now need the prefix for disambiguating tests
> from output of test runs, should we add the prefix back in not just
> here, but for all the tests, e.g. the names FakeDeviceTest and
> FakeDeviceDescriptorTest may otherwise also be ambiguous.
>
> Alternatively, would it be possible to have the test runner output the
> namespaces? This way we could avoid a lot of redundancy in the names.
>

I don't see how that's possible without introducing a new TEST_FOO macro,
which seems like an unnecessary new thing. I'll update the other fixtures
for now.


> https://codereview.chromium.org/2660743002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Ken Rockot(use gerrit already), 2017-01-30 20:45:30]

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

[Ken Rockot(use gerrit already), 2017-01-30 20:45:53]

rockot@chromium.org changed reviewers:
+ dpranke@chromium.org

[Ken Rockot(use gerrit already), 2017-01-30 20:45:53]

+dpranke@ for top-level BUILD.gn

[commit-bot: I haz the power, 2017-01-30 20:46:16]

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, 2017-01-30 21:48:56]

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

[commit-bot: I haz the power, 2017-01-30 21:48:57]

Dry run: This issue passed the CQ dry run.

[Dirk Pranke, 2017-01-31 03:09:56]

lgtm

[Ken Rockot(use gerrit already), 2017-01-31 03:21:12]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2017-01-31 03:21:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
chfremer@chromium.org
Link to the patchset: https://codereview.chromium.org/2660743002/#ps100001
(title: ".")

[Ken Rockot(use gerrit already), 2017-01-31 03:21:22]

Description was changed from

==========
Merge video_capture tests into service_unittests

Also renames a "ServiceTest" fixture to "VideoCaptureServiceTest"
so it's easier to distinguish from other services' tests in the
test suite output.

BUG=686329
==========

to

==========
Merge video_capture tests into service_unittests

BUG=686329
==========

[commit-bot: I haz the power, 2017-01-31 03:21:37]

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, 2017-01-31 03:26:14]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1485832872399920,
"parent_rev": "9e3e9494dcbd592e5ca4e1d3f058d797b0343dad", "commit_rev":
"942b85c7245a77a477c276a9bfc51d1507ff484f"}

[commit-bot: I haz the power, 2017-01-31 03:27:07]

Description was changed from

==========
Merge video_capture tests into service_unittests

BUG=686329
==========

to

==========
Merge video_capture tests into service_unittests

BUG=686329

Review-Url: https://codereview.chromium.org/2660743002
Cr-Commit-Position: refs/heads/master@{#447176}
Committed:
https://chromium.googlesource.com/chromium/src/+/942b85c7245a77a477c276a9bfc5...
==========

[commit-bot: I haz the power, 2017-01-31 03:27:09]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/942b85c7245a77a477c276a9bfc5...