FORM Storage Tests (#354)

* Add a unit test requiring that FORM throws an exception when trying to read a data product from disk with a type that doesn't match the type on disk.
* Added test that two FORM AssociativeContainers can share the same Association.
* Added FORM test that three Associative_Containers can share the same Association.  
  - This models a physics algorithm in phlex writing two data products through FORM.
* Added FORM tests for error handling when setting up Containers.
* Made form::test::read<>() return std::unique_ptr<> instead of copies of dangling pointers.
* Updated (so-far-unused) form::test::getTechnology() to use form::technology::ROOT_TTREE for default instead of raw integers for better maintainability.
* Made all test_utils.hpp inline in case someone writes a test complicated enough that it gets included twice.
* Moved FORM's USE_ROOT_STORAGE compile definition to somewhere where it propagates automatically: the root_storage library itself. 
* Updated include guards in test_utils.hpp to match new convention on main.
* Removed unneeded assert()s from FORM Storage test.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Author: aolivier23

form/CMakeLists.txt

@@ -14,9 +14,6 @@ include_directories(${PROJECT_SOURCE_DIR}/form)
 
 # ROOT Storage toggle
 option(FORM_USE_ROOT_STORAGE "Enable ROOT Storage" ON)
-if(FORM_USE_ROOT_STORAGE)
-  add_definitions(-DUSE_ROOT_STORAGE)
-endif()
 
 # Add sub directories
 add_subdirectory(form)

form/root_storage/CMakeLists.txt

@@ -5,6 +5,7 @@ find_package(ROOT REQUIRED COMPONENTS Core RIO Tree)
 
 # Component(s) in the package:
 add_library(root_storage root_tfile.cpp root_ttree_container.cpp root_tbranch_container.cpp)
+target_compile_definitions(root_storage PUBLIC USE_ROOT_STORAGE)
 
 # Link the ROOT libraries
 target_link_libraries(root_storage PUBLIC ROOT::Core ROOT::RIO ROOT::Tree storage)

test/form/CMakeLists.txt

@@ -50,3 +50,9 @@ cet_test(form_basics_test USE_CATCH2_MAIN SOURCE form_basics_test.cpp LIBRARIES
          form
 )
 target_include_directories(form_basics_test PRIVATE ${PROJECT_SOURCE_DIR}/form)
+
+cet_test(form_storage_test USE_CATCH2_MAIN SOURCE form_storage_test.cpp LIBRARIES
+         root_storage
+         storage
+)
+target_include_directories(form_storage_test PRIVATE ${PROJECT_SOURCE_DIR}/form)

test/form/form_storage_test.cpp

@@ -0,0 +1,109 @@
+//Tests for FORM's storage layer's design requirements
+
+#include "test/form/test_utils.hpp"
+
+#include <catch2/catch_test_macros.hpp>
+
+#include <cmath>
+#include <numeric>
+#include <vector>
+
+using namespace form::detail::experimental;
+
+TEST_CASE("Storage_Container read wrong type", "[form]")
+{
+  int const technology = form::technology::ROOT_TTREE;
+  std::vector<int> primes = {2, 3, 5, 7, 11, 13, 17, 19};
+  form::test::write(technology, primes);
+
+  auto file = createFile(technology, form::test::testFileName, 'i');
+  auto container = createContainer(technology, form::test::makeTestBranchName<std::vector<int>>());
+  container->setFile(file);
+  void const* dataPtr;
+  CHECK_THROWS_AS(container->read(0, &dataPtr, typeid(double)), std::runtime_error);
+}
+
+TEST_CASE("Storage_Container sharing an Association", "[form]")
+{
+  int const technology = form::technology::ROOT_TTREE;
+  std::vector<float> piData(10, 3.1415927);
+  std::string indexData = "[EVENT=00000001;SEG=00000001]";
+
+  form::test::write(technology, piData, indexData);
+
+  auto [piResult, indexResult] = form::test::read<std::vector<float>, std::string>(technology);
+
+  float const originalSum = std::accumulate(piData.begin(), piData.end(), 0.f);
+  float const readSum = std::accumulate(piResult->begin(), piResult->end(), 0.f);
+  float const floatDiff = readSum - originalSum;
+
+  SECTION("float container sum") { CHECK(fabs(floatDiff) < std::numeric_limits<float>::epsilon()); }
+
+  SECTION("index") { CHECK(*indexResult == indexData); }
+}
+
+TEST_CASE("Storage_Container multiple containers in Association", "[form]")
+{
+  int const technology = form::technology::ROOT_TTREE;
+  std::vector<float> piData(10, 3.1415927);
+  std::vector<int> magicData(17);
+  std::iota(magicData.begin(), magicData.end(), 42);
+  std::string indexData = "[EVENT=00000001;SEG=00000001]";
+
+  form::test::write(technology, piData, magicData, indexData);
+
+  auto [piResult, magicResult, indexResult] =
+    form::test::read<std::vector<float>, std::vector<int>, std::string>(technology);
+
+  SECTION("float container")
+  {
+    float const originalSum = std::accumulate(piData.begin(), piData.end(), 0.f);
+    float const readSum = std::accumulate(piResult->begin(), piResult->end(), 0.f);
+    float const floatDiff = readSum - originalSum;
+    CHECK(fabs(floatDiff) < std::numeric_limits<float>::epsilon());
+  }
+
+  SECTION("int container")
+  {
+    int const originalMagic = std::accumulate(magicData.begin(), magicData.end(), 0);
+    int const readMagic = std::accumulate(magicResult->begin(), magicResult->end(), 0);
+    int const magicDiff = readMagic - originalMagic;
+    CHECK(magicDiff == 0);
+  }
+
+  SECTION("index data") { CHECK(*indexResult == indexData); }
+}
+
+TEST_CASE("FORM Container setup error handling")
+{
+  int const technology = form::technology::ROOT_TTREE;
+  auto file = createFile(technology, "testContainerErrorHandling.root", 'o');
+  auto container = createContainer(technology, "test/testData");
+
+  std::vector<float> testData;
+  void const* ptrTestData = &testData;
+  auto const& typeInfo = typeid(testData);
+
+  SECTION("fill() before setParent()")
+  {
+    CHECK_THROWS_AS(container->setupWrite(typeInfo), std::runtime_error);
+    CHECK_THROWS_AS(container->fill(ptrTestData), std::runtime_error);
+  }
+
+  SECTION("commit() before setParent()")
+  {
+    CHECK_THROWS_AS(container->commit(), std::runtime_error);
+  }
+
+  SECTION("read() before setParent()")
+  {
+    CHECK_THROWS_AS(container->read(0, &ptrTestData, typeInfo), std::runtime_error);
+  }
+
+  SECTION("mismatched file type")
+  {
+    std::shared_ptr<IStorage_File> wrongFile(
+      new Storage_File("testContainerErrorHandling.root", 'o'));
+    CHECK_THROWS_AS(container->setFile(wrongFile), std::runtime_error);
+  }
+}

test/form/test_utils.hpp

@@ -0,0 +1,127 @@
+//Utilities to make FORM unit tests easier to write and maintain
+
+#ifndef FORM_TEST_UTILS_HPP
+#define FORM_TEST_UTILS_HPP
+
+#include "storage/istorage.hpp"
+#include "storage/storage_associative_container.hpp"
+#include "util/factories.hpp"
+
+#include "TClass.h"
+
+#include <iostream>
+#include <memory>
+
+using namespace form::detail::experimental;
+
+namespace form::test {
+
+  inline std::string const testTreeName = "FORMTestTree";
+  inline std::string const testFileName = "FORMTestFile.root";
+
+  template <class PROD>
+  inline std::string getTypeName()
+  {
+    return TClass::GetClass<PROD>()->GetName();
+  }
+
+  template <class PROD>
+  inline std::string makeTestBranchName()
+  {
+    return testTreeName + "/" + getTypeName<PROD>();
+  }
+
+  inline std::vector<std::shared_ptr<IStorage_Container>> doWrite(
+    std::shared_ptr<IStorage_File>& /*file*/,
+    int const /*technology*/,
+    std::shared_ptr<IStorage_Container>& /*parent*/)
+  {
+    return std::vector<std::shared_ptr<IStorage_Container>>();
+  }
+
+  template <class PROD, class... PRODS>
+  inline std::vector<std::shared_ptr<IStorage_Container>> doWrite(
+    std::shared_ptr<IStorage_File>& file,
+    int const technology,
+    std::shared_ptr<IStorage_Container>& parent,
+    PROD& prod,
+    PRODS&... prods)
+  {
+    auto const branchName = makeTestBranchName<PROD>();
+    auto container = createContainer(technology, branchName);
+    auto assoc = dynamic_pointer_cast<Storage_Associative_Container>(container);
+    if (assoc) {
+      assoc->setParent(parent);
+    }
+    container->setFile(file);
+    container->setupWrite(typeid(PROD));
+
+    auto result = doWrite(file, technology, parent, prods...);
+    container->fill(&prod); //This must happen after setupWrite()
+    result.push_back(container);
+    return result;
+  }
+
+  template <class... PRODS>
+  inline void write(int const technology, PRODS&... prods)
+  {
+    auto file = createFile(technology, testFileName.c_str(), 'o');
+    auto parent = createAssociation(technology, testTreeName);
+    parent->setFile(file);
+    parent->setupWrite();
+
+    auto keepContainersAlive = doWrite(file, technology, parent, prods...);
+    keepContainersAlive.back()
+      ->commit(); //Elements are in reverse order of container construction, so this makes sure container owner calls commit()
+  }
+
+  template <class PROD>
+  inline std::unique_ptr<PROD const> doRead(std::shared_ptr<IStorage_File>& file,
+                                            int const technology,
+                                            std::shared_ptr<IStorage_Container>& parent)
+  {
+    auto container = createContainer(technology, makeTestBranchName<PROD>());
+    auto assoc = dynamic_pointer_cast<Storage_Associative_Container>(container);
+    if (assoc) {
+      assoc->setParent(parent);
+    }
+    container->setFile(file);
+    void const* dataPtr = new PROD();
+    void const** dataPtrPtr = &dataPtr;
+
+    if (!container->read(0, dataPtrPtr, typeid(PROD)))
+      throw std::runtime_error("Failed to read a " + getTypeName<PROD>());
+
+    return std::unique_ptr<PROD const>(static_cast<PROD const*>(dataPtr));
+  }
+
+  template <class... PRODS>
+  inline std::tuple<std::unique_ptr<PRODS const>...> read(int const technology)
+  {
+    auto file = createFile(technology, testFileName, 'i');
+    auto parent = createAssociation(technology, testTreeName);
+    parent->setFile(file);
+
+    return std::make_tuple(doRead<PRODS>(file, technology, parent)...);
+  }
+
+  inline int getTechnology(int const argc, char const** argv)
+  {
+    if (argc > 2 || (argc == 2 && (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help")))) {
+      std::cerr << "Expected exactly one argument, but got " << argc - 1 << "\n"
+                << "USAGE: testSchemaWriteOldProduct <technologyInt>\n";
+      return -1;
+    }
+
+    //Default to TTree with TFile
+    int technology = form::technology::ROOT_TTREE;
+
+    if (argc == 2)
+      technology = std::stoi(argv[1]);
+
+    return technology;
+  }
+
+} // namespace form::test
+
+#endif // FORM_TEST_UTILS_HPP