wenbingshen commented on PR #3637: URL: https://github.com/apache/bookkeeper/pull/3637#issuecomment-1325091224
> It does split the code into smaller components but it leaves the components interconnected - tasks that have to access Auditor's internals etc. Typically this is a design smell that will backfire in the future when changing the code or it will become a burden when you start writing unit tests and mocks for the tasks Second point is the unit tests. I think that new tasks deserve some unit tests. @dlg99 1. I updated the code to avoid circular dependency between Auditor and CheckTask. 2. I added unit tests corresponding to different tasks. 3. Now these tasks can run independently without Auditor. 4. These tasks all have basic unit tests, indicating that they can run independently without Auditor. Since there are too many items that can be used for testing, if we want other unit tests to cover more options, I hope to use another PR. The previous unit tests are sufficient to prove that the changes in this PR maintain compatibility. If tests not fail, PTAL. Thanks! -- 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]
