Adding GetAudioInputDeviceNames to AudioManager, this function is supposed to do device enumerati...

media/audio/audio_input_device_unittest.cc

Index: media/audio/audio_input_device_unittest.cc
===================================================================
--- media/audio/audio_input_device_unittest.cc	(revision 0)
+++ media/audio/audio_input_device_unittest.cc	(revision 0)
@@ -0,0 +1,31 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "media/audio/audio_manager.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+TEST(AudioInputDeviceTest, EnumerateDevices) {

[awong, 2011/05/23 17:36:15]

Add a 1-line (keep it very short) comment explaining the assertion you are
trying to validate with the unittest.  Side-effect is that if you can't name it
in one line, this test is probably too unfocused. ;)

[xians, 2011/05/23 20:15:59]

Done.

+  AudioInputDeviceNames device_names;
+  ASSERT_TRUE(AudioManager::GetAudioManager() != NULL);
+  AudioManager::GetAudioManager()->GetAudioInputDeviceNames(&device_names);

[awong, 2011/05/23 17:36:15]

This looks like it's going to actually query ALSA during a unittest.

That's generally a bad idea since we don't know if the buildbots will have audio
devices.

[xians, 2011/05/23 20:15:59]

True. I changed the test to handle the case when there is no soundcard in the
machine.

[awong, 2011/05/23 21:17:58]

Would it be possible to mock out the Alsa responses like we do in the other
tests?

+  ASSERT_FALSE(device_names.empty());
+  int i(0);

[awong, 2011/05/23 17:36:15]

Most code in Chromium doesn't use the constructor syntax to initialize
primitives.  Doing "int i = 0;" is preferred.

That being said, in this code, I suggest just keeping it out of the loop, and
removeing i all together.  So

AIDN::const_iterator it = device_names.begin();
EXPECT_EQ("Default", it->device_name);
EXPECT_EQ("Default_0", it->unique_id);
++it;

for (; it != device_names.end(); ++it) {
  EXPECT_NE("", it->device_name);
  EXPECT_NE("", it->unique_id);
}

[xians, 2011/05/23 20:15:59]

Done.

+  for (AudioInputDeviceNames::iterator it = device_names.begin();
+      it != device_names.end();
+      ++i, ++it) {
+    if (i == 0) {
+      EXPECT_EQ("Default", it->device_name);
+      EXPECT_EQ("Default_0", it->unique_id);
+    } else {
+      EXPECT_NE("", it->device_name);
+      EXPECT_NE("", it->unique_id);
+    }
+    DLOG(INFO) << i << " Device Name: " << it->device_name;

[awong, 2011/05/23 17:36:15]

Don't unconditionally dump output inside a unittest.  Gets really annoying in
the log files.

[xians, 2011/05/23 20:15:59]

Done.

+    DLOG(INFO) << "  Device Id: " << it->unique_id;
+  }
+
+  device_names.clear();

[awong, 2011/05/23 17:36:15]

device_names is a temporary...no reason to expicitly call clear() here.  Remove?

[xians, 2011/05/23 20:15:59]

Done.

+}
Property changes on: media/audio/audio_input_device_unittest.cc
___________________________________________________________________
Added: svn:eol-style
   + LF