From ff86a660e707fccc69e652d735b46f71fbf9cbc9 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Mon, 6 Jan 2025 16:24:09 -0700 Subject: [PATCH] tests that create temporary filesystem entries should use paths prefixed with TEST_WORKING_DIRECTORY --- .github/workflows/check.yml | 2 +- .github/workflows/test.yml | 2 +- test/unit/googletest/CMakeLists.txt | 17 +++++----- .../{PoolArgs.cpp => PoolArgs.cpp.in} | 20 ++++++------ test/unit/googletest/dbutils.cpp.in | 2 +- test/unit/googletest/external.cpp.in | 29 +++++------------ .../unit/googletest/{swap.cpp => swap.cpp.in} | 2 +- .../{template_db.cpp => template_db.cpp.in} | 17 +++++----- .../googletest/{trace.cpp => trace.cpp.in} | 2 +- test/unit/googletest/utils.cpp.in | 32 +++++++++---------- .../googletest/{xattrs.cpp => xattrs.cpp.in} | 2 +- 11 files changed, 56 insertions(+), 71 deletions(-) rename test/unit/googletest/{PoolArgs.cpp => PoolArgs.cpp.in} (94%) rename test/unit/googletest/{swap.cpp => swap.cpp.in} (99%) rename test/unit/googletest/{template_db.cpp => template_db.cpp.in} (94%) rename test/unit/googletest/{trace.cpp => trace.cpp.in} (99%) rename test/unit/googletest/{xattrs.cpp => xattrs.cpp.in} (99%) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 29add85d4..80bc7fef8 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -269,7 +269,7 @@ jobs: run: sudo contrib/CI/ubuntu.sh - name: Configure CMake - run: cmake -B ${{ github.workspace }}/build ${{ env.COMMON_CONFIG }} -DDEP_USE_JEMALLOC=Off + run: cmake -B ${{ github.workspace }}/build ${{ env.COMMON_CONFIG }} -DDEP_USE_JEMALLOC=Off -DTEST_WORKING_DIRECTORY="$(pwd)" - name: Build run: cmake --build ${{ github.workspace }}/build -j diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index acb4f5b84..31792b7b5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -127,7 +127,7 @@ jobs: run: contrib/CI/macos.sh - name: Configure CMake - run: cmake -B ${{ github.workspace }}/build ${{ env.COMMON_CONFIG }} + run: cmake -B ${{ github.workspace }}/build ${{ env.COMMON_CONFIG }} -DTEST_WORKING_DIRECTORY="$(pwd)" - name: Build run: cmake --build ${{ github.workspace }}/build -j diff --git a/test/unit/googletest/CMakeLists.txt b/test/unit/googletest/CMakeLists.txt index 8744488d6..aedef9934 100644 --- a/test/unit/googletest/CMakeLists.txt +++ b/test/unit/googletest/CMakeLists.txt @@ -71,7 +71,6 @@ if (CMAKE_CXX_COMPILER) include_directories(${DEP_INSTALL_PREFIX}/googletest/include) set(TEST_SRC OutputBuffers.cpp - PoolArgs.cpp QueuePerThreadPool.cpp SinglyLinkedList.cpp bf.cpp @@ -79,27 +78,27 @@ if (CMAKE_CXX_COMPILER) debug.cpp histogram.cpp print.cpp - swap.cpp - template_db.cpp - trace.cpp trie.cpp - utils.cpp - xattrs.cpp ) - # these test files use CMAKE_BINARY_DIR + # these test files use CMake variables set(NEED_PATH BottomUp.cpp + PoolArgs.cpp aggregate.cpp dbutils.cpp external.cpp + swap.cpp + template_db.cpp + trace.cpp utils.cpp - ) + xattrs.cpp + ) foreach(SRC ${NEED_PATH}) configure_file("${SRC}.in" "${SRC}" @ONLY) list(APPEND TEST_SRC "${CMAKE_CURRENT_BINARY_DIR}/${SRC}" - ) + ) endforeach() add_executable(unit_tests ${TEST_SRC} $) diff --git a/test/unit/googletest/PoolArgs.cpp b/test/unit/googletest/PoolArgs.cpp.in similarity index 94% rename from test/unit/googletest/PoolArgs.cpp rename to test/unit/googletest/PoolArgs.cpp.in index 70eb7f5a4..6c60e5ce6 100644 --- a/test/unit/googletest/PoolArgs.cpp +++ b/test/unit/googletest/PoolArgs.cpp.in @@ -178,7 +178,7 @@ TEST(PoolArgs, OUTFILE) { char outname[MAXPATH]; in.outname.data = outname; - in.outname.len = snprintf(outname, sizeof(outname), "XXXXXX"); + in.outname.len = snprintf(outname, sizeof(outname), "@TEST_WORKING_DIRECTORY@/XXXXXX"); const int fd = mkstemp(outname); EXPECT_NE(fd, -1); EXPECT_EQ(close(fd), 0); @@ -219,7 +219,7 @@ TEST(PoolArgs, OUTFILE_aggregate) { char outname[MAXPATH]; in.outname.data = outname; - in.outname.len = snprintf(outname, sizeof(outname), "XXXXXX"); + in.outname.len = snprintf(outname, sizeof(outname), "@TEST_WORKING_DIRECTORY@/XXXXXX"); const int fd = mkstemp(outname); EXPECT_NE(fd, -1); EXPECT_EQ(close(fd), 0); @@ -262,7 +262,7 @@ TEST(PoolArgs, OUTDB) { char outname[MAXPATH]; in.outname.data = outname; - in.outname.len = snprintf(outname, sizeof(outname), "XXXXXX"); + in.outname.len = snprintf(outname, sizeof(outname), "@TEST_WORKING_DIRECTORY@/XXXXXX"); const int fd = mkstemp(outname); EXPECT_NE(fd, -1); EXPECT_EQ(close(fd), 0); @@ -299,7 +299,7 @@ TEST(PoolArgs, OUTDB_aggregate) { char outname[MAXPATH]; in.outname.data = outname; - in.outname.len = snprintf(outname, sizeof(outname), "XXXXXX"); + in.outname.len = snprintf(outname, sizeof(outname), "@TEST_WORKING_DIRECTORY@/XXXXXX"); const int fd = mkstemp(outname); EXPECT_NE(fd, -1); EXPECT_EQ(close(fd), 0); @@ -341,11 +341,11 @@ TEST(PoolArgs, bad_outdb) { struct input in; setup_input(&in, OUTDB, false); - char parent[7]; - snprintf(parent, sizeof(parent), "XXXXXX"); + char parent[sizeof("@TEST_WORKING_DIRECTORY@/") + 7]; + snprintf(parent, sizeof(parent), "@TEST_WORKING_DIRECTORY@/XXXXXX"); ASSERT_EQ(mkdtemp(parent), parent); - char prefix[13]; + char prefix[sizeof(parent) + 6]; snprintf(prefix, sizeof(prefix), "%s/outdb", parent); in.outname.data = prefix; @@ -365,11 +365,11 @@ TEST(PoolArgs, bad_outfile) { struct input in; setup_input(&in, OUTFILE, false); - char parent[7]; - snprintf(parent, sizeof(parent), "XXXXXX"); + char parent[sizeof("@TEST_WORKING_DIRECTORY@/") + 7]; + snprintf(parent, sizeof(parent), "@TEST_WORKING_DIRECTORY@/XXXXXX"); ASSERT_EQ(mkdtemp(parent), parent); - char prefix[15]; + char prefix[sizeof(parent) + 8]; snprintf(prefix, sizeof(prefix), "%s/outfile", parent); in.outname.data = prefix; diff --git a/test/unit/googletest/dbutils.cpp.in b/test/unit/googletest/dbutils.cpp.in index 7d80ebb46..627ebd8c1 100644 --- a/test/unit/googletest/dbutils.cpp.in +++ b/test/unit/googletest/dbutils.cpp.in @@ -803,7 +803,7 @@ TEST(get_rollupscore, nullptr) { TEST(bottomup_collect_treesummary, nullptr) { EXPECT_EQ(bottomup_collect_treesummary(nullptr, "", nullptr, ROLLUPSCORE_KNOWN_YES), 1); - char dirname[] = "XXXXXX"; + char dirname[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; ASSERT_EQ(mkdtemp(dirname), dirname); sll_t sll; diff --git a/test/unit/googletest/external.cpp.in b/test/unit/googletest/external.cpp.in index e289c74ef..bce05aa21 100644 --- a/test/unit/googletest/external.cpp.in +++ b/test/unit/googletest/external.cpp.in @@ -78,26 +78,13 @@ static size_t bad_filename(char **dst, const size_t dst_size, TEST(external, read_file) { // user external db list - char filename[] = "XXXXXX"; + char filename[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; const int fd = mkstemp(filename); ASSERT_GE(fd, 0); - // create bad relative path (bad path) - { - char bad_rel_path[] = "XXXXXX"; // full path fails to resolve in external_read_file - const int bad_rel_path_fd = mkstemp(bad_rel_path); - ASSERT_GE(bad_rel_path_fd, 0); - EXPECT_EQ(close(bad_rel_path_fd), 0); - EXPECT_EQ(remove(bad_rel_path), 0); - - // add to list - EXPECT_EQ(write(fd, bad_rel_path, (std::size_t) 6), (ssize_t) 6); - EXPECT_EQ(write(fd, "\n", (std::size_t) 1), (ssize_t) 1); - } - // create bad absolute path (bad path) { - char bad_rel_path[] = "XXXXXX"; + char bad_rel_path[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; const int bad_rel_path_fd = mkstemp(bad_rel_path); ASSERT_GE(bad_rel_path_fd, 0); EXPECT_EQ(close(bad_rel_path_fd), 0); @@ -125,20 +112,20 @@ TEST(external, read_file) { } // create file with contents (good path, not a db) - char not_db_filename[] = "XXXXXX"; + char not_db_filename[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; { const int not_db_fd = mkstemp(not_db_filename); ASSERT_GE(not_db_fd, 0); - EXPECT_EQ(write(not_db_fd, not_db_filename, (std::size_t) 6), (ssize_t) 6); + EXPECT_EQ(write(not_db_fd, not_db_filename, sizeof(not_db_filename) - 1), (ssize_t) sizeof(not_db_filename) - 1); EXPECT_EQ(close(not_db_fd), 0); // add to list - EXPECT_EQ(write(fd, not_db_filename, (std::size_t) 6), (ssize_t) 6); + EXPECT_EQ(write(fd, not_db_filename, sizeof(not_db_filename) - 1), (ssize_t) sizeof(not_db_filename) - 1); EXPECT_EQ(write(fd, "\n", (std::size_t) 1), (ssize_t) 1); } // create db file (good path, is a db) - char dbname[] = "XXXXXX"; + char dbname[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; { const int db_fd = mkstemp(dbname); ASSERT_GE(db_fd, 0); @@ -150,7 +137,7 @@ TEST(external, read_file) { closedb(db); // add to list - EXPECT_EQ(write(fd, dbname, (std::size_t) 6), (ssize_t) 6); + EXPECT_EQ(write(fd, dbname, sizeof(dbname) - 1), (ssize_t) sizeof(dbname) - 1); EXPECT_EQ(write(fd, "\n", (std::size_t) 1), (ssize_t) 1); } @@ -168,7 +155,7 @@ TEST(external, read_file) { const long long int, const char *) -> int { return 0; }, nullptr), - (std::size_t) 4); // bad relative path does not pass + (std::size_t) 4); // check that path is valid db in.check_extdb_valid = true; diff --git a/test/unit/googletest/swap.cpp b/test/unit/googletest/swap.cpp.in similarity index 99% rename from test/unit/googletest/swap.cpp rename to test/unit/googletest/swap.cpp.in index 837887436..8aed2e869 100644 --- a/test/unit/googletest/swap.cpp +++ b/test/unit/googletest/swap.cpp.in @@ -231,7 +231,7 @@ TEST(swap, stop_twice) { } TEST(swap, nonexistant_prefix) { - char parent[] = "XXXXXX"; + char parent[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; ASSERT_NE(mkdtemp(parent), nullptr); char prefix[1024]; diff --git a/test/unit/googletest/template_db.cpp b/test/unit/googletest/template_db.cpp.in similarity index 94% rename from test/unit/googletest/template_db.cpp rename to test/unit/googletest/template_db.cpp.in index 361ae1781..e3ad61a59 100644 --- a/test/unit/googletest/template_db.cpp +++ b/test/unit/googletest/template_db.cpp.in @@ -108,8 +108,8 @@ TEST(template_db, create_copy) { struct stat st; EXPECT_EQ(lstat(template_name, &st), -1); - char dst_name[7]; - snprintf(dst_name, sizeof(dst_name), "XXXXXX"); + char dst_name[sizeof("@TEST_WORKING_DIRECTORY@/") + 7]; + snprintf(dst_name, sizeof(dst_name), "@TEST_WORKING_DIRECTORY@/XXXXXX"); int dst = mkstemp(dst_name); ASSERT_NE(dst, -1); EXPECT_EQ(close(dst), 0); @@ -157,7 +157,7 @@ TEST(create_empty_dbdb, good) { ASSERT_EQ(create_template(&tdb, create_test_tables, template_name), 0); // create new directory - char dirname[] = "XXXXXX" ; + char dirname[] = "@TEST_WORKING_DIRECTORY@/XXXXXX" ; ASSERT_NE(mkdtemp(dirname), nullptr); refstr_t dst; @@ -173,10 +173,9 @@ TEST(create_empty_dbdb, good) { // create file under dirname not called db.db char filename[MAXPATH]; - SNFORMAT_S(filename, sizeof(filename), 3, + SNFORMAT_S(filename, sizeof(filename), 2, dst.data, dst.len, - "/", (std::size_t) 1, - "XXXXXX", 6); + "/XXXXXX", (std::size_t) 7); const int fd = mkstemp(filename); ASSERT_GT(fd, -1); EXPECT_EQ(close(fd), 0); @@ -189,8 +188,8 @@ TEST(create_empty_dbdb, good) { // symlink to char symlinktarget[MAXPATH]; SNFORMAT_S(symlinktarget, sizeof(symlinktarget), 2, - "../", (std::size_t) 3, - dst.data, dst.len); + dst.data, dst.len, + "/..", (std::size_t) 3); // symink to directory causes error ASSERT_EQ(symlink(symlinktarget, dbname), 0); @@ -216,7 +215,7 @@ TEST(create_empty_dbdb, good) { } TEST(create_empty_dbdb, path_is_file) { - char filename[] = "XXXXXX" ; + char filename[] = "@TEST_WORKING_DIRECTORY@/XXXXXX" ; const int fd = mkstemp(filename); ASSERT_GT(fd, -1); EXPECT_EQ(close(fd), 0); diff --git a/test/unit/googletest/trace.cpp b/test/unit/googletest/trace.cpp.in similarity index 99% rename from test/unit/googletest/trace.cpp rename to test/unit/googletest/trace.cpp.in index 874b5341d..600b497bc 100644 --- a/test/unit/googletest/trace.cpp +++ b/test/unit/googletest/trace.cpp.in @@ -295,7 +295,7 @@ TEST(scout_trace, no_cleanup) { ASSERT_EQ(rc, (int) len); // write trace to file - char tracename[] = "XXXXXX"; + char tracename[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; const int fd = mkstemp(tracename); ASSERT_GT(fd, -1); ASSERT_EQ(write(fd, line, len), (ssize_t) len); diff --git a/test/unit/googletest/utils.cpp.in b/test/unit/googletest/utils.cpp.in index 6fdd66116..1cef1004f 100644 --- a/test/unit/googletest/utils.cpp.in +++ b/test/unit/googletest/utils.cpp.in @@ -400,7 +400,7 @@ TEST(shortpath, split) { } TEST(dupdir, parentfirst) { - char root[] = "XXXXXX"; + char root[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; ASSERT_NE(mkdtemp(root), nullptr); char parent[MAXPATH]; @@ -444,7 +444,7 @@ TEST(dupdir, parentfirst) { } TEST(dupdir, childtfirst) { - char root[] = "XXXXXX"; + char root[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; ASSERT_NE(mkdtemp(root), nullptr); char parent[MAXPATH]; @@ -509,7 +509,7 @@ TEST(dupdir, childtfirst) { } TEST(dupdir, fileasdir) { - char parent[] = "XXXXXX"; + char parent[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; const int fd = mkstemp(parent); ASSERT_GE(fd, -1); EXPECT_EQ(close(fd), 0); @@ -527,7 +527,7 @@ TEST(dupdir, fileasdir) { } TEST(mkpath, parentfirst) { - char root[] = "XXXXXX"; + char root[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; ASSERT_NE(mkdtemp(root), nullptr); char parent[MAXPATH]; @@ -576,7 +576,7 @@ TEST(mkpath, parentfirst) { } TEST(mkpath, childfirst) { - char root[] = "XXXXXX"; + char root[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; ASSERT_NE(mkdtemp(root), nullptr); char parent[MAXPATH]; @@ -622,7 +622,7 @@ TEST(mkpath, childfirst) { } TEST(mkpath, fileasdir) { - char parent[] = "XXXXXX"; + char parent[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; const int fd = mkstemp(parent); ASSERT_GE(fd, -1); EXPECT_EQ(close(fd), 0); @@ -846,7 +846,7 @@ TEST(setup_directory_skip, file) { "skip0", // duplicate }; - char skip_name[] = "XXXXXX"; + char skip_name[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; const int skip_fd = mkstemp(skip_name); ASSERT_NE(skip_fd, -1); EXPECT_EQ(close(skip_fd), 0); @@ -876,7 +876,7 @@ TEST(setup_directory_skip, bad) { trie_t *skip = trie_alloc(); ASSERT_NE(skip, nullptr); - char name[] = "XXXXXX"; + char name[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; const int fd = mkstemp(name); ASSERT_GT(fd, -1); EXPECT_EQ(setup_directory_skip(skip, name), 0); @@ -947,8 +947,8 @@ TEST(getline, fd) { memset(LINE, 'X', LINE_LEN); - char name[7]; - snprintf(name, sizeof(name), "XXXXXX"); + char name[sizeof("@TEST_WORKING_DIRECTORY@/") + 7]; + snprintf(name, sizeof(name), "@TEST_WORKING_DIRECTORY@/XXXXXX"); const int fd = mkstemp(name); ASSERT_GT(fd, -1); EXPECT_EQ(remove(name), 0); @@ -1035,7 +1035,7 @@ TEST(copyfd, good) { std::mt19937 gen(rd()); std::uniform_int_distribution <> dist(0, 255); - char srcname[] = "XXXXXX"; + char srcname[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; int src = mkstemp(srcname); ASSERT_GT(src, -1); @@ -1063,7 +1063,7 @@ TEST(copyfd, good) { for(std::size_t i = 0; i < sizeof(offsets) / sizeof(offsets[0]); i++) { // new dst each time - char dstname[] = "XXXXXX"; + char dstname[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; int dst = mkstemp(dstname); ASSERT_GT(dst, -1); @@ -1100,11 +1100,11 @@ TEST(copyfd, good) { } TEST(copyfd, bad) { - char srcname[] = "XXXXXX"; + char srcname[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; int src = mkstemp(srcname); ASSERT_GT(src, -1); - char dstname[] = "XXXXXX"; + char dstname[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; int dst = mkstemp(dstname); ASSERT_GT(dst, -1); @@ -1119,7 +1119,7 @@ TEST(copyfd, bad) { } TEST(set_metadata, bad) { - char dir[] = "XXXXXX"; + char dir[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; ASSERT_NE(mkdtemp(dir), nullptr); char path[MAXPATH]; @@ -1143,7 +1143,7 @@ TEST(set_metadata, bad) { } TEST(write_read, size) { - char filename[] = "XXXXXX"; + char filename[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; int fd = mkstemp(filename); ASSERT_GT(fd, -1); EXPECT_EQ(remove(filename), 0); diff --git a/test/unit/googletest/xattrs.cpp b/test/unit/googletest/xattrs.cpp.in similarity index 99% rename from test/unit/googletest/xattrs.cpp rename to test/unit/googletest/xattrs.cpp.in index d4d2ce181..1bfbbe565 100644 --- a/test/unit/googletest/xattrs.cpp +++ b/test/unit/googletest/xattrs.cpp.in @@ -107,7 +107,7 @@ TEST(xattrs, alloc) { } TEST(xattrs, get) { - char name[] = "XXXXXX"; + char name[] = "@TEST_WORKING_DIRECTORY@/XXXXXX"; const int fd = mkstemp(name); ASSERT_NE(fd, -1); close(fd);