dsmiley commented on code in PR #2571: URL: https://github.com/apache/solr/pull/2571#discussion_r1690194393
########## solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudHttp2SolrClientTest.java: ########## @@ -93,6 +94,22 @@ public class CloudHttp2SolrClientTest extends SolrCloudTestCase { private static CloudSolrClient httpBasedCloudSolrClient = null; private static CloudSolrClient zkBasedCloudSolrClient = null; + private static LogListener entireClusterStateLogs; + private static LogListener collectionClusterStateLogs; + private static LogListener commandLogs; Review Comment: LogListener is Closeable; it's intended to be used with try-with-resources pattern. By having a static field of them, you need to take care to close them. I'm not sure if LogListener is used this way across our tests... not sure I like it. Any way by listening globally for the whole suite, isn't asserting it kind of difficult to reason about as different tests will do stuff? -- 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: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org