From 579b31d92a54c158c4a41ec6dd327886298fe4ca Mon Sep 17 00:00:00 2001 From: Samuel Li Date: Fri, 11 Oct 2024 20:55:08 -0700 Subject: [PATCH 1/3] add googletest --- CMakeLists.txt | 29 +++++++ googletest_scripts/CMakeLists.txt | 10 +++ googletest_scripts/ptr_cache_test.cpp | 112 ++++++++++++++++++++++++++ 3 files changed, 151 insertions(+) create mode 100644 googletest_scripts/CMakeLists.txt create mode 100644 googletest_scripts/ptr_cache_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 54a3280a02..ff1d991cde 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -102,6 +102,7 @@ option (BUILD_TEST_APPS "Build test applications" OFF) option (DIST_INSTALLER "Generate installer for distributing vapor binaries. Will generate standard make install if off" OFF) option (USE_OMP "Use OpenMP on some calculations" OFF) option (CONDA_BUILD "Use Conda to build" OFF) +option( BUILD_GOOGLE_TEST "Build unit tests using GoogleTest" OFF ) if (UNIX AND NOT APPLE) include (CMakeDependentOption) cmake_dependent_option (DIST_APPIMAGE "Generate an AppImage for VAPOR's installation across multiple Linux platforms" OFF "DIST_INSTALLER" ON) @@ -116,6 +117,34 @@ if( USE_OMP ) endif() endif() +# +# Build Google Test +# +if( BUILD_GOOGLE_TEST ) + # Control internal options of GoogleTest + # + set( INSTALL_GTEST OFF CACHE INTERNAL "Not install GoogleTest") + set( BUILD_GMOCK ON CACHE INTERNAL "Build gmock") + + # Let's use the new mechanism to incorporate GoogleTest + # + include(FetchContent) + if(${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.24") + FetchContent_Declare( googletest + URL https://github.com/google/googletest/archive/refs/heads/main.zip + DOWNLOAD_EXTRACT_TIMESTAMP NEW ) + endif() + + # Prevent overriding the parent project's compiler/linker settings on Windows + # + set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) + FetchContent_MakeAvailable(googletest) + + enable_testing() # calling this function before adding subdirectory to enable + # invoking ctest from the top-level build directory. + add_subdirectory( googletest_scripts ) +endif() + set (GENERATE_FULL_INSTALLER ON) if (BUILD_GUI) set (BUILD_VDC ON) diff --git a/googletest_scripts/CMakeLists.txt b/googletest_scripts/CMakeLists.txt new file mode 100644 index 0000000000..8ba1bfe0bd --- /dev/null +++ b/googletest_scripts/CMakeLists.txt @@ -0,0 +1,10 @@ +# Add an executable that tests a specific part of VAPOR. +add_executable( ptr_cache_test ptr_cache_test.cpp ) +target_include_directories( ptr_cache_test PRIVATE ${PROJECT_SOURCE_DIR}/include/vapor/ ) +target_link_libraries( ptr_cache_test PUBLIC GTest::gtest_main ) + + +include(GoogleTest) + +# Enable this executable to be tested using `ctest .` +gtest_discover_tests( ptr_cache_test ) diff --git a/googletest_scripts/ptr_cache_test.cpp b/googletest_scripts/ptr_cache_test.cpp new file mode 100644 index 0000000000..4c5cae2f94 --- /dev/null +++ b/googletest_scripts/ptr_cache_test.cpp @@ -0,0 +1,112 @@ +#include "gtest/gtest.h" + +#include "ptr_cache.hpp" // Only include the module that's tested + +namespace { + +struct MyObj { + int i = 0, j = 1; +}; + +// Besides correct insertion/query/eviction behaviors the `ptr_cache` has to be +// deleting objects correctly upon eviction. However, memory errors are not +// something GoogleTest can detect. As a result, one needs to run this +// executable in valgrind to make sure that there are no memory errors: +// valgrind ./googletest_scripts/ptr_cache_test + +// Test the cache when queries don't count as a use. +// It needs to insert and evict correctly. +TEST(ptr_cache_query_false, insert_eviction) +{ + VAPoR::ptr_cache cache; + const auto* p = cache.query(1); + EXPECT_EQ(p, nullptr); + + cache.insert(1, new MyObj{1, 100}); + cache.insert(2, new MyObj{2, 200}); + cache.insert(3, new MyObj{3, 300}); + + // Make sure that we have all 3 objects + p = cache.query(1); + EXPECT_NE(p, nullptr); // not nullptr + EXPECT_EQ(p->j, 100); + p = cache.query(2); + EXPECT_NE(p, nullptr); + EXPECT_EQ(p->j, 200); + p = cache.query(3); + EXPECT_NE(p, nullptr); + EXPECT_EQ(p->j, 300); + p = cache.query(4); + EXPECT_EQ(p, nullptr); + + // Insert a new object; make sure that "1" is evicted. + cache.insert(4, new MyObj{4, 400}); + p = cache.query(4); + EXPECT_NE(p, nullptr); + EXPECT_EQ(p->j, 400); + p = cache.query(1); + EXPECT_EQ(p, nullptr); + + // Do a query on "2" + p = cache.query(2); + + // Insert another object; make sure that "2" is evicted. + cache.insert(5, new MyObj{5, 500}); + p = cache.query(5); + EXPECT_NE(p, nullptr); + EXPECT_EQ(p->j, 500); + p = cache.query(2); + EXPECT_EQ(p, nullptr); +} + + +// Test the cache when queries count as a use. +TEST(ptr_cache_query_true, insert_eviction) +{ + VAPoR::ptr_cache cache; + + cache.insert(1, new MyObj{1, 100}); + cache.insert(2, new MyObj{2, 200}); + cache.insert(3, new MyObj{3, 300}); + + // Make sure that we have all 3 objects + const auto* p = cache.query(1); + EXPECT_NE(p, nullptr); // not nullptr + EXPECT_EQ(p->j, 100); + p = cache.query(2); + EXPECT_NE(p, nullptr); + EXPECT_EQ(p->j, 200); + p = cache.query(3); + EXPECT_NE(p, nullptr); + EXPECT_EQ(p->j, 300); + p = cache.query(4); + EXPECT_EQ(p, nullptr); + + // Insert a new object; make sure that "1" is evicted. + cache.insert(4, new MyObj{4, 400}); + p = cache.query(4); + EXPECT_NE(p, nullptr); + EXPECT_EQ(p->j, 400); + p = cache.query(1); + EXPECT_EQ(p, nullptr); + + // Do a query on "2", then insert, it should be "3" that's evicted. + p = cache.query(2); + cache.insert(5, new MyObj{5, 500}); + p = cache.query(5); + EXPECT_NE(p, nullptr); + EXPECT_EQ(p->j, 500); + p = cache.query(3); + EXPECT_EQ(p, nullptr); + + // Do another query on "2", then insert, it should be "4" that's evicted. + p = cache.query(2); + cache.insert(6, new MyObj{6, 600}); + p = cache.query(6); + EXPECT_NE(p, nullptr); + EXPECT_EQ(p->j, 600); + p = cache.query(4); + EXPECT_EQ(p, nullptr); +} + +} // End of namespace From 6c70510785e3e259a156b086eb669ac166de03ae Mon Sep 17 00:00:00 2001 From: Samuel Li Date: Fri, 11 Oct 2024 21:02:58 -0700 Subject: [PATCH 2/3] add a comment --- googletest_scripts/ptr_cache_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/googletest_scripts/ptr_cache_test.cpp b/googletest_scripts/ptr_cache_test.cpp index 4c5cae2f94..44485635ca 100644 --- a/googletest_scripts/ptr_cache_test.cpp +++ b/googletest_scripts/ptr_cache_test.cpp @@ -20,7 +20,7 @@ TEST(ptr_cache_query_false, insert_eviction) { VAPoR::ptr_cache cache; const auto* p = cache.query(1); - EXPECT_EQ(p, nullptr); + EXPECT_EQ(p, nullptr); // nothing in the cache yet, should get nullptr cache.insert(1, new MyObj{1, 100}); cache.insert(2, new MyObj{2, 200}); @@ -47,7 +47,7 @@ TEST(ptr_cache_query_false, insert_eviction) p = cache.query(1); EXPECT_EQ(p, nullptr); - // Do a query on "2" + // Do a query on "2", but queries don't count as a use. p = cache.query(2); // Insert another object; make sure that "2" is evicted. From 31af6da4b6fdd80f0affecdeebeb13678809b795 Mon Sep 17 00:00:00 2001 From: Ian Franda <134353359+ifranda@users.noreply.github.com> Date: Tue, 15 Oct 2024 12:46:43 -0500 Subject: [PATCH 3/3] Removing if() statement --- CMakeLists.txt | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ff1d991cde..41d91de396 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -129,11 +129,9 @@ if( BUILD_GOOGLE_TEST ) # Let's use the new mechanism to incorporate GoogleTest # include(FetchContent) - if(${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.24") - FetchContent_Declare( googletest - URL https://github.com/google/googletest/archive/refs/heads/main.zip - DOWNLOAD_EXTRACT_TIMESTAMP NEW ) - endif() + FetchContent_Declare( googletest + URL https://github.com/google/googletest/archive/refs/heads/main.zip + DOWNLOAD_EXTRACT_TIMESTAMP NEW ) # Prevent overriding the parent project's compiler/linker settings on Windows #