SolrTestCase now supports @LogLevel

[dsmiley]

Refactored the functionality into a TestRule, and moved from STCJ4 up to SolrTestCase.
Move reinstatement of the root log level to shutdown()

(part of bigger goal of abandoning SolrTestCaseJ4)

[dsmiley]

I did some basic observational testing of some classes that used LogLevel at method and class level to see that logs showed up for only certain methods but not others.

Not doing a JIRA for a test refactorings like this.

[dsmiley]

The move of this re-instate / reset of the root log level into shutdown() is debatable. I was trying to reduce little details in STC/STCJ4.

[dsmiley]

IMO, this is the perfect use case for a JUnit's TestRule. It takes advantage of observing the annotations that are present so that it can insert the Statement or skip it. And it works at both the class and method level with the same Statement. And it's generally useful -- I could see someone taking this into their own projects.

Maybe a case could be made for putting this rule as an inner class under LogLevel, so that it's more packaged together.

[hossman]

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?)

[dsmiley]

isn't that the real value of the Rule API, that's it's easy to wire in and mix/match?

Rules bring an encapsulation benefit -- I'd say that's the biggest IMO. Also mix/match, yes. But I don't see why any test would want to opt-out of this one given how cheap/light-weight it is, with its annotation based activation of the underlying Statement wrapper. I think we want to make it super-easy to debug a test with logs -- just add an annotation. Likewise I'm grateful other rules are defined always like system property clearing; I don't want tests to waste lines of code over basic book-keeping. I'd feel differently about a rule that seems heavy and/or is imposing in some way.