hossman commented on PR #2869: URL: https://github.com/apache/solr/pull/2869#issuecomment-2494322602
I don't know about the `shutdown()` changes, but moving this logic into a `Rule` seems great, and and using that `@Rule` in `SolrTestCase` (instead of `SolrTestCaseJ4`) seems fine... But I'm curious: why not just leave that `@Rule` in `SolrTestCaseJ4` (for backcompat) and let any other `SolrTestCase` subclasses that want to use this functionality add the `@Rule` themselves? ... isn't that the real value of the `Rule` API, that's it's easy to wire in and mix/match? Either way I would suggest updating the `TestLogLevelAnnotations extends ...` declaration to use `SolrTestCase` -- that's the best way to "prove" that making these changes "works".(There are also some comments in that class that probably need updated? ... i think the code they comment is all still true, but they make references to `SolrTestCaseJ4` that should probably now refer to something else?) -- 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
