Avoid sharing temporary files amongst tests

ecmwf · Nov 28, 2023 · befbfba · befbfba
2 parents 1c75073 + ae116e0
commit befbfba
Show file tree

Hide file tree

Showing 6 changed files with 95 additions and 53 deletions.
diff --git a/ANode/CMakeLists.txt b/ANode/CMakeLists.txt
@@ -246,8 +246,10 @@ target_clangformat(u_anode
 set(test_srcs
   # Headers
   parser/test/PersistHelper.hpp
+  parser/test/TemporaryFile.hpp
   # Sources
   parser/test/PersistHelper.cpp
+  parser/test/TemporaryFile.cpp
   parser/test/TestAutoAddExterns.cpp
   parser/test/TestDefsStructurePersistAndReload.cpp
   parser/test/TestMementoPersistAndReload.cpp
@@ -304,8 +306,10 @@ if (ENABLE_ALL_TESTS)
   set(test_srcs
     # Headers
     parser/test/PersistHelper.hpp
+    parser/test/TemporaryFile.hpp
     # Sources
     parser/test/PersistHelper.cpp
+    parser/test/TemporaryFile.cpp
     parser/test/TestParserPerformance_main.cpp # test entry point
     parser/test/TestSingleDefsFile.cpp
   )
@@ -331,9 +335,11 @@ if (ENABLE_ALL_TESTS)
   set(test_srcs
     # Headers
     parser/test/PersistHelper.hpp
+    parser/test/TemporaryFile.hpp
     # Sources
     parser/test/ParseTimer.cpp
     parser/test/PersistHelper.cpp
+    parser/test/TemporaryFile.cpp
   )
 
   # The following is not technically a test (as it makes no checks),

diff --git a/ANode/parser/test/ParseTimer.cpp b/ANode/parser/test/ParseTimer.cpp
@@ -36,6 +36,7 @@
 #include "Suite.hpp"
 #include "System.hpp"
 #include "Task.hpp"
+#include "TemporaryFile.hpp"
 
 using namespace std;
 using namespace ecf;
@@ -133,14 +134,12 @@ int main(int argc, char* argv[]) {
     }
     {
         // Test time for persisting to defs file only
-        std::string tmpFilename = "tmp.def";
+        TemporaryFile temporary("tmp_%%%%-%%%%-%%%%-%%%%.def");
 
         timer.start();
-        defs.save_as_checkpt(tmpFilename);
+        defs.save_as_checkpt(temporary.path());
         cout << " Save as DEFS checkpoint, time taken            = " << timer.format(3, Str::cpu_timer_format())
              << endl;
-
-        std::remove(tmpFilename.c_str());
     }
 
     {

diff --git a/ANode/parser/test/PersistHelper.cpp b/ANode/parser/test/PersistHelper.cpp
@@ -22,6 +22,7 @@
 #include "Defs.hpp"
 #include "Ecf.hpp"
 #include "File.hpp"
+#include "TemporaryFile.hpp"
 
 namespace fs = boost::filesystem;
 using namespace std;
@@ -34,39 +35,33 @@ bool PersistHelper::test_persist_and_reload(const Defs& theInMemoryDefs,
     errorMsg_.clear();
     file_size_ = 0;
 
-#ifdef DEBUG
-    std::string tmpFilename = "tmp_d.def";
-#else
-    std::string tmpFilename     = "tmp.def";
-#endif
+    TemporaryFile temporary("tmp_%%%%-%%%%-%%%%-%%%%.def");
+
     {
         // The file MUST be written in the *SAME* form that it was read, Otherwise they will not compare:
-        theInMemoryDefs.save_as_filename(tmpFilename, file_type_on_disk);
+        theInMemoryDefs.save_as_filename(temporary.path(), file_type_on_disk);
     }
 
     // Reload the file we just persisted and compare with in memory defs
     Defs savedDef;
-    return reload_from_defs_file(theInMemoryDefs, savedDef, tmpFilename, do_compare);
+    return reload_from_defs_file(theInMemoryDefs, savedDef, temporary.path(), do_compare);
 }
 
 bool PersistHelper::test_defs_checkpt_and_reload(const Defs& theInMemoryDefs, bool do_compare) {
     // Write parsed file to disk, and reload, then compare defs, they should be the same
     errorMsg_.clear();
     file_size_ = 0;
 
-#ifdef DEBUG
-    std::string tmpFilename = "tmp_d.def";
-#else
-    std::string tmpFilename     = "tmp.def";
-#endif
+    TemporaryFile temporary("tmp_%%%%-%%%%-%%%%-%%%%.def");
+
     {
         // The file MUST be written in the *SAME* form that it was read, Otherwise they will not compare:
-        theInMemoryDefs.save_as_checkpt(tmpFilename);
+        theInMemoryDefs.save_as_checkpt(temporary.path());
     }
 
     // Reload the file we just persisted and compare with in memory defs
     Defs savedDef;
-    bool reload_result = reload_from_defs_file(theInMemoryDefs, savedDef, tmpFilename, do_compare);
+    bool reload_result = reload_from_defs_file(theInMemoryDefs, savedDef, temporary.path(), do_compare);
     if (reload_result)
         return savedDef.checkInvariants(errorMsg_);
     return false;
@@ -97,22 +92,19 @@ bool PersistHelper::test_state_persist_and_reload_with_checkpt(const Defs& theIn
 
     DebugEquality debug_equality; // only as affect in DEBUG build
 
-#ifdef DEBUG
-    std::string tmpFilename = "tmp_d.def";
-#else
-    std::string tmpFilename     = "tmp.def";
-#endif
+    TemporaryFile temporary("tmp_%%%%-%%%%-%%%%-%%%%.def");
+
     {
         // The file MUST be written in the *SAME* form that it was read, Otherwise they will not compare:
-        theInMemoryDefs.save_as_checkpt(tmpFilename); // will save edit history
+        theInMemoryDefs.save_as_checkpt(temporary.path()); // will save edit history
     }
 
     Defs reload_strings_def;
     {
         // Open file, and parse as a string.
         std::string defs_as_string;
-        if (!File::open(tmpFilename, defs_as_string)) {
-            errorMsg_ += "Could not file file: " + tmpFilename;
+        if (!File::open(temporary.path(), defs_as_string)) {
+            errorMsg_ += "Could not file file: " + temporary.path();
             return false;
         }
         std::string error_msg, warning;
@@ -127,7 +119,7 @@ bool PersistHelper::test_state_persist_and_reload_with_checkpt(const Defs& theIn
 
     // Reload the file we just persisted and compare with in memory defs
     Defs reloaded_defs;
-    if (!reload_from_defs_file(theInMemoryDefs, reloaded_defs, tmpFilename)) {
+    if (!reload_from_defs_file(theInMemoryDefs, reloaded_defs, temporary.path())) {
         return false;
     }
     if (!reloaded_defs.checkInvariants(errorMsg_)) {
@@ -271,22 +263,19 @@ bool PersistHelper::reload_from_defs_file(const Defs& theInMemoryDefs,
 }
 
 bool PersistHelper::reload_from_cereal_checkpt_file(const Defs& theInMemoryDefs, Defs& reloaded_defs, bool do_compare) {
-    // make sure edit history is saved
-#ifdef DEBUG
-    std::string tmpCheckPt_file = "tmp.check_debug";
-#else
-    std::string tmpCheckPt_file = "tmp.check";
-#endif
-    theInMemoryDefs.cereal_save_as_checkpt(tmpCheckPt_file);
+    // make sure edit history is stored
+    TemporaryFile temporary("tmp.check_%%%%-%%%%-%%%%-%%%%");
 
-    DebugEquality debug_equality; // only as affect in DEBUG build
+    theInMemoryDefs.cereal_save_as_checkpt(temporary.path());
+
+    DebugEquality debug_equality; // only as effect in DEBUG build
 
     try {
         // Parse the file we just persisted and load the defs file into memory.
-        reloaded_defs.cereal_restore_from_checkpt(tmpCheckPt_file);
+        reloaded_defs.cereal_restore_from_checkpt(temporary.path());
 
         if (do_compare) {
-            // Make sure the checkpoint file file we just parsed match's the one we persisted
+            // Make sure the checkpoint the file we just parsed corresponds to the one we stored
             bool match = reloaded_defs == theInMemoryDefs;
             if (!match) {
                 std::stringstream ss;
@@ -322,8 +311,7 @@ bool PersistHelper::reload_from_cereal_checkpt_file(const Defs& theInMemoryDefs,
         errorMsg_ = "PersistHelper::reload_from_cereal_checkpt_file: " + string(e.what());
     }
 
-    file_size_ = fs::file_size(tmpCheckPt_file);
-    std::remove(tmpCheckPt_file.c_str());
+    file_size_ = temporary.size();
 
     return errorMsg_.empty();
 }
diff --git a/ANode/parser/test/TemporaryFile.cpp b/ANode/parser/test/TemporaryFile.cpp
@@ -0,0 +1,26 @@
+/*
+ * Copyright 2023- ECMWF.
+ *
+ * This software is licensed under the terms of the Apache Licence version 2.0
+ * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0.
+ * In applying this licence, ECMWF does not waive the privileges and immunities
+ * granted to it by virtue of its status as an intergovernmental organisation
+ * nor does it submit to any jurisdiction.
+ */
+
+#include "TemporaryFile.hpp"
+
+TemporaryFile::TemporaryFile() : path_{fs::unique_path("tmp_%%%%-%%%%-%%%%-%%%%")} {
+}
+
+TemporaryFile::TemporaryFile(const std::string& pattern) : path_{fs::unique_path(pattern)} {
+}
+
+TemporaryFile::~TemporaryFile() {
+    try {
+        fs::remove(path_);
+    }
+    catch (...) {
+        // Nothing to do...
+    }
+}
diff --git a/ANode/parser/test/TemporaryFile.hpp b/ANode/parser/test/TemporaryFile.hpp
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2023- ECMWF.
+ *
+ * This software is licensed under the terms of the Apache Licence version 2.0
+ * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0.
+ * In applying this licence, ECMWF does not waive the privileges and immunities
+ * granted to it by virtue of its status as an intergovernmental organisation
+ * nor does it submit to any jurisdiction.
+ */
+
+#ifndef ECFLOW_NODE_PARSER_TEST_TEMPORARYFILE_HPP
+#define ECFLOW_NODE_PARSER_TEST_TEMPORARYFILE_HPP
+
+#include <string>
+
+#include <boost/filesystem.hpp>
+
+namespace fs = boost::filesystem;
+
+class TemporaryFile {
+public:
+    TemporaryFile();
+    explicit TemporaryFile(const std::string& pattern);
+
+    ~TemporaryFile();
+
+    [[nodiscard]] inline std::string path() const { return path_.string(); }
+    [[nodiscard]] inline size_t size() const { return fs::file_size(path_); }
+
+private:
+    fs::path path_;
+};
+
+#endif
diff --git a/ANode/parser/test/TestSingleDefsFile.cpp b/ANode/parser/test/TestSingleDefsFile.cpp
@@ -18,10 +18,8 @@
 #include <string>
 
 #include <boost/chrono.hpp>
-#include <boost/date_time/posix_time/posix_time_types.hpp>
 #include <boost/filesystem/operations.hpp>
 #include <boost/filesystem/path.hpp>
-
 #include <boost/test/unit_test.hpp>
 #include <boost/timer/timer.hpp>
 
@@ -39,6 +37,7 @@
 #include "Suite.hpp"
 #include "System.hpp"
 #include "Task.hpp"
+#include "TemporaryFile.hpp"
 
 namespace fs = boost::filesystem;
 using namespace std;
@@ -173,24 +172,18 @@ BOOST_AUTO_TEST_CASE(test_single_defs) {
 
     {
         // Test time for persisting to defs file only
-#ifdef DEBUG
-        std::string tmpFilename = "tmp_d.def";
-#else
-        std::string tmpFilename = "tmp.def";
-#endif
+        TemporaryFile temporary("tmp_%%%%-%%%%-%%%%-%%%%.def");
 
         timer.start();
         PrintStyle style(PrintStyle::DEFS);
-        std::ofstream ofs(tmpFilename.c_str());
+        std::ofstream ofs(temporary.path());
         ofs << defs;
         BOOST_CHECK_MESSAGE(get_seconds(timer.elapsed().user) < expectedTimeForDefsPersistOnly,
                             "Performance regression, expected < " << expectedTimeForDefsPersistOnly
                                                                   << " to persist defs file, but found "
                                                                   << timer.format(3, Str::cpu_timer_format()));
         cout << " Persist only, time taken                               = " << timer.format(3, Str::cpu_timer_format())
              << " < limit(" << expectedTimeForDefsPersistOnly << ")" << endl;
-
-        std::remove(tmpFilename.c_str());
     }
 
     {
@@ -331,10 +324,6 @@ BOOST_AUTO_TEST_CASE(test_single_defs) {
     // Explicitly destroy, To keep valgrind happy
     Log::destroy();
     System::destroy();
-
-    // cout << "Printing Defs \n";
-    // PrintStyle style(PrintStyle::DEFS);
-    //	std::cout << defs;
 }
 
 BOOST_AUTO_TEST_SUITE_END()