hunyadi-dev commented on a change in pull request #835:
URL: https://github.com/apache/nifi-minifi-cpp/pull/835#discussion_r458844014



##########
File path: extensions/http-curl/tests/HTTPSiteToSiteTests.cpp
##########
@@ -81,7 +81,10 @@ class SiteToSiteTestHarness : public CoapIntegrationBase {
 
   void cleanup() override {}
 
-  void runAssertions() override {}
+  void runAssertions() override {
+    // There is nothing to verify here, but we are expected to wait for all 
paralell events to execute 
+    std::this_thread::sleep_for(std::chrono::milliseconds(wait_time_));

Review comment:
       Maybe you misunderstand what is going on here. It is not my 
implementation that suddenly started to misuse the tesrunner, it was misused 
before as well. This change just made it glaringly obvious, which is I think a 
good thing to say the least.
   
   I am fine with someone having this refactored properly, I just do not want 
to do the extra work of reorganizing the structure for a test I do not even 
knows what it does. As for `waitToVerifyProcessors` it was clearly a poor 
decision to have this function lying around with a generic implementation 
forcing to wait pretty much all the tests.
   
   Ideally, one could go and?
   1. decouple the assertions from the test-runners if it is not the 
testrunners responsibility to run them **OR**
   1. move them inside and don't just use a testrunner for its byproduct of 
setting up some objects.
   
   I would go with the latter.




----------------------------------------------------------------
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:
[email protected]


Reply via email to