Fix fragile time-based waiting in integration tests

tests/integrationtests/AdaptorAndProxy_test.cpp

@@ -66,7 +66,7 @@ public:
         s_connection->releaseName(INTERFACE_NAME);
     }
 
-    static void waitUntil(std::atomic<bool>& flag, std::chrono::milliseconds timeout = 1s)
+    static bool waitUntil(std::atomic<bool>& flag, std::chrono::milliseconds timeout = 5s)
     {
         std::chrono::milliseconds elapsed{};
         std::chrono::milliseconds step{5ms};
@@ -74,8 +74,10 @@ public:
             std::this_thread::sleep_for(step);
             elapsed += step;
             if (elapsed > timeout)
-                throw std::runtime_error("Waiting timed out");
+                return false;
         } while (!flag);
+
+        return true;
     }
 
 private:
@@ -216,14 +218,8 @@ TEST_F(SdbusTestObject, CallsMultiplyMethodWithNoReplyFlag)
 {
     m_proxy->multiplyWithNoReply(INT64_VALUE, DOUBLE_VALUE);
 
-    for (auto i = 0; i < 100; ++i)
+    ASSERT_TRUE(waitUntil(m_adaptor->m_wasMultiplyCalled));
-    {
+    ASSERT_THAT(m_adaptor->m_multiplyResult, Eq(INT64_VALUE * DOUBLE_VALUE));
-        if (m_adaptor->wasMultiplyCalled())
-            break;
-        std::this_thread::sleep_for(10ms);
-    }
-    ASSERT_TRUE(m_adaptor->wasMultiplyCalled());
-    ASSERT_THAT(m_adaptor->getMultiplyResult(), Eq(INT64_VALUE * DOUBLE_VALUE));
 }
 
 TEST_F(SdbusTestObject, CallsMethodThatThrowsError)
@@ -248,13 +244,7 @@ TEST_F(SdbusTestObject, CallsErrorThrowingMethodWithDontExpectReplySet)
 {
     ASSERT_NO_THROW(m_proxy->throwErrorWithNoReply());
 
-    for (auto i = 0; i < 100; ++i)
+    ASSERT_TRUE(waitUntil(m_adaptor->m_wasThrowErrorCalled));
-    {
-        if (m_adaptor->wasThrowErrorCalled())
-            break;
-        std::this_thread::sleep_for(10ms);
-    }
-    ASSERT_TRUE(m_adaptor->wasThrowErrorCalled());
 }
 
 TEST_F(SdbusTestObject, RunsServerSideAsynchoronousMethodAsynchronously)
@@ -372,39 +362,35 @@ TEST_F(SdbusTestObject, FailsCallingMethodOnNonexistentObject)
 
 TEST_F(SdbusTestObject, EmitsSimpleSignalSuccesfully)
 {
-    auto count = m_proxy->getSimpleCallCount();
+    m_adaptor->emitSimpleSignal();
-    m_adaptor->simpleSignal();
-    usleep(10000);
 
-    ASSERT_THAT(m_proxy->getSimpleCallCount(), Eq(count + 1));
+    ASSERT_TRUE(waitUntil(m_proxy->m_gotSimpleSignal));
 }
 
 TEST_F(SdbusTestObject, EmitsSignalWithMapSuccesfully)
 {
-    m_adaptor->signalWithMap({{0, "zero"}, {1, "one"}});
+    m_adaptor->emitSignalWithMap({{0, "zero"}, {1, "one"}});
-    usleep(10000);
 
-    auto map = m_proxy->getMap();
+    ASSERT_TRUE(waitUntil(m_proxy->m_gotSignalWithMap));
-    ASSERT_THAT(map[0], Eq("zero"));
+    ASSERT_THAT(m_proxy->m_mapFromSignal[0], Eq("zero"));
-    ASSERT_THAT(map[1], Eq("one"));
+    ASSERT_THAT(m_proxy->m_mapFromSignal[1], Eq("one"));
 }
 
 TEST_F(SdbusTestObject, EmitsSignalWithVariantSuccesfully)
 {
     double d = 3.14;
-    m_adaptor->signalWithVariant(3.14);
+    m_adaptor->emitSignalWithVariant(d);
-    usleep(10000);
 
-    ASSERT_THAT(m_proxy->getVariantValue(), d);
+    ASSERT_TRUE(waitUntil(m_proxy->m_gotSignalWithVariant));
+    ASSERT_THAT(m_proxy->m_variantFromSignal, d);
 }
 
 TEST_F(SdbusTestObject, EmitsSignalWithoutRegistrationSuccesfully)
 {
-    m_adaptor->signalWithoutRegistration({"platform", {"av"}});
+    m_adaptor->emitSignalWithoutRegistration({"platform", {"av"}});
-    usleep(10000);
 
-    auto signature = m_proxy->getSignatureFromSignal();
+    ASSERT_TRUE(waitUntil(m_proxy->m_gotSignalWithSignature));
-    ASSERT_THAT(signature["platform"], Eq("av"));
+    ASSERT_THAT(m_proxy->m_signatureFromSignal["platform"], Eq("av"));
 }
 
 // Properties
@@ -486,7 +472,7 @@ TEST_F(SdbusTestObject, EmitsPropertyChangedSignalForSelectedProperties)
     m_proxy->action(DEFAULT_ACTION_VALUE*2);
     m_adaptor->emitPropertiesChangedSignal(INTERFACE_NAME, {"blocking"});
 
-    waitUntil(signalReceived);
+    ASSERT_TRUE(waitUntil(signalReceived));
 }
 
 TEST_F(SdbusTestObject, EmitsPropertyChangedSignalForAllProperties)
@@ -506,7 +492,7 @@ TEST_F(SdbusTestObject, EmitsPropertyChangedSignalForAllProperties)
 
     m_adaptor->emitPropertiesChangedSignal(INTERFACE_NAME);
 
-    waitUntil(signalReceived);
+    ASSERT_TRUE(waitUntil(signalReceived));
 }
 
 TEST_F(SdbusTestObject, DoesNotProvideObjectManagerInterfaceByDefault)
@@ -563,7 +549,7 @@ TEST_F(SdbusTestObject, EmitsInterfacesAddedSignalForSelectedObjectInterfaces)
 
     m_adaptor->emitInterfacesAddedSignal({INTERFACE_NAME});
 
-    waitUntil(signalReceived);
+    ASSERT_TRUE(waitUntil(signalReceived));
 }
 
 TEST_F(SdbusTestObject, EmitsInterfacesAddedSignalForAllObjectInterfaces)
@@ -581,7 +567,7 @@ TEST_F(SdbusTestObject, EmitsInterfacesAddedSignalForAllObjectInterfaces)
 
     m_adaptor->emitInterfacesAddedSignal();
 
-    waitUntil(signalReceived);
+    ASSERT_TRUE(waitUntil(signalReceived));
 }
 
 TEST_F(SdbusTestObject, EmitsInterfacesRemovedSignalForSelectedObjectInterfaces)
@@ -599,7 +585,7 @@ TEST_F(SdbusTestObject, EmitsInterfacesRemovedSignalForSelectedObjectInterfaces)
 
     m_adaptor->emitInterfacesRemovedSignal({INTERFACE_NAME});
 
-    waitUntil(signalReceived);
+    ASSERT_TRUE(waitUntil(signalReceived));
 }
 
 TEST_F(SdbusTestObject, EmitsInterfacesRemovedSignalForAllObjectInterfaces)
@@ -616,5 +602,5 @@ TEST_F(SdbusTestObject, EmitsInterfacesRemovedSignalForAllObjectInterfaces)
 
     m_adaptor->emitInterfacesRemovedSignal();
 
-    waitUntil(signalReceived);
+    ASSERT_TRUE(waitUntil(signalReceived));
 }

tests/integrationtests/TestingAdaptor.h

@@ -47,24 +47,31 @@ public:
         unregisterAdaptor();
     }
 
-    bool wasMultiplyCalled() const { return m_multiplyCalled; }
-    double getMultiplyResult() const { return m_multiplyResult; }
-    bool wasThrowErrorCalled() const { return m_throwErrorCalled; }
-
 protected:
 
-    void noArgNoReturn() const {}
+    void noArgNoReturn() const
+    {
+    }
 
-    int32_t getInt() const { return INT32_VALUE; }
+    int32_t getInt() const
+    {
+        return INT32_VALUE;
+    }
 
-    std::tuple<uint32_t, std::string> getTuple() const { return std::make_tuple(UINT32_VALUE, STRING_VALUE); }
+    std::tuple<uint32_t, std::string> getTuple() const
+    {
+        return std::make_tuple(UINT32_VALUE, STRING_VALUE);
+    }
 
-    double multiply(const int64_t& a, const double& b) const { return a * b; }
+    double multiply(const int64_t& a, const double& b) const
+    {
+        return a * b;
+    }
 
     void multiplyWithNoReply(const int64_t& a, const double& b) const
     {
         m_multiplyResult = a * b;
-        m_multiplyCalled = true;
+        m_wasMultiplyCalled = true;
     }
 
     std::vector<int16_t> getInts16FromStruct(const sdbus::Struct<uint8_t, int16_t, double, std::string, std::vector<int16_t>>& x) const
@@ -142,8 +149,14 @@ protected:
         }
     }
 
-    sdbus::Signature getSignature() const { return SIGNATURE_VALUE; }
+    sdbus::Signature getSignature() const
-    sdbus::ObjectPath getObjectPath() const { return OBJECT_PATH_VALUE; }
+    {
+        return SIGNATURE_VALUE;
+    }
+    sdbus::ObjectPath getObjectPath() const
+    {
+        return OBJECT_PATH_VALUE;
+    }
 
     ComplexType getComplex() const
     {
@@ -175,25 +188,45 @@ protected:
 
     void throwError() const
     {
-        m_throwErrorCalled = true;
+        m_wasThrowErrorCalled = true;
         throw sdbus::createError(1, "A test error occurred");
     }
 
-    std::string state() { return m_state; }
+    std::string state()
-    uint32_t action() { return m_action; }
+    {
-    void action(const uint32_t& value) { m_action = value; }
+        return m_state;
-    bool blocking() { return m_blocking; }
+    }
-    void blocking(const bool& value) { m_blocking = value; }
+
+    uint32_t action()
+    {
+        return m_action;
+    }
+
+    void action(const uint32_t& value)
+    {
+        m_action = value;
+    }
+
+    bool blocking()
+    {
+        return m_blocking;
+    }
+
+    void blocking(const bool& value)
+    {
+        m_blocking = value;
+    }
 
 private:
     const std::string m_state{DEFAULT_STATE_VALUE};
     uint32_t m_action{DEFAULT_ACTION_VALUE};
     bool m_blocking{DEFAULT_BLOCKING_VALUE};
 
+public: // for tests
     // For dont-expect-reply method call verifications
-    mutable std::atomic<bool> m_multiplyCalled{};
+    mutable std::atomic<bool> m_wasMultiplyCalled{};
     mutable double m_multiplyResult{};
-    mutable std::atomic<bool> m_throwErrorCalled{};
+    mutable std::atomic<bool> m_wasThrowErrorCalled{};
 };
 
 

tests/integrationtests/TestingProxy.h

@@ -27,6 +27,7 @@
 #define SDBUS_CPP_INTEGRATIONTESTS_TESTINGPROXY_H_
 
 #include "proxy-glue.h"
+#include <atomic>
 
 class TestingProxy : public sdbus::ProxyInterfaces< ::testing_proxy
                                                   , sdbus::Peer_proxy
@@ -46,29 +47,33 @@ public:
         unregisterProxy();
     }
 
-    int getSimpleCallCount() const { return m_simpleCallCounter; }
-    std::map<int32_t, std::string> getMap() const { return m_map; }
-    double getVariantValue() const { return m_variantValue; }
-    std::map<std::string, std::string> getSignatureFromSignal() const { return m_signature; }
-
     void installDoOperationClientSideAsyncReplyHandler(std::function<void(uint32_t res, const sdbus::Error* err)> handler)
     {
         m_DoOperationClientSideAsyncReplyHandler = handler;
     }
 
 protected:
-    void onSimpleSignal() override { ++m_simpleCallCounter; }
+    void onSimpleSignal() override
+    {
+        m_gotSimpleSignal = true;
+    }
 
-    void onSignalWithMap(const std::map<int32_t, std::string>& m) override { m_map = m; }
+    void onSignalWithMap(const std::map<int32_t, std::string>& m) override
+    {
+        m_mapFromSignal = m;
+        m_gotSignalWithMap = true;
+    }
 
     void onSignalWithVariant(const sdbus::Variant& v) override
     {
-        m_variantValue = v.get<double>();
+        m_variantFromSignal = v.get<double>();
+        m_gotSignalWithVariant = true;
     }
 
     void onSignalWithoutRegistration(const sdbus::Struct<std::string, sdbus::Struct<sdbus::Signature>>& s) override
     {
-        m_signature[std::get<0>(s)] = static_cast<std::string>(std::get<0>(std::get<1>(s)));
+        m_signatureFromSignal[std::get<0>(s)] = static_cast<std::string>(std::get<0>(std::get<1>(s)));
+        m_gotSignalWithSignature = true;
     }
 
     void onDoOperationReply(uint32_t returnValue, const sdbus::Error* error) override
@@ -101,11 +106,14 @@ protected:
     }
 
 //private:
-public:
+public: // for tests
-    int m_simpleCallCounter{};
+    std::atomic<bool> m_gotSimpleSignal;
-    std::map<int32_t, std::string> m_map;
+    std::atomic<bool> m_gotSignalWithMap;
-    double m_variantValue;
+    std::map<int32_t, std::string> m_mapFromSignal;
-    std::map<std::string, std::string> m_signature;
+    std::atomic<bool> m_gotSignalWithVariant;
+    double m_variantFromSignal;
+    std::atomic<bool> m_gotSignalWithSignature;
+    std::map<std::string, std::string> m_signatureFromSignal;
 
     std::function<void(uint32_t res, const sdbus::Error* err)> m_DoOperationClientSideAsyncReplyHandler;
     std::function<void(const std::string&, const std::map<std::string, sdbus::Variant>&, const std::vector<std::string>&)> m_onPropertiesChangedHandler;

tests/integrationtests/adaptor-glue.h

@@ -118,22 +118,22 @@ protected:
     }
 
 public:
-    void simpleSignal()
+    void emitSimpleSignal()
     {
         object_.emitSignal("simpleSignal").onInterface(INTERFACE_NAME);
     }
 
-    void signalWithMap(const std::map<int32_t, std::string>& map)
+    void emitSignalWithMap(const std::map<int32_t, std::string>& map)
     {
         object_.emitSignal("signalWithMap").onInterface(INTERFACE_NAME).withArguments(map);
     }
 
-    void signalWithVariant(const sdbus::Variant& v)
+    void emitSignalWithVariant(const sdbus::Variant& v)
     {
         object_.emitSignal("signalWithVariant").onInterface(INTERFACE_NAME).withArguments(v);
     }
 
-    void signalWithoutRegistration(const sdbus::Struct<std::string, sdbus::Struct<sdbus::Signature>>& s)
+    void emitSignalWithoutRegistration(const sdbus::Struct<std::string, sdbus::Struct<sdbus::Signature>>& s)
     {
         object_.emitSignal("signalWithoutRegistration").onInterface(INTERFACE_NAME).withArguments(s);
     }