a "lambda wrapper" (similar to how expectException(...) works) would probably make more sense...
I briefly toyed with this, but quickly realized it was actaully a big pain in the ass, because there's no easy way for a method like this to propogate any Throwable thrown by the wrapped lambda w/o just delcaring "throws Throwable" (not an issue for expectException(...) because by design it wants to catch exceptions and it wants to "fail" if a different type of exception is thrown)
I realized what makes a lot more sense is a LogInterceptor implements AutoClosable that sets up the log Filter in it's constructor/factory method, and removes the interception in it's close() method – that way you can use a "try-with-resources" to wrap the code you want to ignore exceptions in.
A Pro/Con of this approach is that you can call methods on the LogInterceptor to "inspect" the intercepted log messages inside the try block, w/o needing to wait until all of the logic in the block is done ... the "Con" part being that you MUST call at least some method on the LogInterceptor inside the try block, or the compiler warn/fails tha it's unused. (Which i think is actually a "Pro": If a test wants to expect/ignore some ERROR message logs, it should assert something about them)
The attached patch implements this idea – but I should note that I decided pretty quickly that because of how log4j (and it's APIs) work, just having each LogInterceptor add a Filter on the root logger ddin't seem like it would really cut it if we also wanted to support "inspection"; because if we wanted to have multiple LogInterceptor 's in use at the same time, each interceptor/Filter could potentially get in each others way. It seems cleaner to ask you what logger to "inspect" and use an Appender there with a Filter that only ACCEPTs the LogEvents you are interested in, while also adding a Filter to the ROOT logger to prevent those ERRORs from making it to the log file.
The patch works, and you can see from the tests what it might look like if we started using it – but the more i've been working on it, the more disatisfied i've become with the both the surface API and a llot of the implementation details.
I won't elaborate too much here in jira – it's pretty thoroughly spelled out in nocommit comments in the patch, where i also flesh out what i think would be better – for now i'll just point out:
- I do plan to keep working on this
- If i get hit by a bus and never finish this, please remember:
- a lot of complexity in the current patch (and in the suggestions in the nocommit comments) comes from wanting to implement something BETTER then the existing SolrTestCaseJ4.ignoreException functionality
- I'd really like us to be able write better tests that are more strict about if/when they expect an ERROR (or a WARN) to be logged
- ideally even start failing the build if any test causes solr to log an ERROR that isn't expected
- The original idea of just (temporarily) adding a Filter to the ROOT logger (that DENY's an ERROR log matching the Pattern or (sub)String) is still very viable and would be simple to implement as an alternative if someone is interested.
Strawman idea of what this might look like from an API standpoint:
(If/when we do this, we could potentially make SolrTestCase start to fail if ANY unexpected ERROR/WARN makes it past the configured filters)