kirklund opened a new pull request #6809: URL: https://github.com/apache/geode/pull/6809
…der when group-transaction-events is true (#6663)" This reverts commit b377e3f808562fdf59d2753a5a58fd9b9d603883. --- I've reviewed the code that went in with #b377e3, and I think we should revert it to rework WanCopyRegionFunction and its unit test. The unit test currently mocks three methods in WanCopyRegionFunction (known as partial mocking) which is a sign that dependencies are hidden inside the class and need to be pulled up to the constructor. WanCopyRegionFunction also has a static final ExecutorService which is never shutdown. The ExecutorService should live outside WanCopyRegionFunction, be passed in via the constructor, and be shutdown when the Cache closes. The intermittent test failures are not specific to Windows and are likely to also fail on a slow Linux machine. We really need to avoid Thread sleeps especially in unit tests. This and the partial mocking are signs that the class needs to change to better enable unit testing. I'll be more than happy to help but I think we need to revert it so we have time to discuss it without everyone else being affected by failing tests. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
