Chromium Code Reviews
Help | Chromium Project | Sign in
(2)

Issue 2828793003: GN: Tighten up test target visibility + refactorings (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 days, 12 hours ago by kjellander_webrtc
Modified:
1 day, 18 hours ago
Reviewers:
stefan-webrtc
CC:
webrtc-reviews_webrtc.org, yujie_mao (webrtc), zhuangzesen_agora.io, aleloi, tlegrand-webrtc, qiang.lu, peah-webrtc, bjornv1, video-team_agora.io, AleBzk, tterriberry_mozilla.com, sdk-team_agora.io, minyue-webrtc, mflodman, Andrew MacDonald, zhengzhonghou_agora.io, stefan-webrtc, kwiberg-webrtc, danilchap, henrika_webrtc, audio-team_agora.io, hlundin-webrtc, niklas.enbom, the sun, aluebs-webrtc, Taylor Brandstetter, mbonadei
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

GN: Tighten up test target visibility + refactorings Make all rtc_source_test target that contains tests that are included in a test executable only be visible to the rtc_test target. Doing this exposed a couple of errors and dependency problems that were resolved. Having this could have prevented duplicated execution of tests like the case that was recently fixed by deadbeef@ in https://codereview.webrtc.org/2820263004 New targets: * //webrtc/modules/rtp_rtcp:fec_test_helper * //webrtc/modules/rtp_rtcp:mock_rtp_rtcp * //webrtc/modules/remote_bitrate_estimator:mock_remote_bitrate_observer The mock files and targets should probably be moved into webrtc/test in the future, but that's out of the scope of this CL. BUG=webrtc:5716 NOTRY=True Review-Url: https://codereview.webrtc.org/2828793003 Cr-Commit-Position: refs/heads/master@{#17863} Committed: https://chromium.googlesource.com/external/webrtc/+/e0629c045e699a896682edc1c0c67dab0f26428d

Patch Set 1 #

Patch Set 2 : Exclude Android and iOS #

Patch Set 3 : Add fec_test_helper target #

Patch Set 4 : Disable Win compile warning #

Total comments: 6

Patch Set 5 : Fixed comments and a few errors #

Patch Set 6 : Rebased #

Patch Set 7 : Fixed error in video_tests #

Patch Set 8 : Fix libfuzzer breakage #

Patch Set 9 : Move out fec_test_helper from rtc_include_tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+319 lines, -19 lines) Patch
M webrtc/BUILD.gn View 1 2 3 4 5 6 7 8 3 chunks +0 lines, -3 lines 0 comments Download
M webrtc/api/BUILD.gn View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
M webrtc/audio/BUILD.gn View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
M webrtc/base/BUILD.gn View 1 2 3 4 5 5 chunks +35 lines, -0 lines 0 comments Download
M webrtc/call/BUILD.gn View 1 2 3 4 5 2 chunks +14 lines, -0 lines 0 comments Download
M webrtc/modules/audio_coding/BUILD.gn View 1 2 3 4 5 3 chunks +20 lines, -0 lines 0 comments Download
M webrtc/modules/audio_conference_mixer/BUILD.gn View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
M webrtc/modules/audio_device/BUILD.gn View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download
M webrtc/modules/audio_mixer/BUILD.gn View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
M webrtc/modules/audio_processing/BUILD.gn View 1 2 3 4 2 chunks +14 lines, -0 lines 0 comments Download
M webrtc/modules/bitrate_controller/BUILD.gn View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
M webrtc/modules/congestion_controller/BUILD.gn View 1 2 3 4 2 chunks +8 lines, -1 line 0 comments Download
M webrtc/modules/desktop_capture/BUILD.gn View 1 2 3 4 2 chunks +14 lines, -0 lines 0 comments Download
M webrtc/modules/media_file/BUILD.gn View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
M webrtc/modules/pacing/BUILD.gn View 1 2 3 4 2 chunks +8 lines, -1 line 0 comments Download
M webrtc/modules/remote_bitrate_estimator/BUILD.gn View 1 2 3 4 4 chunks +26 lines, -1 line 0 comments Download
M webrtc/modules/rtp_rtcp/BUILD.gn View 1 2 3 4 5 6 7 8 4 chunks +47 lines, -3 lines 0 comments Download
M webrtc/modules/utility/BUILD.gn View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
M webrtc/modules/video_coding/BUILD.gn View 1 2 3 4 2 chunks +13 lines, -0 lines 0 comments Download
M webrtc/modules/video_processing/BUILD.gn View 1 2 3 4 1 chunk +7 lines, -0 lines 0 comments Download
M webrtc/p2p/BUILD.gn View 1 2 3 4 5 2 chunks +14 lines, -0 lines 0 comments Download
M webrtc/sdk/BUILD.gn View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download
M webrtc/test/BUILD.gn View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M webrtc/test/fuzzers/BUILD.gn View 1 2 3 4 5 6 7 8 chunks +11 lines, -8 lines 0 comments Download
M webrtc/tools/network_tester/BUILD.gn View 1 2 3 4 1 chunk +8 lines, -1 line 0 comments Download
M webrtc/video/BUILD.gn View 1 2 3 4 5 6 4 chunks +16 lines, -1 line 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 17 (9 generated)
kjellander_webrtc
Check PS#3 for majority of bots. PS#4 only contains win fix.
6 days, 10 hours ago (2017-04-20 19:06:35 UTC) #4
kjellander_webrtc
On 2017/04/20 19:06:35, kjellander_webrtc wrote: > Check PS#3 for majority of bots. PS#4 only contains ...
5 days, 20 hours ago (2017-04-21 09:13:44 UTC) #5
kjellander_webrtc
Clarified some decisions with comments https://codereview.webrtc.org/2828793003/diff/60001/webrtc/BUILD.gn File webrtc/BUILD.gn (left): https://codereview.webrtc.org/2828793003/diff/60001/webrtc/BUILD.gn#oldcode285 webrtc/BUILD.gn:285: "api:rtc_api_unittests", The removed targets ...
5 days, 17 hours ago (2017-04-21 12:13:59 UTC) #8
stefan-webrtc
https://codereview.webrtc.org/2828793003/diff/60001/webrtc/api/BUILD.gn File webrtc/api/BUILD.gn (right): https://codereview.webrtc.org/2828793003/diff/60001/webrtc/api/BUILD.gn#newcode233 webrtc/api/BUILD.gn:233: if (!is_android && !is_ios) { # Generated targets makes ...
5 days, 16 hours ago (2017-04-21 12:28:28 UTC) #9
kjellander_webrtc
PTAL https://codereview.webrtc.org/2828793003/diff/60001/webrtc/api/BUILD.gn File webrtc/api/BUILD.gn (right): https://codereview.webrtc.org/2828793003/diff/60001/webrtc/api/BUILD.gn#newcode233 webrtc/api/BUILD.gn:233: if (!is_android && !is_ios) { # Generated targets ...
5 days, 16 hours ago (2017-04-21 12:48:21 UTC) #10
stefan-webrtc
lgtm
1 day, 20 hours ago (2017-04-25 08:31:57 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/2828793003/160001
1 day, 18 hours ago (2017-04-25 11:02:16 UTC) #14
commit-bot: I haz the power
1 day, 18 hours ago (2017-04-25 11:04:56 UTC) #17
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/external/webrtc/+/e0629c045e699a896682edc1c...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46