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]