From: Gabe Black <gabe.black@gmail.com>
Date: Sat, 5 Dec 2020 07:44:59 +0000 (-0800)
Subject: tests: Stop using memcmp in the circlebuf test.
X-Git-Tag: develop-gem5-snapshot~274
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=12602e4dca021d89b045e96bd495559c0a7ad999;p=gem5.git

tests: Stop using memcmp in the circlebuf test.

Comparing arrays with memcmp is fairly easy to do and will correctly
identify when a value is incorrect, but gtest doesn't know what
comparison actually happened and can't print any diagnostic information
to help the person running the test determine what went wrong.

Unfortunately, gtest is also slightly too smart and also not smart
enough to make it easy to compare the contents of sub-arrays with each
other. The thing you're checking the value of *must* be an array with a
well defined size (not a pointer), and the size *must* exactly match the
number of elements it expects to find.

One fairly clean way to get around this problem would be to use the new
std::span type introduced in c++20 which lets you refer to a sub-section
of another container in place, adjusting indexing, sizing, etc as
needed. Unfortunately since we only require up to c++-14 currently we
can't use that type.

Instead, we can create vectors which hold copies of the required data.
This is suboptimal since it means we're copying around data which
doesn't really need to be copied, but it means that the templates in
gtest will get a type they can handle, and the sizes will match like it
expects them to. Since the number of checks/copies is still small, the
overhead should be trivial in practice.

A helper function, subArr, has been added to help keep things fairly
clutter free.

Change-Id: I9f88c583a6a742346b177dba7cae791824b65942
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/38895
Maintainer: Gabe Black <gabe.black@gmail.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Carvalho <odanrc@yahoo.com.br>
---

diff --git a/src/base/circlebuf.test.cc b/src/base/circlebuf.test.cc
index a2babc5f3..6b81b61ff 100644
--- a/src/base/circlebuf.test.cc
+++ b/src/base/circlebuf.test.cc
@@ -35,15 +35,29 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <vector>
+
 #include "base/circlebuf.hh"
 
+using testing::ElementsAreArray;
+
 const char data[] = {
     0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7,
     0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf,
 };
 
+// A better way to implement this would be with std::span, but that is only
+// available starting in c++20.
+template <typename T>
+std::vector<T>
+subArr(T *arr, int size, int offset=0)
+{
+    return std::vector<T>(arr + offset, arr + offset + size);
+}
+
 // Basic non-overflow functionality
 TEST(CircleBufTest, BasicReadWriteNoOverflow)
 {
@@ -54,14 +68,14 @@ TEST(CircleBufTest, BasicReadWriteNoOverflow)
     buf.write(data, 8);
     EXPECT_EQ(buf.size(), 8);
     buf.peek(foo, 8);
-    EXPECT_EQ(memcmp(foo, data, 8), 0);
+    EXPECT_THAT(subArr(foo, 8), ElementsAreArray(data, 8));
 
     // Read 2
     buf.read(foo, 2);
-    EXPECT_EQ(memcmp(foo, data, 2), 0);
+    EXPECT_THAT(subArr(foo, 2), ElementsAreArray(data, 2));
     EXPECT_EQ(buf.size(), 6);
     buf.read(foo, 6);
-    EXPECT_EQ(memcmp(foo, data + 2, 6), 0);
+    EXPECT_THAT(subArr(foo, 6), ElementsAreArray(data + 2, 6));
     EXPECT_EQ(buf.size(), 0);
 }
 
@@ -74,7 +88,7 @@ TEST(CircleBufTest, SingleWriteOverflow)
     buf.write(data, 16);
     EXPECT_EQ(buf.size(), 8);
     buf.peek(foo, 8);
-    EXPECT_EQ(memcmp(data + 8, foo, 8), 0);
+    EXPECT_THAT(subArr(foo, 8), ElementsAreArray(data + 8, 8));
 }
 
 
@@ -89,8 +103,8 @@ TEST(CircleBufTest, MultiWriteOverflow)
     buf.write(data + 8, 6);
     EXPECT_EQ(buf.size(), 8);
     buf.peek(foo, 8);
-    EXPECT_EQ(memcmp(data + 4, foo, 2), 0);
-    EXPECT_EQ(memcmp(data + 8, foo + 2, 6), 0);
+    EXPECT_THAT(subArr(foo, 2), ElementsAreArray(data + 4, 2));
+    EXPECT_THAT(subArr(foo, 6, 2), ElementsAreArray(data + 8, 6));
 }
 
 // Pointer wrap around
@@ -110,9 +124,9 @@ TEST(CircleBufTest, PointerWrapAround)
     // Normalized: _start == 2, _stop = 4
     buf.read(foo + 4, 6);
     EXPECT_EQ(buf.size(), 2);
-    EXPECT_EQ(memcmp(data, foo, 10), 0);
+    EXPECT_THAT(subArr(foo, 10), ElementsAreArray(data, 10));
     // Normalized: _start == 4, _stop = 4
     buf.read(foo + 10, 2);
     EXPECT_EQ(buf.size(), 0);
-    EXPECT_EQ(memcmp(data, foo, 12), 0);
+    EXPECT_THAT(subArr(foo, 12), ElementsAreArray(data, 12));
 }