Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2760623002:
Probing EndToEndTests. (Closed)
|
Created:
3 years, 9 months ago by philipel Modified:
3 years, 9 months ago Reviewers:
stefan-webrtc CC:
webrtc-reviews_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, stefan-webrtc, tterriberry_mozilla.com, mflodman Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionProbing EndToEndTests.
Add tests for inital probing and mid-call probing by reconfiguring max bitrate.
BUG=webrtc:6997
Review-Url: https://codereview.webrtc.org/2760623002
Cr-Commit-Position: refs/heads/master@{#17316}
Committed: https://chromium.googlesource.com/external/webrtc/+/e828c9690a3df28ee87ce2f64757db319616b3a2
Patch Set 1 #
Total comments: 6
Patch Set 2 : Feedback. #Patch Set 3 : Disabled for memcheck bot. #Patch Set 4 : Make tests less flaky. #Patch Set 5 : Correctly disabled tests on memcheck bot. #
Messages
Total messages: 34 (20 generated)
philipel@webrtc.org changed reviewers: + stefan@webrtc.org
https://codereview.webrtc.org/2760623002/diff/1/webrtc/video/end_to_end_tests.cc File webrtc/video/end_to_end_tests.cc (right): https://codereview.webrtc.org/2760623002/diff/1/webrtc/video/end_to_end_tests... webrtc/video/end_to_end_tests.cc:2220: int start_bitrate_bps_; const https://codereview.webrtc.org/2760623002/diff/1/webrtc/video/end_to_end_tests... webrtc/video/end_to_end_tests.cc:2222: int state_ = 0; Use an enum for readability. It also seems like this member belongs in TriggerMidCallProbingTest. https://codereview.webrtc.org/2760623002/diff/1/webrtc/video/end_to_end_tests... webrtc/video/end_to_end_tests.cc:2233: if (test_end_ms - clock_->TimeInMilliseconds() <= 0) { I would have written this: if (clock_->TimeInMilliseconds() - test_start_ms > kTestTimeoutMs) { easier to read that way IMO
https://codereview.webrtc.org/2760623002/diff/1/webrtc/video/end_to_end_tests.cc File webrtc/video/end_to_end_tests.cc (right): https://codereview.webrtc.org/2760623002/diff/1/webrtc/video/end_to_end_tests... webrtc/video/end_to_end_tests.cc:2220: int start_bitrate_bps_; On 2017/03/17 13:14:26, stefan-webrtc wrote: > const Done. https://codereview.webrtc.org/2760623002/diff/1/webrtc/video/end_to_end_tests... webrtc/video/end_to_end_tests.cc:2222: int state_ = 0; On 2017/03/17 13:14:26, stefan-webrtc wrote: > Use an enum for readability. It also seems like this member belongs in > TriggerMidCallProbingTest. I don't really think an enum adds anything in terms of readability when you have such linear transitions, it just adds a bit more boilerplate. I put |state_| in the base class in case we wanted to write more ProbingTests. https://codereview.webrtc.org/2760623002/diff/1/webrtc/video/end_to_end_tests... webrtc/video/end_to_end_tests.cc:2233: if (test_end_ms - clock_->TimeInMilliseconds() <= 0) { On 2017/03/17 13:14:26, stefan-webrtc wrote: > I would have written this: > if (clock_->TimeInMilliseconds() - test_start_ms > kTestTimeoutMs) { > > easier to read that way IMO Agree, fixed.
lgtm
The CQ bit was checked by philipel@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_clang_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_clang_dbg/build...) android_compile_mips_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_mips_db...) android_compile_x64_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x64_dbg...) android_compile_x86_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x86_dbg...) android_compile_x86_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x86_rel...) android_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_rel/builds/22399) linux_asan on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_asan/builds/23189) linux_more_configs on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_more_configs/buil...) linux_msan on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_msan/builds/18874) linux_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_rel/builds/24317) linux_ubsan_vptr on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan_vptr/builds...) win_baremetal on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_baremetal/builds/19790)
The CQ bit was checked by philipel@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: linux_memcheck on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_memcheck/builds/6431)
The CQ bit was checked by philipel@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org Link to the patchset: https://codereview.webrtc.org/2760623002/#ps40001 (title: "Disabled for memcheck bot.")
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_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_arm64_rel/build...)
The CQ bit was checked by philipel@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org Link to the patchset: https://codereview.webrtc.org/2760623002/#ps60001 (title: "Make tests less flaky.")
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: linux_memcheck on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_memcheck/builds/6468)
The CQ bit was checked by philipel@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.
The CQ bit was checked by philipel@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org Link to the patchset: https://codereview.webrtc.org/2760623002/#ps80001 (title: "Correctly disabled tests on memcheck bot.")
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": 80001, "attempt_start_ts": 1490091727138850,
"parent_rev": "0a2391f74c001357c661d4fe39671cc82175d5c4", "commit_rev":
"e828c9690a3df28ee87ce2f64757db319616b3a2"}
Message was sent while issue was closed.
Description was changed from ========== Probing EndToEndTests. Add tests for inital probing and mid-call probing by reconfiguring max bitrate. BUG=none ========== to ========== Probing EndToEndTests. Add tests for inital probing and mid-call probing by reconfiguring max bitrate. BUG=none Review-Url: https://codereview.webrtc.org/2760623002 Cr-Commit-Position: refs/heads/master@{#17316} Committed: https://chromium.googlesource.com/external/webrtc/+/e828c9690a3df28ee87ce2f64... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/external/webrtc/+/e828c9690a3df28ee87ce2f64...
Message was sent while issue was closed.
Description was changed from ========== Probing EndToEndTests. Add tests for inital probing and mid-call probing by reconfiguring max bitrate. BUG=none Review-Url: https://codereview.webrtc.org/2760623002 Cr-Commit-Position: refs/heads/master@{#17316} Committed: https://chromium.googlesource.com/external/webrtc/+/e828c9690a3df28ee87ce2f64... ========== to ========== Probing EndToEndTests. Add tests for inital probing and mid-call probing by reconfiguring max bitrate. BUG=webrtc:6997 Review-Url: https://codereview.webrtc.org/2760623002 Cr-Commit-Position: refs/heads/master@{#17316} Committed: https://chromium.googlesource.com/external/webrtc/+/e828c9690a3df28ee87ce2f64... ========== |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
