Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2888533005:
MockAecDump and integration tests between AecDump and AudioProcessing (Closed)
|
Created:
3 years, 7 months ago by aleloi Modified:
3 years, 6 months ago Reviewers:
peah-webrtc CC:
webrtc-reviews_webrtc.org, AleBzk, peah-webrtc, Andrew MacDonald, aleloi, tterriberry_mozilla.com, audio-team_agora.io, hlundin-webrtc, kwiberg-webrtc, minyue-webrtc, the sun, aluebs-webrtc, bjornv1 Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionMockAecDump and integration tests between AecDump and AudioProcessing
This CL adds a MockAecDump and integration tests that inject the mock
into AudioProcessingImpl. The tests check the call pattern between
AudioProcessingImpl and AecDump. The existing tests ApmTest.* and
DebugDumpTest.* (not touched by this CL) check that the data written
by AecDumpImpl is valid.
The tests check that the protobuf-writing methods for the different
protobuf message types in audio_processing/debug.proto are indeed
called for the different modes of AudioProcessingImpl operation.
BUG=webrtc:7404
Review-Url: https://codereview.webrtc.org/2888533005
Cr-Commit-Position: refs/heads/master@{#18501}
Committed: https://chromium.googlesource.com/external/webrtc/+/20e4a73b9b4660f3c7864d158941637bfca14b4b
Patch Set 1 : Updated test deps #Patch Set 2 : Added explicit convertion for Android compiler #
Total comments: 1
Patch Set 3 : rebase on top of latest changes & removed 'forced' argument. #Patch Set 4 : Fix rebase/merge issues. #Patch Set 5 : Changed call order to match header. #Patch Set 6 : Remove test which is special case of next test. #
Messages
Total messages: 44 (39 generated)
The CQ bit was checked by aleloi@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: Try jobs failed on following builders: ios32_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_ios9_dbg/buil...) ios64_sim_ios10_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios10_dbg/bui...) ios_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/25675) ios_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/24592) mac_asan on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/24696)
The CQ bit was checked by aleloi@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/...
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Description was changed from ========== MockAecDump and integration tests AecDump <--> AudioProcessing. BUG=webrtc:7404 ========== to ========== MockAecDump and integration tests AecDump <--> AudioProcessing. The integration tests check the call pattern between AudioProcessingImpl and AecDump. Existing tests ApmTest.* and DebugDumpTest.* check that the data written by AecDumpImpl is valid. BUG=webrtc:7404 ==========
aleloi@webrtc.org changed reviewers: + peah@webrtc.org
Description was changed from ========== MockAecDump and integration tests AecDump <--> AudioProcessing. The integration tests check the call pattern between AudioProcessingImpl and AecDump. Existing tests ApmTest.* and DebugDumpTest.* check that the data written by AecDumpImpl is valid. BUG=webrtc:7404 ========== to ========== MockAecDump and integration tests AecDump <--> AudioProcessing. The integration tests check the call pattern between AudioProcessingImpl and AecDump. Existing tests ApmTest.* and DebugDumpTest.* check that the data written by AecDumpImpl is valid. BUG=webrtc:7404 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_compile_x64_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x64_dbg...) android_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_rel/builds/24142)
The CQ bit was checked by aleloi@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.
Nice! lgtm https://codereview.webrtc.org/2888533005/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec_dump/mock_aec_dump.h (right): https://codereview.webrtc.org/2888533005/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec_dump/mock_aec_dump.h:26: // To log an input/output pair, call the AddCapture* methods Is this comment needed in a mock? Can't one instead read the comments in the parent class for the usage description?
The CQ bit was checked by aleloi@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: Try jobs failed on following builders: linux_memcheck on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_memcheck/builds/8173)
The CQ bit was checked by aleloi@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/...
Patchset #4 (id:100001) has been deleted
Description was changed from ========== MockAecDump and integration tests AecDump <--> AudioProcessing. The integration tests check the call pattern between AudioProcessingImpl and AecDump. Existing tests ApmTest.* and DebugDumpTest.* check that the data written by AecDumpImpl is valid. BUG=webrtc:7404 ========== to ========== MockAecDump and integration tests between AecDump and AudioProcessing This CL adds a MockAecDump and integration tests that inject the mock into AudioProcessingImpl. The tests check the call pattern between AudioProcessingImpl and AecDump. The existing tests ApmTest.* and DebugDumpTest.* check that the data written by AecDumpImpl is valid. BUG=webrtc:7404 ==========
The CQ bit was checked by aleloi@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from peah@webrtc.org Link to the patchset: https://codereview.webrtc.org/2888533005/#ps140001 (title: "Changed call order to match header.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Description was changed from ========== MockAecDump and integration tests between AecDump and AudioProcessing This CL adds a MockAecDump and integration tests that inject the mock into AudioProcessingImpl. The tests check the call pattern between AudioProcessingImpl and AecDump. The existing tests ApmTest.* and DebugDumpTest.* check that the data written by AecDumpImpl is valid. BUG=webrtc:7404 ========== to ========== MockAecDump and integration tests between AecDump and AudioProcessing This CL adds a MockAecDump and integration tests that inject the mock into AudioProcessingImpl. The tests check the call pattern between AudioProcessingImpl and AecDump. The existing tests ApmTest.* and DebugDumpTest.* check that the data written by AecDumpImpl is valid. The tests check that the protobuf-writing methods for the different protobuf message types in audio_processing/debug.proto are indeed called for the different modes of AudioProcessingImpl operation. BUG=webrtc:7404 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_memcheck on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_memcheck/builds/8197)
The CQ bit was checked by aleloi@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/...
Patchset #6 (id:160001) has been deleted
Description was changed from ========== MockAecDump and integration tests between AecDump and AudioProcessing This CL adds a MockAecDump and integration tests that inject the mock into AudioProcessingImpl. The tests check the call pattern between AudioProcessingImpl and AecDump. The existing tests ApmTest.* and DebugDumpTest.* check that the data written by AecDumpImpl is valid. The tests check that the protobuf-writing methods for the different protobuf message types in audio_processing/debug.proto are indeed called for the different modes of AudioProcessingImpl operation. BUG=webrtc:7404 ========== to ========== MockAecDump and integration tests between AecDump and AudioProcessing This CL adds a MockAecDump and integration tests that inject the mock into AudioProcessingImpl. The tests check the call pattern between AudioProcessingImpl and AecDump. The existing tests ApmTest.* and DebugDumpTest.* (not touched by this CL) check that the data written by AecDumpImpl is valid. The tests check that the protobuf-writing methods for the different protobuf message types in audio_processing/debug.proto are indeed called for the different modes of AudioProcessingImpl operation. BUG=webrtc:7404 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by aleloi@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from peah@webrtc.org Link to the patchset: https://codereview.webrtc.org/2888533005/#ps180001 (title: "Remove test which is special case of next test.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 180001, "attempt_start_ts": 1496934660824140,
"parent_rev": "317005a03baaa10d81f059df358b58e603a021e3", "commit_rev":
"20e4a73b9b4660f3c7864d158941637bfca14b4b"}
Message was sent while issue was closed.
Description was changed from ========== MockAecDump and integration tests between AecDump and AudioProcessing This CL adds a MockAecDump and integration tests that inject the mock into AudioProcessingImpl. The tests check the call pattern between AudioProcessingImpl and AecDump. The existing tests ApmTest.* and DebugDumpTest.* (not touched by this CL) check that the data written by AecDumpImpl is valid. The tests check that the protobuf-writing methods for the different protobuf message types in audio_processing/debug.proto are indeed called for the different modes of AudioProcessingImpl operation. BUG=webrtc:7404 ========== to ========== MockAecDump and integration tests between AecDump and AudioProcessing This CL adds a MockAecDump and integration tests that inject the mock into AudioProcessingImpl. The tests check the call pattern between AudioProcessingImpl and AecDump. The existing tests ApmTest.* and DebugDumpTest.* (not touched by this CL) check that the data written by AecDumpImpl is valid. The tests check that the protobuf-writing methods for the different protobuf message types in audio_processing/debug.proto are indeed called for the different modes of AudioProcessingImpl operation. BUG=webrtc:7404 Review-Url: https://codereview.webrtc.org/2888533005 Cr-Commit-Position: refs/heads/master@{#18501} Committed: https://chromium.googlesource.com/external/webrtc/+/20e4a73b9b4660f3c7864d158... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:180001) as https://chromium.googlesource.com/external/webrtc/+/20e4a73b9b4660f3c7864d158... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
