Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2123243003:
Added voice_engine_unittests and voe_auto_test targets to GN. (Closed)
|
Created:
4 years, 5 months ago by ossu Modified:
4 years, 5 months ago CC:
webrtc-reviews_webrtc.org, henrika_webrtc, Andrew MacDonald, tterriberry_mozilla.com, audio-team_agora.io, hlundin-webrtc, peah-webrtc, minyue-webrtc, the sun Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAdded voice_engine_unittests and voe_auto_test targets to GN.
I didn't convert the APK tests, since I couldn't figure it out, nor
find any that had already been converted to copy from. I might get to it
in a new CL or even second patch-set.
BUG=webrtc:6039
Committed: https://crrev.com/a16f7a83a1bd764e660c566af8718782961f8264
Cr-Commit-Position: refs/heads/master@{#13410}
Patch Set 1 #Patch Set 2 : Added suppression of error 4373 on Windows. #
Total comments: 6
Patch Set 3 : Capitalization #Messages
Total messages: 29 (12 generated)
ossu@webrtc.org changed reviewers: + phoglund@webrtc.org
PTAL. Also, do you know what to do about .apk tests in GN? They've been made strangely conditional in .gyp but I couldn't find any that had been converted to GN.
The CQ bit was checked by ossu@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I don't know how APK tests might look like in gn unfortunately. I know there is android_library in GN, so you probably want android_test or android_app or something like that. https://codereview.webrtc.org/2123243003/diff/20001/webrtc/voice_engine/BUILD.gn File webrtc/voice_engine/BUILD.gn (right): https://codereview.webrtc.org/2123243003/diff/20001/webrtc/voice_engine/BUILD... webrtc/voice_engine/BUILD.gn:155: if (is_clang) { Where is this from? Can't find anything corresponding in the gyp files. https://codereview.webrtc.org/2123243003/diff/20001/webrtc/voice_engine/BUILD... webrtc/voice_engine/BUILD.gn:227: # some tests are not supported on android yet, exclude these tests. Nit: uppercase S https://codereview.webrtc.org/2123243003/diff/20001/webrtc/voice_engine/BUILD... webrtc/voice_engine/BUILD.gn:243: "/wd4373", # virtual function override. Nit: uppercase v
https://codereview.webrtc.org/2123243003/diff/20001/webrtc/voice_engine/BUILD.gn File webrtc/voice_engine/BUILD.gn (right): https://codereview.webrtc.org/2123243003/diff/20001/webrtc/voice_engine/BUILD... webrtc/voice_engine/BUILD.gn:155: if (is_clang) { On 2016/07/07 12:26:42, phoglund wrote: > Where is this from? Can't find anything corresponding in the gyp files. I took them from the voice_engine target above.. I guess the base flags for GYP builds don't include these checks, whereas they do when building with GN. Without 'em, I get roughly a million warnings about style issues. https://codereview.webrtc.org/2123243003/diff/20001/webrtc/voice_engine/BUILD... webrtc/voice_engine/BUILD.gn:227: # some tests are not supported on android yet, exclude these tests. On 2016/07/07 12:26:42, phoglund wrote: > Nit: uppercase S I stole these comments from gyp, but I'll fix 'em for ya. :) https://codereview.webrtc.org/2123243003/diff/20001/webrtc/voice_engine/BUILD... webrtc/voice_engine/BUILD.gn:243: "/wd4373", # virtual function override. On 2016/07/07 12:26:42, phoglund wrote: > Nit: uppercase v Acknowledged.
Thank you. lgtm
The CQ bit was checked by ossu@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/6738)
ossu@webrtc.org changed reviewers: + tina.legrand@webrtc.org
Hi Tina, I added these two build targets to GN. PTAL.
On 2016/07/07 14:26:18, ossu wrote: > Hi Tina, > > I added these two build targets to GN. PTAL. Great! LGTM
The CQ bit was checked by ossu@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by ossu@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by ossu@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Added voice_engine_unittests and voe_auto_test targets to GN. I didn't convert the APK tests, since I couldn't figure it out, nor find any that had already been converted to copy from. I might get to it in a new CL or even second patch-set. BUG=webrtc:6039 ========== to ========== Added voice_engine_unittests and voe_auto_test targets to GN. I didn't convert the APK tests, since I couldn't figure it out, nor find any that had already been converted to copy from. I might get to it in a new CL or even second patch-set. BUG=webrtc:6039 Committed: https://crrev.com/a16f7a83a1bd764e660c566af8718782961f8264 Cr-Commit-Position: refs/heads/master@{#13410} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/a16f7a83a1bd764e660c566af8718782961f8264 Cr-Commit-Position: refs/heads/master@{#13410} |
