[GitHub] [nifi-minifi-cpp] msharee9 commented on issue #743: Minificpp 1169 - Simplify C2 metrics collection and reporting

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

msharee9 commented on issue #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#issuecomment-601870193
 
 
   > `C2FailedUpdateTest` fails for me consistently on macOS, and also seems to fail on Travis.
   > Also, the C2JstackTest never seems to return on Travis, that's why jobs are timing out (I could not reproduce this locally).
   > I am going to review the review fixes next, but these test failures must be addressed.
   
   It fails because we have an unnecessary and out of context assert to test the number of calls which somehow depends on the number of times the client POSTs to the CivetServer.
   assert(h_ex.calls_ <= (some time duration) + 1);
   I think that line came as a copy/paste from some other tests. It is certainly not needed here and also does not test anything related to the C2 failed update itself.
   
   The number of calls to handlePost should not be tested against some arbitrary amount of time duration. It does not make sense at all.
   
   Also I created a follow up ticket to cleanup the C2 Update tests. There is a lot of code duplication between some C2 update tests.
   
   https://issues.apache.org/jira/browse/MINIFICPP-1183

----------------------------------------------------------------
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


With regards,
Apache Git Services