Gabe Black has submitted this change. (
https://gem5-review.googlesource.com/c/public/gem5/+/38895 )
Change subject: tests: Stop using memcmp in the circlebuf test.
......................................................................
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.bl...@gmail.com>
Tested-by: kokoro <noreply+kok...@google.com>
Reviewed-by: Daniel Carvalho <oda...@yahoo.com.br>
---
M src/base/circlebuf.test.cc
1 file changed, 22 insertions(+), 8 deletions(-)
Approvals:
Daniel Carvalho: Looks good to me, approved
Gabe Black: Looks good to me, approved
kokoro: Regressions pass
diff --git a/src/base/circlebuf.test.cc b/src/base/circlebuf.test.cc
index a2babc5..6b81b61 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 @@
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 @@
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 @@
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 @@
// 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));
}
--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/38895
To unsubscribe, or for help writing mail filters, visit
https://gem5-review.googlesource.com/settings
Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I9f88c583a6a742346b177dba7cae791824b65942
Gerrit-Change-Number: 38895
Gerrit-PatchSet: 4
Gerrit-Owner: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: Bobby R. Bruce <bbr...@ucdavis.edu>
Gerrit-Reviewer: Daniel Carvalho <oda...@yahoo.com.br>
Gerrit-Reviewer: Gabe Black <gabe.bl...@gmail.com>
Gerrit-Reviewer: kokoro <noreply+kok...@google.com>
Gerrit-MessageType: merged
_______________________________________________
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
