[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #953: MINIFICPP-1421 - Disable C2JstackTest

[GitBox <gi...@apache.org>]

fgerlits commented on a change in pull request #953:
URL: https://github.com/apache/nifi-minifi-cpp/pull/953#discussion_r542223028



##########
File path: extensions/http-curl/tests/C2JstackTest.cpp
##########
@@ -18,35 +18,46 @@
 
 #undef NDEBUG
 #include <string>
+#include <atomic>
 #include "TestBase.h"
 #include "HTTPIntegrationBase.h"
 #include "HTTPHandlers.h"
 #include "utils/IntegrationTestUtils.h"
 
 class VerifyC2DescribeJstack : public VerifyC2Describe {
  public:
+  explicit VerifyC2DescribeJstack(const std::atomic_bool& acknowledgement_checked) : VerifyC2Describe(), acknowledgement_checked_(acknowledgement_checked) {}
   void runAssertions() override {
-    using org::apache::nifi::minifi::utils::verifyLogLinePresenceInPollTime;
-    assert(verifyLogLinePresenceInPollTime(std::chrono::milliseconds(wait_time_), "SchedulingAgent"));
+    // This check was previously only confirming the presence of log sinks.
+    // See: https://issues.apache.org/jira/browse/MINIFICPP-1421

Review comment:
       I don't think this comment is useful; if someone is interested in the history of this function, they can use `git blame`.

##########
File path: extensions/http-curl/tests/C2JstackTest.cpp
##########
@@ -18,35 +18,46 @@
 
 #undef NDEBUG
 #include <string>
+#include <atomic>
 #include "TestBase.h"
 #include "HTTPIntegrationBase.h"
 #include "HTTPHandlers.h"
 #include "utils/IntegrationTestUtils.h"
 
 class VerifyC2DescribeJstack : public VerifyC2Describe {
  public:
+  explicit VerifyC2DescribeJstack(const std::atomic_bool& acknowledgement_checked) : VerifyC2Describe(), acknowledgement_checked_(acknowledgement_checked) {}
   void runAssertions() override {
-    using org::apache::nifi::minifi::utils::verifyLogLinePresenceInPollTime;
-    assert(verifyLogLinePresenceInPollTime(std::chrono::milliseconds(wait_time_), "SchedulingAgent"));
+    // This check was previously only confirming the presence of log sinks.
+    // See: https://issues.apache.org/jira/browse/MINIFICPP-1421
+    using org::apache::nifi::minifi::utils::verifyEventHappenedInPollTime;
+    assert(verifyEventHappenedInPollTime(std::chrono::milliseconds(wait_time_), [&] { return acknowledgement_checked_.load(); }));
   }
+ protected:
+  const std::atomic_bool& acknowledgement_checked_;
 };
 
 class DescribeJstackHandler : public HeartbeatHandler {
  public:
+  explicit DescribeJstackHandler(std::atomic_bool& acknowledgement_checked) : HeartbeatHandler(), acknowledgement_checked_(acknowledgement_checked) {}
   void handleHeartbeat(const rapidjson::Document&, struct mg_connection * conn) override {
     sendHeartbeatResponse("DESCRIBE", "jstack", "889398", conn);
   }
 
   void handleAcknowledge(const rapidjson::Document& root) override {
     assert(root.HasMember("Flowcontroller threadpool #0"));
+    acknowledgement_checked_.store(true);
   }
+ protected:
+  std::atomic_bool& acknowledgement_checked_;
 };
 
 int main(int argc, char **argv) {
   const cmd_args args = parse_cmdline_args(argc, argv, "heartbeat");
-  VerifyC2DescribeJstack harness;
+  std::atomic_bool acknowledgement_checked{ false };

Review comment:
       `acknowledgement_received` may be a better name for this, as we set it to `true` when the ack is received, not when we check that it has been received




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org