Bitexactness test for the noise suppressor

webrtc/modules/audio_processing/test/bitexactness_tools.cc

+/*
+ *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include <math.h>
+#include <string>
+
+#include "webrtc/base/array_view.h"
+#include "webrtc/modules/audio_processing/test/bitexactness_tools.h"

[hlundin-webrtc, 2016/03/16 12:44:28]

bitexactness_tools.h should be included first of all.
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

[peah-webrtc, 2016/03/17 13:15:01]

Done.

+
+namespace webrtc {
+namespace test {
+
+::testing::AssertionResult AssertFloatsNotEqual(const char* m_expr,
+                                                const char* n_expr,
+                                                const float& output,
+                                                const float& reference) {
+  // Compare the output in the reference in a soft manner.
+  float threshold = 1.0f / 32768.0f;

[hlundin-webrtc, 2016/03/16 12:44:28]

const float kThreshold

[peah-webrtc, 2016/03/17 13:15:01]

Done.

+  bool equal = (fabs(output - reference) <= threshold);
+
+  // If the values are deemed not to be similar, return a report of the
+  // difference.
+  if (!equal) {
+    // Lambda function that produces a formatted string with the values.

[hlundin-webrtc, 2016/03/16 12:44:29]

Is this a lambda?

[peah-webrtc, 2016/03/17 13:15:01]

No, the comment in the code is not correct. This function has now been removed.

+    return ::testing::AssertionFailure()
+           << "Actual: " << std::to_string(output) + "f" << std::endl
+           << "Expected: " << std::to_string(reference) + "f" << std::endl;
+  }
+  return ::testing::AssertionSuccess();
+}
+
+::testing::AssertionResult AssertVectorsNotEqual(
+    const char* m_expr,

[hlundin-webrtc, 2016/03/16 12:44:28]

It seems to me that you are not using m_expr and n_expr, so I believe you can
use the slightly simpler approach of a "function that returns an
AssertionResult"
(https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuid...)
instead of a predicate-formatter
(https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuid...).
This means that the predicate can be invoked using simple
{ASSERT|EXPECT}_{TRUE|FALSE} instead of the clunkier
{ASSERT|EXPECT}_PRED_FORMATn.

[peah-webrtc, 2016/03/17 13:15:01]

Awesome!!! Splendid suggestion!

Done.

+    const char* n_expr,
+    const rtc::ArrayView<const float>& output,
+    const rtc::ArrayView<const float>& reference) {
+  // Compare the output in the reference in a soft manner.
+  float threshold = 1.0f / 32768.0f;

[hlundin-webrtc, 2016/03/16 12:44:28]

const float kThreshold

[peah-webrtc, 2016/03/17 13:15:01]

Good point. This code has now been refactored, so it is a bit different now.
Done.

+  bool equal = true;
+  for (size_t k = 0; k < reference.size(); ++k) {

[hlundin-webrtc, 2016/03/16 12:44:29]

What if output is shorter than reference? Also, it seems that if the output is
longer than reference, they can still be considered equal. If this is the
intention, please, document this in the header file.

[peah-webrtc, 2016/03/17 13:15:01]

Good find!

Done.

+    if (fabs(output[k] - reference[k]) > threshold) {
+      equal = false;
+      break;
+    }
+  }
+

[hlundin-webrtc, 2016/03/16 12:44:28]

You may want to consider early return here in the case of success.
if (equal)
  return ::testing::AssertionSuccess();

This saves you one level of indentation on the rest of the code.

[peah-webrtc, 2016/03/17 13:15:01]

Done.

+  // If the vectors are deemed not to be similar, return a report of the
+  // difference.
+  if (!equal) {
+    // Lambda function that produces a formatted string with the data in the
+    // vector.
+    auto print_vector_in_c_format = [](const rtc::ArrayView<const float>& v,
+                                       size_t num_values_to_print) {
+      std::string s = "{ ";
+      for (size_t k = 0; k < num_values_to_print; ++k) {

[hlundin-webrtc, 2016/03/16 12:44:28]

What if num_values_to_print is larger than the length of v?

[peah-webrtc, 2016/03/17 13:15:01]

Done.

+        s += std::to_string(v[k]) + "f";
+        s += (k < (num_values_to_print - 1)) ? ", " : "";
+      }
+      return s + " }";
+    };
+
+    return ::testing::AssertionFailure()
+           << "Actual: " << print_vector_in_c_format(output, reference.size())
+           << std::endl
+           << "Expected: "
+           << print_vector_in_c_format(reference, reference.size())
+           << std::endl;
+  }
+  return ::testing::AssertionSuccess();
+}
+
+}  // namespace test
+}  // namespace webrtc