Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1840173005:
Changed tests to be DISABLED on non-supported platforms rather than not to build at all. (Closed)
|
Created:
4 years, 8 months ago by peah-webrtc Modified:
4 years, 8 months ago Reviewers:
hlundin-webrtc CC:
webrtc-reviews_webrtc.org, peah-webrtc, Andrew MacDonald, tterriberry_mozilla.com, audio-team_agora.io, hlundin-webrtc, kwiberg-webrtc, minyue-webrtc, the sun, aluebs-webrtc, bjornv1 Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionChanged tests to be DISABLED on non-supported platforms rather than not to build at all.
BUG=
Committed: https://crrev.com/51fbdd6ada1a3f0aa2647855ad319e66b5ebe2a7
Cr-Commit-Position: refs/heads/master@{#12170}
Patch Set 1 #
Total comments: 3
Patch Set 2 : Added TODO comments referring to the bug that tracks the issue of ARM nonbitexactness #
Messages
Total messages: 19 (8 generated)
peah@webrtc.org changed reviewers: + henrik.lundin@webrtc.org
lgtm with a suggestion. https://codereview.webrtc.org/1840173005/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/echo_cancellation_unittest.cc (right): https://codereview.webrtc.org/1840173005/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/echo_cancellation_unittest.cc:138: #if !(defined(WEBRTC_ARCH_ARM64) || defined(WEBRTC_ARCH_ARM) || \ Avoid the verbose repetition by doing #define DISABLED_PLATFORMS (defined(WEBRTC_ARCH_ARM64) || defined(WEBRTC_ARCH_ARM) || defined(WEBRTC_ANDROID)) And then #if !DISABLED_PLATFORM ... (Please, don't rely on my code to be correct; verify it...)
https://codereview.webrtc.org/1840173005/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/echo_cancellation_unittest.cc (right): https://codereview.webrtc.org/1840173005/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/echo_cancellation_unittest.cc:138: #if !(defined(WEBRTC_ARCH_ARM64) || defined(WEBRTC_ARCH_ARM) || \ On 2016/03/30 08:27:28, hlundin-webrtc wrote: > Avoid the verbose repetition by doing > #define DISABLED_PLATFORMS (defined(WEBRTC_ARCH_ARM64) || > defined(WEBRTC_ARCH_ARM) || defined(WEBRTC_ANDROID)) > > And then > #if !DISABLED_PLATFORM > ... > > (Please, don't rely on my code to be correct; verify it...) Nice! But it seems like the there is a compiler warning attached to using defined in MACRO definitions. I therefore get build errors when trying this :-( So therefore I'll keep this as it is.
https://codereview.webrtc.org/1840173005/diff/1/webrtc/modules/audio_processi... File webrtc/modules/audio_processing/echo_cancellation_unittest.cc (right): https://codereview.webrtc.org/1840173005/diff/1/webrtc/modules/audio_processi... webrtc/modules/audio_processing/echo_cancellation_unittest.cc:138: #if !(defined(WEBRTC_ARCH_ARM64) || defined(WEBRTC_ARCH_ARM) || \ On 2016/03/30 12:31:24, peah-webrtc wrote: > On 2016/03/30 08:27:28, hlundin-webrtc wrote: > > Avoid the verbose repetition by doing > > #define DISABLED_PLATFORMS (defined(WEBRTC_ARCH_ARM64) || > > defined(WEBRTC_ARCH_ARM) || defined(WEBRTC_ANDROID)) > > > > And then > > #if !DISABLED_PLATFORM > > ... > > > > (Please, don't rely on my code to be correct; verify it...) > > Nice! But it seems like the there is a compiler warning attached to using > defined in MACRO definitions. I therefore get build errors when trying this :-( > > So therefore I'll keep this as it is. OK. It was worth a try.
The CQ bit was checked by peah@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from henrik.lundin@webrtc.org Link to the patchset: https://codereview.webrtc.org/1840173005/#ps20001 (title: "Added TODO comments referring to the bug that tracks the issue of ARM nonbitexactness")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1840173005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1840173005/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by peah@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1840173005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1840173005/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by peah@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1840173005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1840173005/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Changed tests to be DISABLED on non-supported platforms rather than not to build at all. BUG= ========== to ========== Changed tests to be DISABLED on non-supported platforms rather than not to build at all. BUG= Committed: https://crrev.com/51fbdd6ada1a3f0aa2647855ad319e66b5ebe2a7 Cr-Commit-Position: refs/heads/master@{#12170} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/51fbdd6ada1a3f0aa2647855ad319e66b5ebe2a7 Cr-Commit-Position: refs/heads/master@{#12170} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
