Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unit_tests: gtest utility ASSERT_EQ_MAP for easily testing key-value map #3245

Merged
merged 2 commits into from
Feb 20, 2018

Conversation

stoffu
Copy link
Contributor

@stoffu stoffu commented Feb 9, 2018

When one wants to test for a map m whether it contains a particular key-value pair or not, doing something like ASSERT_EQ(value, m.find(key)->second) will crash the whole test if the key isn't found in the map. Although currently there's no such instance in the test thanks to the test data being carefully prepared, this utility can be useful for avoiding the crash when writing new tests.

@stoffu stoffu force-pushed the assert-eq-map branch 2 times, most recently from 31e19ab to 3eb4335 Compare February 9, 2018 13:45
@iDunk5400
Copy link
Contributor

Are you going to PR the header change to Google? Because this forces everyone to uninstall libgtest and use the provided one to be able to compile unit_tests.

@iDunk5400
Copy link
Contributor

Trying to build tests in MSYS2, after uninstalling mingw-w64-x86_64-gtest, produces the following output:

[ 49%] Creating directories for 'googletest'
[ 49%] No download step for 'googletest'
[ 50%] No patch step for 'googletest'
[ 50%] No update step for 'googletest'
[ 50%] Performing configure step for 'googletest'
-- The CXX compiler identification is GNU 7.3.0
-- The C compiler identification is GNU 7.3.0
-- Check for working CXX compiler: C:/msys64/mingw64/bin/g++.exe
-- Check for working CXX compiler: C:/msys64/mingw64/bin/g++.exe -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
[ 50%] Building CXX object src/rpc/CMakeFiles/obj_rpc.dir/instanciations.cpp.obj
-- Detecting CXX compile features - done
-- Check for working C compiler: C:/msys64/mingw64/bin/gcc.exe
-- Check for working C compiler: C:/msys64/mingw64/bin/gcc.exe -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Found PythonInterp: C:/msys64/mingw64/bin/python.exe (found version "2.7.14")
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Looking for pthread_create
-- Looking for pthread_create - found
-- Found Threads: TRUE
-- Configuring done
-- Generating done
-- Build files have been written to: C:/msys64/home/monero/build/release/tests/gtest
[ 50%] Performing build step for 'googletest'
make[4]: Entering directory '/C/msys64/home/monero/build/release/tests/gtest'
make[5]: Entering directory '/C/msys64/home/monero/build/release/tests/gtest'
make[6]: Entering directory '/C/msys64/home/monero/build/release/tests/gtest'
Scanning dependencies of target gtest
make[6]: Leaving directory '/C/msys64/home/monero/build/release/tests/gtest'
make[6]: Entering directory '/C/msys64/home/monero/build/release/tests/gtest'
[ 25%] Building CXX object CMakeFiles/gtest.dir/src/gtest-all.cc.obj
In file included from C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-internal.h:40:0,
                 from C:/msys64/home/monero/tests/gtest/include/gtest/gtest.h:58,
                 from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:39:
C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-port.h:1782:3: error: 'AutoHandle' does not name a type; did you mean 'mutable'?
   AutoHandle thread_;
   ^~~~~~~~~~
   mutable
In file included from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:43:0:
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:637:3: error: 'AutoHandle' does not name a type; did you mean 'LocalHandle'?
   AutoHandle write_handle_;
   ^~~~~~~~~~
   LocalHandle
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:639:3: error: 'AutoHandle' does not name a type; did you mean 'LocalHandle'?
   AutoHandle child_handle_;
   ^~~~~~~~~~
   LocalHandle
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:644:3: error: 'AutoHandle' does not name a type; did you mean 'LocalHandle'?
   AutoHandle event_handle_;
   ^~~~~~~~~~
   LocalHandle
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc: In member function 'virtual int testing::internal::WindowsDeathTest::Wait()':
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:656:36: error: 'child_handle_' was not declared in this scope
   const HANDLE wait_handles[2] = { child_handle_.Get(), event_handle_.Get() };
                                    ^~~~~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:656:36: note: suggested alternative: 'wait_handles'
   const HANDLE wait_handles[2] = { child_handle_.Get(), event_handle_.Get() };
                                    ^~~~~~~~~~~~~
                                    wait_handles
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:656:57: error: 'event_handle_' was not declared in this scope
   const HANDLE wait_handles[2] = { child_handle_.Get(), event_handle_.Get() };
                                                         ^~~~~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:656:57: note: suggested alternative: 'wait_handles'
   const HANDLE wait_handles[2] = { child_handle_.Get(), event_handle_.Get() };
                                                         ^~~~~~~~~~~~~
                                                         wait_handles
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:670:3: error: 'write_handle_' was not declared in this scope
   write_handle_.Reset();
   ^~~~~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:670:3: note: suggested alternative: 'wait_handles'
   write_handle_.Reset();
   ^~~~~~~~~~~~~
   wait_handles
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc: In member function 'virtual testing::internal::DeathTest::TestRole testing::internal::WindowsDeathTest::AssumeRole()':
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:720:3: error: 'write_handle_' was not declared in this scope
   write_handle_.Reset(write_handle);
   ^~~~~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:720:3: note: suggested alternative: 'write_handle'
   write_handle_.Reset(write_handle);
   ^~~~~~~~~~~~~
   write_handle
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:721:3: error: 'event_handle_' was not declared in this scope
   event_handle_.Reset(::CreateEvent(
   ^~~~~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:721:3: note: suggested alternative: 'read_handle'
   event_handle_.Reset(::CreateEvent(
   ^~~~~~~~~~~~~
   read_handle
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:777:3: error: 'child_handle_' was not declared in this scope
   child_handle_.Reset(process_info.hProcess);
   ^~~~~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:777:3: note: suggested alternative: 'write_handle'
   child_handle_.Reset(process_info.hProcess);
   ^~~~~~~~~~~~~
   write_handle
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc: In function 'int testing::internal::GetStatusFileDescriptor(unsigned int, size_t, size_t)':
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:1230:3: error: 'AutoHandle' was not declared in this scope
   AutoHandle parent_process_handle(::OpenProcess(PROCESS_DUP_HANDLE,
   ^~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:1230:3: note: suggested alternative: 'LocalHandle'
   AutoHandle parent_process_handle(::OpenProcess(PROCESS_DUP_HANDLE,
   ^~~~~~~~~~
   LocalHandle
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:1233:7: error: 'parent_process_handle' was not declared in this scope
   if (parent_process_handle.Get() == INVALID_HANDLE_VALUE) {
       ^~~~~~~~~~~~~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:1233:7: note: suggested alternative: 'parent_process_id'
   if (parent_process_handle.Get() == INVALID_HANDLE_VALUE) {
       ^~~~~~~~~~~~~~~~~~~~~
       parent_process_id
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:1249:26: error: 'parent_process_handle' was not declared in this scope
   if (!::DuplicateHandle(parent_process_handle.Get(), write_handle,
                          ^~~~~~~~~~~~~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:1249:26: note: suggested alternative: 'parent_process_id'
   if (!::DuplicateHandle(parent_process_handle.Get(), write_handle,
                          ^~~~~~~~~~~~~~~~~~~~~
                          parent_process_id
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:1264:26: error: 'parent_process_handle' was not declared in this scope
   if (!::DuplicateHandle(parent_process_handle.Get(), event_handle,
                          ^~~~~~~~~~~~~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-death-test.cc:1264:26: note: suggested alternative: 'parent_process_id'
   if (!::DuplicateHandle(parent_process_handle.Get(), event_handle,
                          ^~~~~~~~~~~~~~~~~~~~~
                          parent_process_id
In file included from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:45:0:
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc: In function 'void testing::internal::SleepMilliseconds(int)':
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:179:6: error: redefinition of 'void testing::internal::SleepMilliseconds(int)'
 void SleepMilliseconds(int n) {
      ^~~~~~~~~~~~~~~~~
In file included from C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-internal.h:40:0,
                 from C:/msys64/home/monero/tests/gtest/include/gtest/gtest.h:58,
                 from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:39:
C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-port.h:1448:13: note: 'void testing::internal::SleepMilliseconds(int)' previously defined here
 inline void SleepMilliseconds(int n) {
             ^~~~~~~~~~~~~~~~~
In file included from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:45:0:
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc: At global scope:
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:183:1: error: 'AutoHandle' does not name a type; did you mean 'LocalHandle'?
 AutoHandle::AutoHandle()
 ^~~~~~~~~~
 LocalHandle
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:186:1: error: 'AutoHandle' does not name a type; did you mean 'LocalHandle'?
 AutoHandle::AutoHandle(Handle handle)
 ^~~~~~~~~~
 LocalHandle
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:189:1: error: 'AutoHandle' does not name a type; did you mean 'LocalHandle'?
 AutoHandle::~AutoHandle() {
 ^~~~~~~~~~
 LocalHandle
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:193:1: error: 'AutoHandle' does not name a type; did you mean 'LocalHandle'?
 AutoHandle::Handle AutoHandle::Get() const {
 ^~~~~~~~~~
 LocalHandle
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:197:6: error: 'AutoHandle' has not been declared
 void AutoHandle::Reset() {
      ^~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc: In function 'void testing::internal::Reset()':
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:198:29: error: too many arguments to function 'void testing::internal::Reset()'
   Reset(INVALID_HANDLE_VALUE);
                             ^
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:197:6: note: declared here
 void AutoHandle::Reset() {
      ^~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc: At global scope:
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:201:6: error: 'AutoHandle' has not been declared
 void AutoHandle::Reset(HANDLE handle) {
      ^~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc: In function 'void testing::internal::Reset(HANDLE)':
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:203:7: error: 'handle_' was not declared in this scope
   if (handle_ != handle) {
       ^~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:203:7: note: suggested alternative: 'handle'
   if (handle_ != handle) {
       ^~~~~~~
       handle
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:204:9: error: 'IsCloseable' was not declared in this scope
     if (IsCloseable()) {
         ^~~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:204:9: note: suggested alternative: 'CloseHandle'
     if (IsCloseable()) {
         ^~~~~~~~~~~
         CloseHandle
In file included from C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-internal.h:40:0,
                 from C:/msys64/home/monero/tests/gtest/include/gtest/gtest.h:58,
                 from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:39:
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:209:19: error: 'IsCloseable' was not declared in this scope
     GTEST_CHECK_(!IsCloseable())
                   ^
C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-port.h:1297:37: note: in definition of macro 'GTEST_CHECK_'
     if (::testing::internal::IsTrue(condition)) \
                                     ^~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:209:19: note: suggested alternative: 'CloseHandle'
     GTEST_CHECK_(!IsCloseable())
                   ^
C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-port.h:1297:37: note: in definition of macro 'GTEST_CHECK_'
     if (::testing::internal::IsTrue(condition)) \
                                     ^~~~~~~~~
In file included from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:45:0:
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc: At global scope:
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:215:6: error: 'AutoHandle' has not been declared
 bool AutoHandle::IsCloseable() const {
      ^~~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:215:32: error: non-member function 'bool testing::internal::IsCloseable()' cannot have cv-qualifier
 bool AutoHandle::IsCloseable() const {
                                ^~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc: In function 'bool testing::internal::IsCloseable()':
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:218:10: error: 'handle_' was not declared in this scope
   return handle_ != NULL && handle_ != INVALID_HANDLE_VALUE;
          ^~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:218:10: note: suggested alternative: 'handle_t'
   return handle_ != NULL && handle_ != INVALID_HANDLE_VALUE;
          ^~~~~~~
          handle_t
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc: At global scope:
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:221:1: error: redefinition of 'testing::internal::Notification::Notification()'
 Notification::Notification()
 ^~~~~~~~~~~~
In file included from C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-internal.h:40:0,
                 from C:/msys64/home/monero/tests/gtest/include/gtest/gtest.h:58,
                 from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:39:
C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-port.h:1470:3: note: 'testing::internal::Notification::Notification()' previously defined here
   Notification() : notified_(false) {
   ^~~~~~~~~~~~
In file included from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:45:0:
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:229:6: error: redefinition of 'void testing::internal::Notification::Notify()'
 void Notification::Notify() {
      ^~~~~~~~~~~~
In file included from C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-internal.h:40:0,
                 from C:/msys64/home/monero/tests/gtest/include/gtest/gtest.h:58,
                 from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:39:
C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-port.h:1479:8: note: 'void testing::internal::Notification::Notify()' previously defined here
   void Notify() {
        ^~~~~~
In file included from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:45:0:
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:233:6: error: redefinition of 'void testing::internal::Notification::WaitForNotification()'
 void Notification::WaitForNotification() {
      ^~~~~~~~~~~~
In file included from C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-internal.h:40:0,
                 from C:/msys64/home/monero/tests/gtest/include/gtest/gtest.h:58,
                 from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:39:
C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-port.h:1487:8: note: 'void testing::internal::Notification::WaitForNotification()' previously defined here
   void WaitForNotification() {
        ^~~~~~~~~~~~~~~~~~~
In file included from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:45:0:
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc: In constructor 'testing::internal::ThreadWithParamBase::ThreadWithParamBase(testing::internal::ThreadWithParamBase::Runnable*, testing::internal::Notification*)':
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:377:9: error: class 'testing::internal::ThreadWithParamBase' does not have any field named 'thread_'
       : thread_(ThreadWithParamSupport::CreateThread(runnable,
         ^~~~~~~
In file included from C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-internal.h:40:0,
                 from C:/msys64/home/monero/tests/gtest/include/gtest/gtest.h:58,
                 from C:/msys64/home/monero/tests/gtest/src/gtest-all.cc:39:
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc: In member function 'void testing::internal::ThreadWithParamBase::Join()':
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:386:38: error: 'thread_' was not declared in this scope
   GTEST_CHECK_(::WaitForSingleObject(thread_.Get(), INFINITE) == WAIT_OBJECT_0)
                                      ^
C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-port.h:1297:37: note: in definition of macro 'GTEST_CHECK_'
     if (::testing::internal::IsTrue(condition)) \
                                     ^~~~~~~~~
C:/msys64/home/monero/tests/gtest/src/gtest-port.cc:386:38: note: suggested alternative: '_hread'
   GTEST_CHECK_(::WaitForSingleObject(thread_.Get(), INFINITE) == WAIT_OBJECT_0)
                                      ^
C:/msys64/home/monero/tests/gtest/include/gtest/internal/gtest-port.h:1297:37: note: in definition of macro 'GTEST_CHECK_'
     if (::testing::internal::IsTrue(condition)) \
                                     ^~~~~~~~~
make[6]: *** [CMakeFiles/gtest.dir/build.make:63: CMakeFiles/gtest.dir/src/gtest-all.cc.obj] Error 1
make[6]: Leaving directory '/C/msys64/home/monero/build/release/tests/gtest'
make[5]: *** [CMakeFiles/Makefile2:105: CMakeFiles/gtest.dir/all] Error 2
make[5]: Leaving directory '/C/msys64/home/monero/build/release/tests/gtest'
make[4]: *** [Makefile:130: all] Error 2
make[4]: Leaving directory '/C/msys64/home/monero/build/release/tests/gtest'
make[3]: *** [tests/CMakeFiles/googletest.dir/build.make:111: tests/googletest-prefix/src/googletest-stamp/googletest-build] Error 2
make[3]: Leaving directory '/home/monero/build/release'
make[2]: *** [CMakeFiles/Makefile2:3153: tests/CMakeFiles/googletest.dir/all] Error 2
make[2]: *** Waiting for unfinished jobs....

After applying changes from google/googletest#621, gtest and this PR built successfully, and all tests passed in unit_tests.
(@danrmiller)

@moneromooo-monero
Copy link
Collaborator

The extra define can be moved to one of our files.

@stoffu
Copy link
Contributor Author

stoffu commented Feb 12, 2018

@iDunk5400 @moneromooo-monero
I've moved that macro under tests/unit_tests/unit_tests_utils.h.

@stoffu stoffu changed the title tests: utility ASSERT_EQ_MAP for quickly testing key-value map unit_tests: gtest utility ASSERT_EQ_MAP for easily testing key-value map Feb 12, 2018
@moneromooo-monero
Copy link
Collaborator

Out of curiosity, why GTEST_ASSERT_EQ and not the usual ASSERT_EQ ?

@stoffu
Copy link
Contributor Author

stoffu commented Feb 12, 2018

@moneromooo-monero
I thought they're the same thing and didn't pay much attention to it, but actually the definition

#if !GTEST_DONT_DEFINE_ASSERT_EQ
# define ASSERT_EQ(val1, val2) GTEST_ASSERT_EQ(val1, val2)
#endif

makes a slight difference in that you can disable those macros by setting GTEST_DONT_DEFINE_ASSERT_EQ to 1. So ASSERT_EQ appears to be a slightly better choice here.
Updated, thanks.

Copy link
Contributor

@fluffypony fluffypony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed

@fluffypony fluffypony merged commit 54c256b into monero-project:master Feb 20, 2018
fluffypony added a commit that referenced this pull request Feb 20, 2018
54c256b unit_tests.serialization: refactored with ASSERT_EQ_MAP (stoffu)
e6a6093 unit_tests: added gtest utility ASSERT_EQ_MAP for easily testing key-value map (stoffu)
@stoffu stoffu deleted the assert-eq-map branch February 20, 2018 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants