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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 3 months ago by Bjorn
Modified:
5 years, 3 months 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, 3 months 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, 3 months 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, 3 months 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, 3 months 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, 3 months 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, 3 months 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, 3 months 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, 3 months 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, 3 months 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, 3 months ago (2014-05-16 10:06:41 UTC) #10
Henrik Grunell
lgtm
5 years, 3 months ago (2014-05-16 10:18:41 UTC) #11
Tina
LGTM
5 years, 3 months 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, 3 months ago (2014-05-16 12:08:44 UTC) #13
Bjorn
Shijing, could you please take a look at this?
5 years, 3 months 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, 3 months ago (2014-05-20 09:11:16 UTC) #15
Bjorn
5 years, 3 months 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