[GitHub] [kafka] rondagostino commented on pull request #11066: MINOR: Test ReplicaManager Metric Names

2021-07-15 Thread GitBox
rondagostino commented on pull request #11066: URL: https://github.com/apache/kafka/pull/11066#issuecomment-881063696 Maybe rename `ReplicaManagerMetricNamesTest` to `ClusterMetricNamesTest` so that it isn't specific to `ReplicaManager`? We could define it like this: ```

[GitHub] [kafka] rondagostino commented on pull request #11066: MINOR: Test ReplicaManager Metric Names

2021-07-15 Thread GitBox
rondagostino commented on pull request #11066: URL: https://github.com/apache/kafka/pull/11066#issuecomment-881062140 > Can we do the integration test approach, but have it be part of RaftClusterTest.scala? I think we would only be testing for the KRaft case if we did that, and I

[GitHub] [kafka] rondagostino commented on pull request #11066: MINOR: Test ReplicaManager Metric Names

2021-07-15 Thread GitBox
rondagostino commented on pull request #11066: URL: https://github.com/apache/kafka/pull/11066#issuecomment-881031042 I've implemented the test in 2 separate ways, and we should choose which one to keep. 1. A standard unit test, in `ReplicaManagerTest` 2. A parameterized test