ctubbsii commented on PR #2712: URL: https://github.com/apache/accumulo/pull/2712#issuecomment-1135321175
> > Tests often use mock objects to test specific code paths. When you change the behavior of the code, you can break certain assumptions that the test code is relying on. > > Instead of breaking those assumptions, commit [6189662](https://github.com/apache/accumulo/commit/6189662b0766debf42d199e148e8f52a9c6fe13c) tries to conform to them by using the new property `Property.GENERAL_THREADPOOL_SIZE` whenever the conflict resolution fails. The old code used `Property.GENERAL_SIMPLETIMER_THREADPOOL_SIZE` in all situations, no matter what the value of the argument `conf` was and happily passed `conf` to `createExecutorService()`. I still think that this is the wrong direction. I don't think we should broaden the catch clause like this... the tests have been identified as making faulty assumptions while creating partially mocked objects ("nice" mocks). Rather than conform to these faulty assumptions, it would be better to improve the mocking so that they are not making the faulty assumptions in the first place. I will take a look to see what is involved in fixing the 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]
