Fixing tests and improving coverage (#188)

* Fix infrastructure generator test

* Add a message passing test of a full QC chain

* Improve test coverage of ObjectsManager

* Improve test coverage of TaskInterface

* Correct test modules' names

* Test of CheckInterface

* Add a basic test of a message coming from Checker

* TaskRunner and TaskRunnerFactory test

* Checker and CheckerFactory test

* Rename testInfrastractureGenerator.json to testSharedConfig.json

* Add argument -b for testWorkflow

* fix warning and add TODO

* Pass --run to testWorkflow

Author: knopers8

Framework/CMakeLists.txt

@@ -224,19 +224,33 @@ set(
   test/testPublisher.cxx
   test/testQcInfoLogger.cxx
   test/testInfrastructureGenerator.cxx
-  test/testQCTask.cxx
+  test/testTaskInterface.cxx
+  test/testTaskRunner.cxx
+  test/testCheckInterface.cxx
+  test/testChecker.cxx
   test/testQuality.cxx
   test/testObjectsManager.cxx
   test/testCcdbDatabase.cxx
+  test/testWorkflow.cxx
 )
 
-foreach(test ${TEST_SRCS})
+set(
+  TEST_ARGS
+  "" "" "" "" "" "" "" "" "" "" "" "" "-b --run"
+)
+
+list(LENGTH TEST_SRCS count)
+math(EXPR count "${count}-1")
+foreach(i RANGE ${count})
+  list(GET TEST_SRCS ${i} test)
+  list(GET TEST_ARGS ${i} arg)
   get_filename_component(test_name ${test} NAME)
   string(REGEX REPLACE ".cxx" "" test_name ${test_name})
+  string(REPLACE " " ";" arg "${arg}") # make list of string (arguments) out of one string
 
   add_executable(${test_name} ${test})
   target_link_libraries(${test_name} PRIVATE QualityControl Boost::unit_test_framework )
-  add_test(NAME ${test_name} COMMAND ${test_name})
+  add_test(NAME ${test_name} COMMAND ${test_name} ${arg})
   set_tests_properties(${test_name} PROPERTIES TIMEOUT 30)
   install(TARGETS ${test_name} DESTINATION tests)
 endforeach()
@@ -330,6 +344,7 @@ install(
 
 install(
   FILES
-  test/testQCFactory.json
+    test/testSharedConfig.json
+    test/testWorkflow.json
   DESTINATION tests
-)
+)

Framework/include/QualityControl/TaskConfig.h

@@ -30,7 +30,7 @@ struct TaskConfig {
   int cycleDurationSeconds;
   int maxNumberCycles;
   std::string consulUrl;
-  o2::configuration::KeyValueMap customParameters;
+  o2::configuration::KeyValueMap customParameters = {};
 };
 
 } // namespace o2::quality_control::core

Framework/src/Checker.cxx

@@ -144,6 +144,9 @@ void Checker::run(framework::ProcessingContext& ctx)
 
 o2::header::DataDescription Checker::createCheckerDataDescription(const std::string taskName)
 {
+  if (taskName.empty()) {
+    BOOST_THROW_EXCEPTION(FatalException() << errinfo_details("Empty taskName for checker's data description"));
+  }
   o2::header::DataDescription description;
   description.runtimeInit(std::string(taskName.substr(0, o2::header::DataDescription::size - 4) + "-chk").c_str());
   return description;

Framework/src/TaskRunner.cxx

@@ -41,6 +41,7 @@ using namespace o2::header;
 using namespace o2::configuration;
 using namespace o2::monitoring;
 using namespace std::chrono;
+using namespace AliceO2::Common;
 
 TaskRunner::TaskRunner(const std::string& taskName, const std::string& configurationSource, size_t id)
   : mDeviceName(createTaskRunnerIdString() + "-" + taskName),
@@ -177,6 +178,9 @@ header::DataOrigin TaskRunner::createTaskDataOrigin()
 
 header::DataDescription TaskRunner::createTaskDataDescription(const std::string& taskName)
 {
+  if (taskName.empty()) {
+    BOOST_THROW_EXCEPTION(FatalException() << errinfo_details("Empty taskName for task's data description"));
+  }
   o2::header::DataDescription description;
   description.runtimeInit(std::string(taskName.substr(0, header::DataDescription::size - 3) + "-mo").c_str());
   return description;

Framework/test/testCheckInterface.cxx

@@ -0,0 +1,96 @@
+// Copyright CERN and copyright holders of ALICE O2. This software is
+// distributed under the terms of the GNU General Public License v3 (GPL
+// Version 3), copied verbatim in the file "COPYING".
+//
+// See http://alice-o2.web.cern.ch/license for full licensing information.
+//
+// In applying this license CERN does not waive the privileges and immunities
+// granted to it by virtue of its status as an Intergovernmental Organization
+// or submit itself to any jurisdiction.
+
+///
+/// \file    testCheckInterface.cxx
+/// \author  Piotr Konopka
+///
+
+#include "QualityControl/CheckInterface.h"
+
+#define BOOST_TEST_MODULE CheckInterface test
+#define BOOST_TEST_MAIN
+#define BOOST_TEST_DYN_LINK
+
+#include <boost/test/unit_test.hpp>
+#include <iostream>
+#include <TObjString.h>
+#include <string>
+
+using namespace o2::quality_control;
+using namespace std;
+
+namespace o2::quality_control
+{
+
+using namespace core;
+
+namespace test
+{
+
+class TestCheck : public checker::CheckInterface
+{
+ public:
+  /// Default constructor
+  TestCheck() = default;
+  /// Destructor
+  ~TestCheck() override = default;
+
+  // Override interface
+  void configure(std::string name) override
+  {
+    mValidString = name;
+  }
+  Quality check(const MonitorObject* mo) override
+  {
+    if (mValidString.empty()) {
+      return Quality::Null;
+    }
+
+    TObjString* str = reinterpret_cast<TObjString*>(mo->getObject());
+    return mValidString == str->String() ? Quality::Good : Quality::Bad;
+  }
+
+  void beautify(MonitorObject* mo, Quality = Quality::Null) override
+  {
+    TObjString* str = reinterpret_cast<TObjString*>(mo->getObject());
+    str->String().Append(" is beautiful now");
+  }
+
+  std::string getAcceptedType() override
+  {
+    return "TObjString";
+  }
+
+  string mValidString;
+};
+
+} /* namespace test */
+} /* namespace o2::quality_control */
+
+BOOST_AUTO_TEST_CASE(test_invoke_all_methods)
+{
+  test::TestCheck testCheck;
+
+  MonitorObject mo(new TObjString("A string"), "str");
+
+  BOOST_CHECK_EQUAL(testCheck.check(&mo), Quality::Null);
+
+  testCheck.configure("A different string");
+  BOOST_CHECK_EQUAL(testCheck.check(&mo), Quality::Bad);
+
+  testCheck.configure("A string");
+  BOOST_CHECK_EQUAL(testCheck.check(&mo), Quality::Good);
+
+  testCheck.beautify(&mo);
+  BOOST_CHECK_EQUAL(reinterpret_cast<TObjString*>(mo.getObject())->String(), "A string is beautiful now");
+
+  BOOST_CHECK_EQUAL(testCheck.getAcceptedType(), "TObjString");
+}

Framework/test/testChecker.cxx

@@ -0,0 +1,71 @@
+// Copyright CERN and copyright holders of ALICE O2. This software is
+// distributed under the terms of the GNU General Public License v3 (GPL
+// Version 3), copied verbatim in the file "COPYING".
+//
+// See http://alice-o2.web.cern.ch/license for full licensing information.
+//
+// In applying this license CERN does not waive the privileges and immunities
+// granted to it by virtue of its status as an Intergovernmental Organization
+// or submit itself to any jurisdiction.
+
+///
+/// \file    testChecker.cxx
+/// \author  Piotr Konopka
+///
+
+#include "QualityControl/CheckerFactory.h"
+#include "QualityControl/Checker.h"
+#include <Framework/DataSampling.h>
+#include <Framework/DataSpecUtils.h>
+
+#define BOOST_TEST_MODULE Checker test
+#define BOOST_TEST_MAIN
+#define BOOST_TEST_DYN_LINK
+
+#include <boost/test/unit_test.hpp>
+#include <iostream>
+
+using namespace o2::quality_control::checker;
+using namespace std;
+using namespace o2::framework;
+using namespace o2::header;
+
+BOOST_AUTO_TEST_CASE(test_checker_factory)
+{
+  std::string configFilePath =
+    std::string("json:/") + getenv("QUALITYCONTROL_ROOT") + "/tests/testSharedConfig.json";
+
+  CheckerFactory checkerFactory;
+  DataProcessorSpec checker = checkerFactory.create("abcChecker", "abcTask", configFilePath);
+
+  BOOST_CHECK_EQUAL(checker.name, "abcChecker");
+
+  BOOST_REQUIRE_EQUAL(checker.inputs.size(), 1);
+  BOOST_CHECK_EQUAL(checker.inputs[0], (InputSpec{ { "mo" }, "QC", "abcTask-mo", 0 }));
+
+  BOOST_REQUIRE_EQUAL(checker.outputs.size(), 1);
+  BOOST_CHECK_EQUAL(checker.outputs[0], (OutputSpec{ "QC", "abcTask-chk", 0 }));
+
+  BOOST_CHECK(checker.algorithm.onInit != nullptr);
+}
+
+BOOST_AUTO_TEST_CASE(test_checker_static)
+{
+  BOOST_CHECK(Checker::createCheckerDataDescription("qwertyuiop") == DataDescription("qwertyuiop-chk"));
+  BOOST_CHECK(Checker::createCheckerDataDescription("012345678901234567890") == DataDescription("012345678901-chk"));
+  BOOST_CHECK_THROW(Checker::createCheckerDataDescription(""), AliceO2::Common::FatalException);
+}
+
+BOOST_AUTO_TEST_CASE(test_checker)
+{
+  std::string configFilePath =
+    std::string("json:/") + getenv("QUALITYCONTROL_ROOT") + "/tests/testSharedConfig.json";
+
+  Checker checker{ "abcChecker", "abcTask", configFilePath };
+
+  BOOST_CHECK_EQUAL(checker.getInputSpec(), (InputSpec{ { "mo" }, "QC", "abcTask-mo", 0 }));
+  BOOST_CHECK_EQUAL(checker.getOutputSpec(), (OutputSpec{ "QC", "abcTask-chk", 0 }));
+
+  // This is maximum that we can do until we are able to test the DPL algorithms in isolation.
+  // TODO: When it is possible, we should try calling run() and init()
+}

Framework/test/testDbFactory.cxx

@@ -20,7 +20,7 @@
 #include "QualityControl/MySqlDatabase.h"
 
 #endif
-#define BOOST_TEST_MODULE Quality test
+#define BOOST_TEST_MODULE DbFactory test
 #define BOOST_TEST_MAIN
 #define BOOST_TEST_DYN_LINK
 

Framework/test/testInfrastructureGenerator.cxx

@@ -13,7 +13,7 @@
 /// \author  Piotr Konopka
 ///
 
-#define BOOST_TEST_MODULE QCFactory test
+#define BOOST_TEST_MODULE InfrastructureGenerator test
 #define BOOST_TEST_MAIN
 #define BOOST_TEST_DYN_LINK
 
@@ -30,15 +30,15 @@ using namespace o2::framework;
 BOOST_AUTO_TEST_CASE(qc_factory_local_test)
 {
   BOOST_REQUIRE_NE(getenv("QUALITYCONTROL_ROOT"), nullptr);
-  std::string configFilePath = std::string("json:/") + getenv("QUALITYCONTROL_ROOT") + "/tests/testQCFactory.json";
+  std::string configFilePath = std::string("json:/") + getenv("QUALITYCONTROL_ROOT") + "/tests/testSharedConfig.json";
 
   {
     auto workflow = InfrastructureGenerator::generateLocalInfrastructure(configFilePath, "o2flp1");
 
     BOOST_REQUIRE_EQUAL(workflow.size(), 1);
 
     BOOST_CHECK_EQUAL(workflow[0].name, "QC-TASK-RUNNER-skeletonTask");
-    BOOST_CHECK_EQUAL(workflow[0].inputs.size(), 1);
+    BOOST_CHECK_EQUAL(workflow[0].inputs.size(), 2);
     BOOST_CHECK_EQUAL(workflow[0].outputs.size(), 1);
     BOOST_CHECK_EQUAL(DataSpecUtils::getOptionalSubSpec(workflow[0].outputs[0]).value_or(-1), 1);
   }
@@ -49,7 +49,7 @@ BOOST_AUTO_TEST_CASE(qc_factory_local_test)
     BOOST_REQUIRE_EQUAL(workflow.size(), 1);
 
     BOOST_CHECK_EQUAL(workflow[0].name, "QC-TASK-RUNNER-skeletonTask");
-    BOOST_CHECK_EQUAL(workflow[0].inputs.size(), 1);
+    BOOST_CHECK_EQUAL(workflow[0].inputs.size(), 2);
     BOOST_CHECK_EQUAL(workflow[0].outputs.size(), 1);
     BOOST_CHECK_EQUAL(DataSpecUtils::getOptionalSubSpec(workflow[0].outputs[0]).value_or(-1), 2);
   }
@@ -63,7 +63,7 @@ BOOST_AUTO_TEST_CASE(qc_factory_local_test)
 
 BOOST_AUTO_TEST_CASE(qc_factory_remote_test)
 {
-  std::string configFilePath = std::string("json:/") + getenv("QUALITYCONTROL_ROOT") + "/tests/testQCFactory.json";
+  std::string configFilePath = std::string("json:/") + getenv("QUALITYCONTROL_ROOT") + "/tests/testSharedConfig.json";
   auto workflow = InfrastructureGenerator::generateRemoteInfrastructure(configFilePath);
 
   // the infrastructure should consist of a merger and checker for the 'skeletonTask' (its taskRunner is declared to be
@@ -93,8 +93,8 @@ BOOST_AUTO_TEST_CASE(qc_factory_remote_test)
   auto taskRunnerAbcTask = std::find_if(
     workflow.begin(), workflow.end(),
     [](const DataProcessorSpec& d) {
-      return d.name == "abcTask" &&
-             d.inputs.size() == 1 &&
+      return d.name == "QC-TASK-RUNNER-abcTask" &&
+             d.inputs.size() == 2 &&
              d.outputs.size() == 1;
     });
   BOOST_CHECK(taskRunnerAbcTask != workflow.end());

Framework/test/testObjectsManager.cxx

@@ -15,11 +15,12 @@
 
 #include "QualityControl/ObjectsManager.h"
 
-#define BOOST_TEST_MODULE Publisher test
+#define BOOST_TEST_MODULE ObjectManager test
 #define BOOST_TEST_MAIN
 #define BOOST_TEST_DYN_LINK
 #include "../include/Common/Exceptions.h"
 #include <TObjString.h>
+#include <TH1F.h>
 #include <boost/test/unit_test.hpp>
 #include <iostream>
 
@@ -67,4 +68,53 @@ BOOST_AUTO_TEST_CASE(unpublish_test)
   BOOST_CHECK_THROW(objectsManager.stopPublishing("asdf"), ObjectNotFoundError);
 }
 
+BOOST_AUTO_TEST_CASE(getters_test)
+{
+  TaskConfig config;
+  config.taskName = "test";
+  config.consulUrl = "http://consul-test.cern.ch:8500";
+  ObjectsManager objectsManager(config);
+
+  TObjString s("content");
+  TH1F h("histo", "h", 100, 0, 99);
+
+  objectsManager.startPublishing(&s);
+  objectsManager.startPublishing(&h);
+
+  // basic gets
+  BOOST_CHECK_NO_THROW(objectsManager.getMonitorObject("content"));
+  BOOST_CHECK_NO_THROW(objectsManager.getMonitorObject("histo"));
+  BOOST_CHECK_THROW(objectsManager.getMonitorObject("unexisting object"), ObjectNotFoundError);
+  BOOST_CHECK_NO_THROW(objectsManager.getObject("content"));
+  BOOST_CHECK_NO_THROW(objectsManager.getObject("histo"));
+  BOOST_CHECK_THROW(objectsManager.getObject("unexisting object"), ObjectNotFoundError);
+
+  // non owning array
+  TObjArray* array = objectsManager.getNonOwningArray();
+  BOOST_CHECK_EQUAL(array->GetEntries(), 2);
+  BOOST_CHECK(array->FindObject("content") != nullptr);
+  BOOST_CHECK(array->FindObject("histo") != nullptr);
+
+  // we confirm that deleting the array does not delete objects
+  delete array;
+  BOOST_CHECK_NO_THROW(objectsManager.getMonitorObject("content"));
+  BOOST_CHECK_NO_THROW(objectsManager.getMonitorObject("histo"));
+}
+
+BOOST_AUTO_TEST_CASE(metadata_test)
+{
+  TaskConfig config;
+  config.taskName = "test";
+  config.consulUrl = "http://consul-test.cern.ch:8500";
+  ObjectsManager objectsManager(config);
+
+  TObjString s("content");
+  TH1F h("histo", "h", 100, 0, 99);
+  objectsManager.startPublishing(&s);
+  objectsManager.startPublishing(&h);
+
+  objectsManager.addMetadata("content", "aaa", "bbb");
+  BOOST_CHECK_EQUAL(objectsManager.getMonitorObject("content")->getMetadataMap()["aaa"], "bbb");
+}
+
 } // namespace o2::quality_control::core