Enabling 'gn check' on webrtc/video

Enabling 'gn check' on webrtc/video.

This CL also renames webrtc/media:rtc_unittest_main to
webrtc/media:rtc_media_tests_utils and it pushes the dependency
on webrtc/base:rtc_base_tests_main down in the dependency
hierarchy so we can avoid a double definition of the main function.

BUG=webrtc:6828
NOTRY=True

Review-Url: https://codereview.webrtc.org/2832063003
Cr-Commit-Position: refs/heads/master@{#17859}
Committed: https://chromium.googlesource.com/external/webrtc/+/9087d49b8396ff0f88085f2175aa67d9c0a24c99

[mbonadei, 2017-04-21 14:44:17]

Description was changed from

==========
Enabling 'gn check' on webrtc/video

BUG=webrtc:6828
==========

to

==========
Enabling 'gn check' on webrtc/video.

I disabled the check on "video_tests" because it pulls
"//webrtc/media/rtc_unittest_main" as a dependency and it defines
the _main (that is already defined by "//webrtc/test:test_main").

I will file a bug to solve this in another CL.

BUG=webrtc:6828
==========

[mbonadei, 2017-04-21 14:44:32]

mbonadei@webrtc.org changed reviewers:
+ kjellander@google.com

[kjellander_webrtc, 2017-04-24 05:12:51]

kjellander@webrtc.org changed reviewers:
+ kjellander@webrtc.org
- kjellander@google.com

[kjellander_webrtc, 2017-04-24 05:14:33]

https://codereview.webrtc.org/2832063003/diff/20001/webrtc/video/BUILD.gn
File webrtc/video/BUILD.gn (right):

https://codereview.webrtc.org/2832063003/diff/20001/webrtc/video/BUILD.gn#new...
webrtc/video/BUILD.gn:217: # //webrtc/media/rtc_unittest_main.
Can you break rtc_unittest_main into two targets so we can depend on the one
this target needs without pulling in the main part? Or just move that file to
another existing target?

[mbonadei, 2017-04-24 12:10:30]

On 2017/04/24 05:14:33, kjellander_webrtc wrote:
> https://codereview.webrtc.org/2832063003/diff/20001/webrtc/video/BUILD.gn
> File webrtc/video/BUILD.gn (right):
> 
>
https://codereview.webrtc.org/2832063003/diff/20001/webrtc/video/BUILD.gn#new...
> webrtc/video/BUILD.gn:217: # //webrtc/media/rtc_unittest_main.
> Can you break rtc_unittest_main into two targets so we can depend on the one
> this target needs without pulling in the main part? Or just move that file to
> another existing target?

It seems that pushing 'webrtc/base:rtc_base_tests_main' down in the dependency
hierarchy solves the problem.
It also make sense that _unittests targets have their dependency on the target
with the main function instead of getting that for free from 'rtc_unittest_main'
that even if the name seem legit, it contains also classes that are useful in a
non testing context.

[mbonadei, 2017-04-24 12:10:39]

https://codereview.webrtc.org/2832063003/diff/40001/webrtc/media/BUILD.gn
File webrtc/media/BUILD.gn (right):

https://codereview.webrtc.org/2832063003/diff/40001/webrtc/media/BUILD.gn#new...
webrtc/media/BUILD.gn:264: rtc_source_set("rtc_unittest_main") {
I think that the _main suffix is misleading here.
It seems to be a set of utilities, should we rename to something else?

[kjellander_webrtc, 2017-04-24 12:41:43]

Let's rename it first.
This kind of relates to https://bugs.chromium.org/p/webrtc/issues/detail?id=5996
so you could reference that if you update the CL description a little.

https://codereview.webrtc.org/2832063003/diff/40001/webrtc/media/BUILD.gn
File webrtc/media/BUILD.gn (right):

https://codereview.webrtc.org/2832063003/diff/40001/webrtc/media/BUILD.gn#new...
webrtc/media/BUILD.gn:264: rtc_source_set("rtc_unittest_main") {
On 2017/04/24 12:10:39, mbonadei wrote:
> I think that the _main suffix is misleading here.
> It seems to be a set of utilities, should we rename to something else?

Yeah, let's rename it to rtc_media_tests_utils. Then it's kind of similar to
//webrtc/base:rtc_base_tests_utils.

[mbonadei, 2017-04-24 13:20:14]

On 2017/04/24 12:41:43, kjellander_webrtc wrote:
> Let's rename it first.
> This kind of relates to
https://bugs.chromium.org/p/webrtc/issues/detail?id=5996
> so you could reference that if you update the CL description a little.
> 
> https://codereview.webrtc.org/2832063003/diff/40001/webrtc/media/BUILD.gn
> File webrtc/media/BUILD.gn (right):
> 
>
https://codereview.webrtc.org/2832063003/diff/40001/webrtc/media/BUILD.gn#new...
> webrtc/media/BUILD.gn:264: rtc_source_set("rtc_unittest_main") {
> On 2017/04/24 12:10:39, mbonadei wrote:
> > I think that the _main suffix is misleading here.
> > It seems to be a set of utilities, should we rename to something else?
> 
> Yeah, let's rename it to rtc_media_tests_utils. Then it's kind of similar to
> //webrtc/base:rtc_base_tests_utils.

PTAL :)

[kjellander_webrtc, 2017-04-24 13:37:06]

https://codereview.webrtc.org/2832063003/diff/60001/webrtc/media/BUILD.gn
File webrtc/media/BUILD.gn (right):

https://codereview.webrtc.org/2832063003/diff/60001/webrtc/media/BUILD.gn#new...
webrtc/media/BUILD.gn:264: rtc_source_set("rtc_unittest_utils") {
Can we use rtc_media_tests_utils instead?

I don't like the rtc_unittests part of the name, since it's only containing
media-specific files.

[mbonadei, 2017-04-24 14:16:31]

https://codereview.webrtc.org/2832063003/diff/60001/webrtc/media/BUILD.gn
File webrtc/media/BUILD.gn (right):

https://codereview.webrtc.org/2832063003/diff/60001/webrtc/media/BUILD.gn#new...
webrtc/media/BUILD.gn:264: rtc_source_set("rtc_unittest_utils") {
On 2017/04/24 13:37:06, kjellander_webrtc wrote:
> Can we use rtc_media_tests_utils instead?
> 
> I don't like the rtc_unittests part of the name, since it's only containing
> media-specific files.

Done.

[mbonadei, 2017-04-24 14:33:12]

Description was changed from

==========
Enabling 'gn check' on webrtc/video.

I disabled the check on "video_tests" because it pulls
"//webrtc/media/rtc_unittest_main" as a dependency and it defines
the _main (that is already defined by "//webrtc/test:test_main").

I will file a bug to solve this in another CL.

BUG=webrtc:6828
==========

to

==========
Enabling 'gn check' on webrtc/video.

I disabled the check on "video_tests" because it pulls
"//webrtc/media/rtc_unittest_main" as a dependency and it defines
the _main (that is already defined by "//webrtc/test:test_main").

I will file a bug to solve this in another CL.

BUG=webrtc:6828
NOTRY=True
==========

[kjellander_webrtc, 2017-04-24 14:33:45]

lgtm

[mbonadei, 2017-04-25 07:31:21]

The CQ bit was checked by mbonadei@webrtc.org

[mbonadei, 2017-04-25 07:31:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
kjellander@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2832063003/#ps120001 (title:
"Revert "running chromium trybots"")

[commit-bot: I haz the power, 2017-04-25 07:31:25]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-04-25 07:35:36]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1493105481173570,
"parent_rev": "df3a191471e65d5b4d1572908966dce0ba27b526", "commit_rev":
"9087d49b8396ff0f88085f2175aa67d9c0a24c99"}

[commit-bot: I haz the power, 2017-04-25 07:35:39]

Description was changed from

==========
Enabling 'gn check' on webrtc/video.

I disabled the check on "video_tests" because it pulls
"//webrtc/media/rtc_unittest_main" as a dependency and it defines
the _main (that is already defined by "//webrtc/test:test_main").

I will file a bug to solve this in another CL.

BUG=webrtc:6828
NOTRY=True
==========

to

==========
Enabling 'gn check' on webrtc/video.

I disabled the check on "video_tests" because it pulls
"//webrtc/media/rtc_unittest_main" as a dependency and it defines
the _main (that is already defined by "//webrtc/test:test_main").

I will file a bug to solve this in another CL.

BUG=webrtc:6828
NOTRY=True

Review-Url: https://codereview.webrtc.org/2832063003
Cr-Commit-Position: refs/heads/master@{#17859}
Committed:
https://chromium.googlesource.com/external/webrtc/+/9087d49b8396ff0f88085f217...
==========

[commit-bot: I haz the power, 2017-04-25 07:35:41]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/external/webrtc/+/9087d49b8396ff0f88085f217...

[kjellander_webrtc, 2017-04-25 07:41:51]

On 2017/04/25 07:35:41, commit-bot: I haz the power wrote:
> Committed patchset #7 (id:120001) as
>
https://chromium.googlesource.com/external/webrtc/+/9087d49b8396ff0f88085f217...

Ah, too bad I lgtm'ed before you updated the description. It became outdated,
but it's not that important.

[mbonadei, 2017-04-25 07:51:32]

Description was changed from

==========
Enabling 'gn check' on webrtc/video.

I disabled the check on "video_tests" because it pulls
"//webrtc/media/rtc_unittest_main" as a dependency and it defines
the _main (that is already defined by "//webrtc/test:test_main").

I will file a bug to solve this in another CL.

BUG=webrtc:6828
NOTRY=True

Review-Url: https://codereview.webrtc.org/2832063003
Cr-Commit-Position: refs/heads/master@{#17859}
Committed:
https://chromium.googlesource.com/external/webrtc/+/9087d49b8396ff0f88085f217...
==========

to

==========
Enabling 'gn check' on webrtc/video.

This CL also renames webrtc/media:rtc_unittest_main to
webrtc/media:rtc_media_tests_utils and it pushes the dependency
on webrtc/base:rtc_base_tests_main down in the dependency
hierarchy so we can avoid a double definition of the main function.

BUG=webrtc:6828
NOTRY=True

Review-Url: https://codereview.webrtc.org/2832063003
Cr-Commit-Position: refs/heads/master@{#17859}
Committed:
https://chromium.googlesource.com/external/webrtc/+/9087d49b8396ff0f88085f217...
==========

[kjellander_webrtc, 2017-04-25 08:50:13]

Found a mistake, but it's being fixed (and prevented) by my upcoming CL.

https://codereview.webrtc.org/2832063003/diff/120001/webrtc/video/BUILD.gn
File webrtc/video/BUILD.gn (right):

https://codereview.webrtc.org/2832063003/diff/120001/webrtc/video/BUILD.gn#ne...
webrtc/video/BUILD.gn:248: "../modules/rtp_rtcp:rtp_rtcp_unittests",
Ah, I missed that this target now pulled in additional tests with this
dependency.

Luckily I'm fixing that and preventing such from happening in the future with my
https://codereview.webrtc.org/2828793003/

[mbonadei, 2017-04-25 09:08:39]

https://codereview.webrtc.org/2832063003/diff/120001/webrtc/video/BUILD.gn
File webrtc/video/BUILD.gn (right):

https://codereview.webrtc.org/2832063003/diff/120001/webrtc/video/BUILD.gn#ne...
webrtc/video/BUILD.gn:248: "../modules/rtp_rtcp:rtp_rtcp_unittests",
On 2017/04/25 08:50:13, kjellander_webrtc wrote:
> Ah, I missed that this target now pulled in additional tests with this
> dependency.
> 
> Luckily I'm fixing that and preventing such from happening in the future with
my
> https://codereview.webrtc.org/2828793003/

Thanks, I missed that too!