WebRTC Code Reviews
Help | Chromium Project | Sign in
(1619)

Issue 17509004: VoEVolumeTest: Enabled Linux flaky tests (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 1 month ago by Bjorn
Modified:
5 years ago
CC:
webrtc-reviews_webrtc.org, rillian-moz, tterriberry, leozwang1, henrika_webrtc
Base URL:
https://webrtc.googlecode.com/svn/trunk
Visibility:
Public.

Description

VoEVolumeTest: Enabled Linux flaky tests Fixed error checks only on Linux to be able to turn on flaky tests. The cause of flaky failures is due to late values in pulse audio. Related (deleted) CLs: https://webrtc-codereview.appspot.com/19469007/ https://webrtc-codereview.appspot.com/19469004/ BUG=367 TESTED=trybots, voe_auto_test repeated R=henrikg@webrtc.org, tina.legrand@webrtc.org, xians@webrtc.org Committed: https://code.google.com/p/webrtc/source/detail?r=6195

Patch Set 1 #

Total comments: 11

Patch Set 2 : Addressed review comments #

Patch Set 3 : Changes w.r.t. review comments #

Total comments: 4

Patch Set 4 : Name change and added log info #

Total comments: 2

Patch Set 5 : if-statement following style guide #

Unified diffs Side-by-side diffs Delta from patch set Stats (+79 lines, -25 lines) Patch
M webrtc/voice_engine/test/auto_test/standard/volume_test.cc View 1 2 3 4 3 chunks +79 lines, -25 lines 0 comments Download
Trybot results:

Messages

Total messages: 16 (0 generated)
Bjorn
Tina, Shijing and Henrik G, I added you all as reviewers, since you've been reviewing ...
5 years, 1 month ago (2014-05-15 09:36:25 UTC) #1
Henrik Grunell
https://review.webrtc.org/17509004/diff/1/webrtc/voice_engine/test/auto_test/standard/volume_test.cc File webrtc/voice_engine/test/auto_test/standard/volume_test.cc (right): https://review.webrtc.org/17509004/diff/1/webrtc/voice_engine/test/auto_test/standard/volume_test.cc#newcode40 webrtc/voice_engine/test/auto_test/standard/volume_test.cc:40: success = voe_volume_control_->GetMicVolume(test_volume) == 0; I think all these ...
5 years, 1 month ago (2014-05-15 10:13:56 UTC) #2
Bjorn
Henrik, can you elaborate a little on your first comment? https://webrtc-codereview.appspot.com/17509004/diff/1/webrtc/voice_engine/test/auto_test/standard/volume_test.cc File webrtc/voice_engine/test/auto_test/standard/volume_test.cc (right): https://webrtc-codereview.appspot.com/17509004/diff/1/webrtc/voice_engine/test/auto_test/standard/volume_test.cc#newcode40 ...
5 years, 1 month ago (2014-05-15 10:24:55 UTC) #3
Tina
Two comments from me. https://webrtc-codereview.appspot.com/17509004/diff/1/webrtc/voice_engine/test/auto_test/standard/volume_test.cc File webrtc/voice_engine/test/auto_test/standard/volume_test.cc (right): https://webrtc-codereview.appspot.com/17509004/diff/1/webrtc/voice_engine/test/auto_test/standard/volume_test.cc#newcode183 webrtc/voice_engine/test/auto_test/standard/volume_test.cc:183: TEST_LOG("Failed to fetch current microphone ...
5 years, 1 month ago (2014-05-15 10:49:29 UTC) #4
Henrik Grunell
LGTM after addressing mine and Tina's comments. Make sure you get Tina's and Shijing's lgtm ...
5 years, 1 month ago (2014-05-15 12:06:10 UTC) #5
Bjorn
https://webrtc-codereview.appspot.com/17509004/diff/1/webrtc/voice_engine/test/auto_test/standard/volume_test.cc File webrtc/voice_engine/test/auto_test/standard/volume_test.cc (right): https://webrtc-codereview.appspot.com/17509004/diff/1/webrtc/voice_engine/test/auto_test/standard/volume_test.cc#newcode40 webrtc/voice_engine/test/auto_test/standard/volume_test.cc:40: success = voe_volume_control_->GetMicVolume(test_volume) == 0; On 2014/05/15 12:06:11, Henrik ...
5 years, 1 month ago (2014-05-16 06:50:36 UTC) #6
Henrik Grunell
Some more minor comments. https://review.webrtc.org/17509004/diff/40001/webrtc/voice_engine/test/auto_test/standard/volume_test.cc File webrtc/voice_engine/test/auto_test/standard/volume_test.cc (right): https://review.webrtc.org/17509004/diff/40001/webrtc/voice_engine/test/auto_test/standard/volume_test.cc#newcode47 webrtc/voice_engine/test/auto_test/standard/volume_test.cc:47: EXPECT_EQ(1000u, test_volume); Would it be ...
5 years, 1 month ago (2014-05-16 07:42:19 UTC) #7
Bjorn
Yet another patch. I have one question though. After running these tests over and over ...
5 years, 1 month ago (2014-05-16 08:15:37 UTC) #8
Henrik Grunell
https://review.webrtc.org/17509004/diff/40002/webrtc/voice_engine/test/auto_test/standard/volume_test.cc File webrtc/voice_engine/test/auto_test/standard/volume_test.cc (right): https://review.webrtc.org/17509004/diff/40002/webrtc/voice_engine/test/auto_test/standard/volume_test.cc#newcode44 webrtc/voice_engine/test/auto_test/standard/volume_test.cc:44: if (success) Use {} for if since used for ...
5 years, 1 month ago (2014-05-16 09:23:37 UTC) #9
Bjorn
https://webrtc-codereview.appspot.com/17509004/diff/40002/webrtc/voice_engine/test/auto_test/standard/volume_test.cc File webrtc/voice_engine/test/auto_test/standard/volume_test.cc (right): https://webrtc-codereview.appspot.com/17509004/diff/40002/webrtc/voice_engine/test/auto_test/standard/volume_test.cc#newcode44 webrtc/voice_engine/test/auto_test/standard/volume_test.cc:44: if (success) On 2014/05/16 09:23:37, Henrik Grunell wrote: > ...
5 years, 1 month ago (2014-05-16 10:06:41 UTC) #10
Henrik Grunell
lgtm
5 years, 1 month ago (2014-05-16 10:18:41 UTC) #11
Tina
LGTM
5 years, 1 month ago (2014-05-16 12:05:16 UTC) #12
Bjorn
Shijing, a friendly ping. It would be nice to commit this today, unless you object ...
5 years, 1 month ago (2014-05-16 12:08:44 UTC) #13
Bjorn
Shijing, could you please take a look at this?
5 years ago (2014-05-19 06:31:21 UTC) #14
no longer working on webrtc
On 2014/05/19 06:31:21, Bjorn wrote: > Shijing, could you please take a look at this? ...
5 years ago (2014-05-20 09:11:16 UTC) #15
Bjorn
5 years ago (2014-05-20 10:43:53 UTC) #16
Message was sent while issue was closed.
Committed patchset #5 manually as r6195 (presubmit successful).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 245c2c2-tainted