tweak the GlobalTimeCoordinator (#2450)

* tweak the GlobalTimeCoordinator a bit to try to resolve some lingering test issues

for more information, see https://pre-commit.ci

* remove cleanup statement from register functions

* update the concurrency library with delayedDestructor mods

* update concurrency library and debug some filterTests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix test fixtures for Cpp shared tests

* update concurrency library

* try running with much bigger timeout

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

.ci/azure-pipelines.yml

@@ -197,7 +197,7 @@ jobs:
       # -----------------------
       # Test HELICS
       # -----------------------
-      - bash: ctest --output-on-failure --timeout 480 -C Release -L "Continuous"
+      - bash: ctest --output-on-failure --timeout 1000 -C Release -L "Continuous"
         displayName: 'Test HELICS'
         workingDirectory: build
 

scripts/run-ci-tests.sh

@@ -95,11 +95,11 @@ else
             ;;
         *continuous*)
             TEST_CONFIG="Continuous"
-            CTEST_OPTIONS+=" --timeout 600"
+            CTEST_OPTIONS+=" --timeout 1000"
             ;;
         *ci*)
             TEST_CONFIG="Continuous"
-            CTEST_OPTIONS+=" --timeout 600"
+            CTEST_OPTIONS+=" --timeout 1000"
             ;;
         *)
             # Use whatever user gave for TEST_CONFIG

src/helics/core/CoreFactory.cpp

@@ -338,7 +338,6 @@ bool registerCore(const std::shared_ptr<Core>& core, CoreType type)
     if (core) {
         res = searchableCores.addObject(cname, core, type);
     }
-    cleanUpCores();
     if (res) {
         delayedDestroyer.addObjectsToBeDestroyed(core);
         addExtraTypes(cname, type);

src/helics/core/GlobalTimeCoordinator.cpp

@@ -88,6 +88,7 @@ bool GlobalTimeCoordinator::updateTimeFactors()
             nextEvent = findNextTriggerEvent(dependencies);
             ++sequenceCounter;
             auto trigTime = (nextEvent < cBigTime) ? nextEvent + Time::epsilon() : nextEvent;
+            mNewRequest = false;
             sendTimeUpdateRequest(trigTime);
             return true;
         }
@@ -102,8 +103,9 @@ bool GlobalTimeCoordinator::updateTimeFactors()
                     verified = dependencies.verifySequenceCounter(trigTime, sequenceCounter);
                 }
 
-                if (trig.first || !verified) {
+                if (trig.first || !verified || mNewRequest) {
                     ++sequenceCounter;
+                    mNewRequest = false;
                     sendTimeUpdateRequest(trigTime);
                     return true;
                 }
@@ -151,6 +153,15 @@ bool GlobalTimeCoordinator::updateTimeFactors()
     return true;
 }
 
+TimeProcessingResult GlobalTimeCoordinator::processTimeMessage(const ActionMessage& cmd)
+{
+    auto res = BaseTimeCoordinator::processTimeMessage(cmd);
+    if (res == TimeProcessingResult::PROCESSED_NEW_REQUEST) {
+        mNewRequest = true;
+    }
+    return res;
+}
+
 void GlobalTimeCoordinator::generateDebuggingTimeInfo(Json::Value& base) const
 {
     base["type"] = "global";

src/helics/core/GlobalTimeCoordinator.hpp

@@ -28,10 +28,12 @@ class GlobalTimeCoordinator: public BaseTimeCoordinator {
     Time currentMinTime{Time::minVal()};
     TimeState currentTimeState{TimeState::initialized};
     Time nextEvent{Time::maxVal()};
+    static constexpr std::int32_t mSequenceIncrement{100};
 
   protected:
     bool iterating{false};  //!< flag indicating that the min dependency is iterating
-
+    bool mNewRequest{
+        true};  //!< flag indicating a new request has been received since the last sequence Update
   public:
     GlobalTimeCoordinator() = default;
 
@@ -48,16 +50,13 @@ class GlobalTimeCoordinator: public BaseTimeCoordinator {
     void sendTimeUpdateRequest(Time triggerTime);
 
   public:
-    /** check if entry to the executing state can be granted*/
+    virtual TimeProcessingResult processTimeMessage(const ActionMessage& cmd) override;
     virtual MessageProcessingResult
         checkExecEntry(GlobalFederateId triggerFed = GlobalFederateId{}) override;
 
-    /** generate a string with the current time status*/
     virtual std::string printTimeStatus() const override;
-    /** generate debugging time information*/
     virtual void generateDebuggingTimeInfo(Json::Value& base) const override;
 
-    /** get the current next time*/
     virtual Time getNextTime() const override { return currentMinTime; }
 };
 }  // namespace helics

tests/helics/application_api/FilterAdditionalTests.cpp

@@ -158,6 +158,7 @@ endpoint only if condition matches.
 */
 TEST_P(filter_type_tests, message_reroute_filter_condition)
 {
+    debugDiagnostic = true;
     auto broker = AddBroker(GetParam(), 2);
     AddFederates<helics::MessageFederate>(GetParam(), 1, broker, 1.0, "filter");
     AddFederates<helics::MessageFederate>(GetParam(), 1, broker, 1.0, "message");

tests/helics/application_api/FilterTests.cpp

@@ -44,6 +44,7 @@ class filter: public ::testing::Test, public FederateTestFixture {};
 /** test registration of filters*/
 TEST_P(filter_single_type_test, message_filter_registration)
 {
+    debugDiagnostic = true;
     auto broker = AddBroker(GetParam(), 2);
 
     AddFederates<helics::MessageFederate>(GetParam(), 1, broker, helics::timeZero, "filter");
@@ -347,6 +348,7 @@ simulation
 
 TEST_P(filter_single_type_test, message_dest_filter_object)
 {
+    debugDiagnostic = true;
     auto broker = AddBroker(GetParam(), 2);
     AddFederates<helics::MessageFederate>(GetParam(), 1, broker, 1.0, "filter");
     AddFederates<helics::MessageFederate>(GetParam(), 1, broker, 1.0, "message");
@@ -531,6 +533,7 @@ TEST_P(filter_single_type_test, message_filter_function_two_stage)
 
 TEST_P(filter_single_type_test, message_filter_function_two_stage_endpoint_target)
 {
+    debugDiagnostic = true;
     auto broker = AddBroker(GetParam(), 3);
     AddFederates<helics::MessageFederate>(GetParam(), 1, broker, 1.0, "filter");
     AddFederates<helics::MessageFederate>(GetParam(), 1, broker, 1.0, "filter2");
@@ -559,6 +562,7 @@ TEST_P(filter_single_type_test, message_filter_function_two_stage_endpoint_targe
 
 TEST_F(filter, message_filter_function_two_stage_endpoint_target_alias)
 {
+    debugDiagnostic = true;
     auto broker = AddBroker("test", 3);
     AddFederates<helics::MessageFederate>("test", 1, broker, 1.0, "filter");
     AddFederates<helics::MessageFederate>("test", 1, broker, 1.0, "filter2");
@@ -589,6 +593,7 @@ TEST_F(filter, message_filter_function_two_stage_endpoint_target_alias)
 
 TEST_P(filter_single_type_test, message_filter_function_two_stage_endpoint_target_dest)
 {
+    debugDiagnostic = true;
     auto broker = AddBroker(GetParam(), 3);
     AddFederates<helics::MessageFederate>(GetParam(), 1, broker, 1.0, "filter");
     AddFederates<helics::MessageFederate>(GetParam(), 1, broker, 1.0, "filter2");

tests/helics/application_api/LoggingTests.cpp

@@ -872,6 +872,7 @@ TEST(logging, log_buffer_core2)
     EXPECT_GE(js2["logs"].size(), 1U);
     EXPECT_LE(js2["logs"].size(), 3U);
     cr.reset();
+    helics::cleanupHelicsLibrary();
 }
 
 TEST(logging, remote_log_broker)
@@ -906,6 +907,7 @@ TEST(logging, remote_log_broker)
     }
     llock.unlock();
     EXPECT_GT(remote_cnt, 0);
+    helics::cleanupHelicsLibrary();
 }
 
 TEST(logging, remote_log_fed)

tests/helics/application_api/MessageFederateAdditionalTests.cpp

@@ -112,6 +112,7 @@ TEST_P(mfed_add_single_type_tests, endpoint_registration_objs)
 
 TEST_P(mfed_add_single_type_tests, send_receive_callback)
 {
+    debugDiagnostic = true;
     SetupTest<helics::MessageFederate>(GetParam(), 1);
     auto mFed1 = GetFederateAs<helics::MessageFederate>(0);
 

tests/helics/application_api/testFixtures.cpp

@@ -92,9 +92,16 @@ FederateTestFixture::~FederateTestFixture()
     }
     federates.clear();
     if (debugDiagnostic) {
-        std::cout << "cleanup" << std::endl;
+        using namespace std::chrono_literals;  // NOLINT
+        std::cout << "cleanup1" << std::endl;
+        helics::BrokerFactory::cleanUpBrokers(100ms);
+        std::cout << "cleanup2" << std::endl;
+        helics::CoreFactory::cleanUpCores(200ms);
+        std::cout << "cleanup3" << std::endl;
+        helics::BrokerFactory::cleanUpBrokers(100ms);
+    } else {
+        helics::cleanupHelicsLibrary();
     }
-    helics::cleanupHelicsLibrary();
     if (debugDiagnostic) {
         std::cout << "finished" << std::endl;
     }

tests/helics/application_api/testFixtures_shared.hpp

@@ -215,4 +215,5 @@ struct FederateTestFixture {
     std::string extraBrokerArgs;
     std::string extraFederateArgs;
     std::string ctype;
+    bool debugDiagnostic{false};
 };