Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2100173002:
Build webrtc_nonparallel_tests under GN. (Closed)
|
Created:
4 years, 5 months ago by pbos-webrtc Modified:
4 years, 4 months ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, qiang.lu, niklas.enbom, yujie_mao (webrtc), peah-webrtc Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionBuild webrtc_nonparallel_tests under GN.
BUG=webrtc:5949, webrtc:6040
R=danilchap@webrtc.org
TBR=kjellander@webrtc.org
Committed: https://crrev.com/ac968bdab6e0b95072266aa09de2afb15c82ce1b
Cr-Commit-Position: refs/heads/master@{#13299}
Patch Set 1 #
Total comments: 4
Patch Set 2 : add #
Total comments: 2
Patch Set 3 : copy original comments #Patch Set 4 : added native_test_support #Patch Set 5 : move deps #
Total comments: 1
Messages
Total messages: 17 (4 generated)
PTAL, this should correspond to https://chromium.googlesource.com/external/webrtc/+/e01000b9a48eab52b6f5adca1....
add
https://codereview.webrtc.org/2100173002/diff/1/webrtc/BUILD.gn File webrtc/BUILD.gn (right): https://codereview.webrtc.org/2100173002/diff/1/webrtc/BUILD.gn#newcode631 webrtc/BUILD.gn:631: if (is_win) { don't you need special dependency for android case? https://codereview.webrtc.org/2100173002/diff/20001/webrtc/BUILD.gn File webrtc/BUILD.gn (right): https://codereview.webrtc.org/2100173002/diff/20001/webrtc/BUILD.gn#newcode634 webrtc/BUILD.gn:634: # TODO(pbos): Reenable: Can you expand the comment when they should be reenabled (any why they are excluded)
copy original comments
PTAL https://codereview.webrtc.org/2100173002/diff/1/webrtc/BUILD.gn File webrtc/BUILD.gn (right): https://codereview.webrtc.org/2100173002/diff/1/webrtc/BUILD.gn#newcode631 webrtc/BUILD.gn:631: if (is_win) { On 2016/06/27 16:53:13, danilchap wrote: > don't you need special dependency for android case? Haven't seen any BUILD.gn tests include native_test_native_code, so I assume (hope) that that's been done differently. Otherwise it still needs to be fixed for other targets and is possibly better done in a sweep. https://codereview.webrtc.org/2100173002/diff/20001/webrtc/BUILD.gn File webrtc/BUILD.gn (right): https://codereview.webrtc.org/2100173002/diff/20001/webrtc/BUILD.gn#newcode634 webrtc/BUILD.gn:634: # TODO(pbos): Reenable: On 2016/06/27 16:53:13, danilchap wrote: > Can you expand the comment when they should be reenabled (any why they are > excluded) Restored the original comments, I don't know more. :)
lgtm https://codereview.webrtc.org/2100173002/diff/1/webrtc/BUILD.gn File webrtc/BUILD.gn (right): https://codereview.webrtc.org/2100173002/diff/1/webrtc/BUILD.gn#newcode631 webrtc/BUILD.gn:631: if (is_win) { On 2016/06/27 17:05:49, pbos-webrtc wrote: > On 2016/06/27 16:53:13, danilchap wrote: > > don't you need special dependency for android case? > > Haven't seen any BUILD.gn tests include native_test_native_code, so I assume > (hope) that that's been done differently. Otherwise it still needs to be fixed > for other targets and is possibly better done in a sweep. this file in rtc_unittests target does that.
added native_test_support
https://codereview.webrtc.org/2100173002/diff/1/webrtc/BUILD.gn File webrtc/BUILD.gn (right): https://codereview.webrtc.org/2100173002/diff/1/webrtc/BUILD.gn#newcode631 webrtc/BUILD.gn:631: if (is_win) { On 2016/06/27 17:11:39, danilchap wrote: > On 2016/06/27 17:05:49, pbos-webrtc wrote: > > On 2016/06/27 16:53:13, danilchap wrote: > > > don't you need special dependency for android case? > > > > Haven't seen any BUILD.gn tests include native_test_native_code, so I assume > > (hope) that that's been done differently. Otherwise it still needs to be fixed > > for other targets and is possibly better done in a sweep. > > this file in rtc_unittests target does that. Done.
move deps
Description was changed from ========== Build webrtc_nonparallel_tests under GN. BUG=webrtc:5949, webrtc:6040 R=danilchap@webrtc.org ========== to ========== Build webrtc_nonparallel_tests under GN. BUG=webrtc:5949, webrtc:6040 R=danilchap@webrtc.org TBR=kjellander@webrtc.org ==========
pbos@webrtc.org changed reviewers: + kjellander@webrtc.org
Message was sent while issue was closed.
Description was changed from ========== Build webrtc_nonparallel_tests under GN. BUG=webrtc:5949, webrtc:6040 R=danilchap@webrtc.org TBR=kjellander@webrtc.org ========== to ========== Build webrtc_nonparallel_tests under GN. BUG=webrtc:5949, webrtc:6040 R=danilchap@webrtc.org TBR=kjellander@webrtc.org Committed: https://crrev.com/ac968bdab6e0b95072266aa09de2afb15c82ce1b Cr-Commit-Position: refs/heads/master@{#13299} ==========
Message was sent while issue was closed.
Description was changed from ========== Build webrtc_nonparallel_tests under GN. BUG=webrtc:5949, webrtc:6040 R=danilchap@webrtc.org TBR=kjellander@webrtc.org Committed: https://crrev.com/ac968bdab6e0b95072266aa09de2afb15c82ce1b Cr-Commit-Position: refs/heads/master@{#13299} ========== to ========== Build webrtc_nonparallel_tests under GN. BUG=webrtc:5949, webrtc:6040 R=danilchap@webrtc.org TBR=kjellander@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/ac968bdab6e0b95072266aa09... ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/ac968bdab6e0b95072266aa09de2afb15c82ce1b Cr-Commit-Position: refs/heads/master@{#13299}
Message was sent while issue was closed.
Committed patchset #5 (id:80001) manually as ac968bdab6e0b95072266aa09de2afb15c82ce1b (presubmit successful).
Message was sent while issue was closed.
https://codereview.webrtc.org/2100173002/diff/80001/webrtc/BUILD.gn File webrtc/BUILD.gn (right): https://codereview.webrtc.org/2100173002/diff/80001/webrtc/BUILD.gn#newcode643 webrtc/BUILD.gn:643: sources -= [ Please don't use subtracting source listings. Have these to be added if (!is_win) instead. When fixing that, please update webrtc/webrtc_tests.gypi too.
Message was sent while issue was closed.
On 2016/07/06 19:25:53, kjellander_webrtc wrote: > https://codereview.webrtc.org/2100173002/diff/80001/webrtc/BUILD.gn > File webrtc/BUILD.gn (right): > > https://codereview.webrtc.org/2100173002/diff/80001/webrtc/BUILD.gn#newcode643 > webrtc/BUILD.gn:643: sources -= [ > Please don't use subtracting source listings. Have these to be added if > (!is_win) instead. > > When fixing that, please update webrtc/webrtc_tests.gypi too. danilchap: since pbos left the team - could you please address this? Should be a simple CL. |
