michael-carter-instaclustr commented on pull request #8844: URL: https://github.com/apache/kafka/pull/8844#issuecomment-649196880
I've made changes to those test now @C0urante . I couple of things worth noting: Changing the mock of the WorkMetricsGroup to a real object needs a fair few more mocks that relate to each other, so I've organised those into a @Before method instead of injecting them via a @Mock annotation, I hope that's okay. Doing so had the benefit that the tests are now more explicit about the values of the metrics being recorded, which may make the deletion of lines in WorkerTest more palatable. Having a look at the tests I'm modifying there, most of them actually already do have expectations on the statusListener being called, so I think it that case it's fair to say that the responsibility has simply moved to the WorkerGroupMetrics class (and therefore the WorkerGroupMetricsTest). For the test that doesn't seem to have any expectations on the status listener (testAddRemoveTask), I believe this might be because it's the WorkerTask's job to call the status listener once it's running, but that aspect is mocked away in that particular test. ---------------------------------------------------------------- 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
