albertogpz commented on pull request #5476: URL: https://github.com/apache/geode/pull/5476#issuecomment-681917316
> A few small changes to clean up the code a bit. I also notice that there are several DUnit test failures associated with this change (more than we would expect just due to the known flakiness of some WAN DUnit tests) so I would prefer not to merge this change until they have been diagnosed and remedied. > > Also, while it's not strictly within the scope of this PR, I notice that in `WANTestBase.createSender()` line 1747, the call to `gateway.create(dsName, remoteDsId);` comes before the call to `gateway.setBatchTimeInterval(batchTimeInterval);` which means the second call has no effect on the batch time interval. If fixing this has no effect on existing tests, then it would be okay to include it as part of this PR, I think. If fixing it causes tests to start failing, a separate JIRA ticket should be created to address the changes. @DonalEvans Thanks a lot for your comments. You have an incredible eye to find every error and identify items to improve. - I have added a fix for the distributed tests that were failing. - I have incorporated all the fixes you have suggested in your comments. - I have also tackled the issue you mentioned regarding `WANTestBase.createSender()` when specifying a `batchTimeInterval`. The way to address it has been to remove the parameter from the method as it had no effect anyway. I think it was me the one who introduced it in the first place when I implemented the "GEODE-7971" and was trying to fix some flaky tests. But obviously, that parameter did not have anything to do to the fix of the test cases. ---------------------------------------------------------------- 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: us...@infra.apache.org
