[lmi-commits] [lmi] master 802eb23 6/6: Refactor for simplicity

[Greg Chicares]

branch: master
commit 802eb23c6b66a432a1d706ce61f7bf1d50e50e4d
Author: Gregory W. Chicares <address@hidden>
Commit: Gregory W. Chicares <address@hidden>

    Refactor for simplicity
    
    This unit test formerly reported
      1608 tests succeeded
    but instead now reports
      650 tests succeeded
    because the replaced file comparison called unit-test macros liberally.
---
 rate_table_test.cpp |  135 ++++++++++++---------------------------------------
 1 file changed, 31 insertions(+), 104 deletions(-)

diff --git a/rate_table_test.cpp b/rate_table_test.cpp
index a2cca17..863f606 100644
--- a/rate_table_test.cpp
+++ b/rate_table_test.cpp
@@ -44,7 +44,6 @@ using namespace soa_v3_format;
 // Unit test helpers for working with files.
 namespace
 {
-
 // Class temporarily redirecting std::cout to a string: this is useful to check
 // that the expected output appears on cout or just to suppress some output
 // we're not interested in.
@@ -115,96 +114,6 @@ class test_file_eraser
   private:
     fs::path path_;
 };
-
-// Check that the two binary files contents is identical, failing the current
-// test if it isn't.
-//
-// BOOST !! We could use BOOST_CHECK_EQUAL_COLLECTIONS if we could use the
-// full Boost.Test framework.
-void check_files_equal
-    (fs::path const& path1
-    ,fs::path const& path2
-    ,char const* file
-    ,int line
-    )
-{
-    fs::ifstream ifs1(path1, std::ios_base::in | std::ios_base::binary);
-    INVOKE_BOOST_TEST(!ifs1.bad(), file, line);
-
-    fs::ifstream ifs2(path2, std::ios_base::in | std::ios_base::binary);
-    INVOKE_BOOST_TEST(!ifs2.bad(), file, line);
-
-    // Compare the file sizes.
-    ifs1.seekg(0, std::ios_base::end);
-    ifs2.seekg(0, std::ios_base::end);
-    INVOKE_BOOST_TEST_EQUAL(ifs1.tellg(), ifs2.tellg(), file, line);
-    if(ifs1.tellg() != ifs2.tellg())
-        {
-        lmi_test::record_error();
-        lmi_test::error_stream()
-            << "Files '" << path1 << "' and '" << path2 << "' "
-            << "have different sizes: " << ifs1.tellg() << " and "
-            << ifs2.tellg() << " respectively."
-            << BOOST_TEST_FLUSH
-            ;
-        return;
-        }
-
-    // Rewind back to the beginning.
-    ifs1.seekg(0, std::ios_base::beg);
-    ifs2.seekg(0, std::ios_base::beg);
-
-    // Look for differences: using istream_iterator<char> here would be simpler
-    // but also much less efficient, so read the file by larger blocks instead.
-    const int buffer_size = 4096;
-    char buf1[buffer_size];
-    char buf2[buffer_size];
-    for(std::streamsize offset = 0;;)
-        {
-        ifs1.read(buf1, buffer_size);
-        INVOKE_BOOST_TEST(!ifs1.bad(), file, line);
-
-        ifs2.read(buf2, buffer_size);
-        INVOKE_BOOST_TEST(!ifs2.bad(), file, line);
-
-        std::streamsize const count = ifs1.gcount();
-        INVOKE_BOOST_TEST_EQUAL(count, ifs2.gcount(), file, line);
-
-        if(!count)
-            {
-            return;
-            }
-
-        for(std::streamsize pos = 0; pos < count; ++pos)
-            {
-            if(buf1[pos] != buf2[pos])
-                {
-                lmi_test::record_error();
-                lmi_test::error_stream()
-                    << "Files '" << path1 << "' and '" << path2 << "' "
-                    << "differ at offset " << offset + pos << ": "
-                    << std::hex << std::setfill('0')
-                    << std::setw(2)
-                    << static_cast<int>(static_cast<unsigned char>(buf1[pos]))
-                    << " != "
-                    << std::setw(2)
-                    << static_cast<int>(static_cast<unsigned char>(buf2[pos]))
-                    << std::dec
-                    << BOOST_TEST_FLUSH
-                    ;
-                return;
-                }
-            }
-
-        offset += count;
-        }
-}
-
-// Macro allowing to easily pass the correct file name and line number to
-// check_files_equal().
-#define TEST_FILES_EQUAL(path1, path2) \
-    check_files_equal(path1, path2, __FILE__, __LINE__)
-
 } // Unnamed namespace.
 
 namespace
@@ -406,19 +315,37 @@ void test_save()
 {
     database qx_ins(qx_ins_path);
 
-    test_file_eraser erase_ndx("eraseme.ndx");
-    test_file_eraser erase_dat("eraseme.dat");
-    qx_ins.save("eraseme");
+    qx_ins.save("eraseme0");
 
-    TEST_FILES_EQUAL("eraseme.ndx", qx_ins_path + ".ndx");
-    TEST_FILES_EQUAL("eraseme.dat", qx_ins_path + ".dat");
+    bool okay_ndx0 = files_are_identical("eraseme0.ndx", qx_ins_path + ".ndx");
+    bool okay_dat0 = files_are_identical("eraseme0.dat", qx_ins_path + ".dat");
+    BOOST_TEST(okay_ndx0);
+    BOOST_TEST(okay_dat0);
 
-    database db_tmp("eraseme");
+    database db_tmp("eraseme0");
     BOOST_TEST_EQUAL(qx_ins.tables_count(), db_tmp.tables_count());
 
-    db_tmp.save("eraseme");
-    TEST_FILES_EQUAL("eraseme.ndx", qx_ins_path + ".ndx");
-    TEST_FILES_EQUAL("eraseme.dat", qx_ins_path + ".dat");
+    // File 'eraseme0.dat' is still open and cannot be removed yet.
+    // Saving 'db_tmp' closes the file so that it can be removed.
+    db_tmp.save("eraseme1");
+
+    // Leave the files for analysis if they didn't match.
+    if(okay_ndx0 && okay_dat0)
+        {
+        BOOST_TEST(0 == std::remove("eraseme0.ndx"));
+        BOOST_TEST(0 == std::remove("eraseme0.dat"));
+        }
+
+    bool okay_ndx1 = files_are_identical("eraseme1.ndx", qx_ins_path + ".ndx");
+    bool okay_dat1 = files_are_identical("eraseme1.dat", qx_ins_path + ".dat");
+    BOOST_TEST(okay_ndx1);
+    BOOST_TEST(okay_dat1);
+    // Leave the files for analysis if they didn't match.
+    if(okay_ndx1 && okay_dat1)
+        {
+        BOOST_TEST(0 == std::remove("eraseme1.ndx"));
+        BOOST_TEST(0 == std::remove("eraseme1.dat"));
+        }
 }
 
 void test_add_table()
@@ -497,10 +424,10 @@ void do_test_copy(std::string const& path)
     database db_new(index_ss, data_ss);
     BOOST_TEST_EQUAL(db_new.tables_count(), tables_count);
 
-    // In general, we can't just use TEST_FILES_EQUAL() to compare the files
-    // here because the order of tables in the original .dat file is lost and
-    // it does not need to be the same as the order in the index file, so we
-    // just compare the logical contents.
+    // Compare binary rate-table files logically rather than literally.
+    // These files are unlikely to be identical because the order of
+    // the tables in the original .dat file is lost and need not be the
+    // same as the order in the index file.
     for(int i = 0; i != tables_count; ++i)
         {
         BOOST_TEST_EQUAL