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

Issue 437002: Refactoring audio_device_test_api for gtest and execution on all platforms. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 8 months ago by kjellander
Modified:
2 years, 6 months ago
CC:
tterriberry, rillian-moz, leozwang1, henrika_webrtc
Base URL:
https://webrtc.googlecode.com/svn/trunk
Visibility:
Public.

Description

Refactoring audio_device_test_api for gtest and execution on all platforms. All the code that was previously in one single function is now broken up into 44 gtest tests. The creation of the Audio Device is done once (SetUpTestCase) and the audio device is initialized before each test (SetUp) and terminated after each test (TearDown). Doing this, the things that execute are basically the same since the test was structured as different sections separated by these calls: TEST(audioDevice->Terminate() == 0); TEST(audioDevice->Init() == 0); I also cleaned up some unused helper functions and removed old test macros when replacing them by gtest macros. The parts that are failing for Mac and Windows are excluded using #ifdef. Separate issues are filed for this code to be fixed. Formatting is updated to follow the WebRTC style guide. BUG=None. TEST=audio_device_test_api in Debug+Release on Linux, Mac and Windows. Test run audio_device_test_func on Linux.

Patch Set 1 : . #

Total comments: 28

Patch Set 2 : Code style fixes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2066 lines, -2399 lines) Patch
M src/modules/audio_device/main/source/audio_device.gypi View 3 chunks +4 lines, -1 line 0 comments Download
M src/modules/audio_device/main/test/audio_device_test_api.cc View 1 2 chunks +1671 lines, -1954 lines 0 comments Download
M src/modules/audio_device/main/test/audio_device_test_defines.h View 5 chunks +8 lines, -47 lines 0 comments Download
M src/modules/audio_device/main/test/audio_device_test_func.cc View 1 chunk +0 lines, -2 lines 0 comments Download
M src/modules/audio_device/main/test/func_test_manager.cc View 1 107 chunks +383 lines, -395 lines 0 comments Download
Trybot results:

Messages

Total messages: 7 (0 generated)
kjellander
This turned out to be a massive refactoring. Fortunately I have a Python script for ...
7 years, 8 months ago (2012-03-06 17:33:15 UTC) #1
henrika_webrtc
Love it. 10 times better than before. Thanks. I just added some nits since the ...
7 years, 8 months ago (2012-03-07 08:44:37 UTC) #2
no longer working on webrtc
Good work, thanks. On 2012/03/06 17:33:15, Henrik K wrote: > This turned out to be ...
7 years, 8 months ago (2012-03-08 14:31:14 UTC) #3
kjellander
Submitted, pending add on Buildbots. http://webrtc-codereview.appspot.com/437002/diff/7/src/modules/audio_device/main/test/audio_device_test_api.cc File src/modules/audio_device/main/test/audio_device_test_api.cc (right): http://webrtc-codereview.appspot.com/437002/diff/7/src/modules/audio_device/main/test/audio_device_test_api.cc#newcode50 src/modules/audio_device/main/test/audio_device_test_api.cc:50: : error_(kRecordingError), On 2012/03/07 ...
7 years, 8 months ago (2012-03-09 08:14:30 UTC) #4
kjellander
On 2012/03/08 14:31:14, xians1 wrote: > Good work, thanks. > > On 2012/03/06 17:33:15, Henrik ...
7 years, 8 months ago (2012-03-09 08:20:28 UTC) #5
nuttie31
On 2012/03/09 08:20:28, kjellander wrote: > On 2012/03/08 14:31:14, xians1 wrote: > > Good work, ...
3 years, 10 months ago (2016-01-13 11:26:03 UTC) #6
nuttie31
3 years, 10 months ago (2016-01-13 11:27:11 UTC) #7
Message was sent while issue was closed.
On 2016/01/13 11:26:03, nuttie31 wrote:
> On 2012/03/09 08:20:28, kjellander wrote:
> > On 2012/03/08 14:31:14, xians1 wrote:
> > > Good work, thanks.
> > > 
> > > On 2012/03/06 17:33:15, Henrik K wrote:
> > > > This turned out to be a massive refactoring. Fortunately I have a 
> > > 
> > > Exactly, it is hard to refactor the big tests like these two.
> > > How do you think breaking down the tests into smaller ones? I am not
asking
> > you
> > > to do it, but just wonder if it is an easy work to do.
> > > We may be able to take care more details if we are starting breaking it.
> > > 
> > > Thought I haven't looked into the details, lgtm.
> > 
> > I think this is a good way to start at least. Extracting a test fixture with
> > SetUp, TearDown (and possible SetUpTestCase and TearDownTestCase as in this
> > case) followed by a list of not-too-large test methods is a good start. Of
> > course, it would be good if the tests are only like max 50 lines instead of
> > hundreds.
> > 
> > Splitting the implementation into separate files for platform specific code
is
> > always good, but I'm not sure it would make the testing smoother. Simply
> > starting out small when writing the tests from scratch and trying to keep
code
> > duplication minimized, is probably the only way to keep them slim and
> readable.
Sign in to reply to this message.

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