markrmiller commented on a change in pull request #301:
URL: https://github.com/apache/solr/pull/301#discussion_r721881065
##########
File path: solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
##########
@@ -64,17 +64,6 @@
@ThreadLeakLingering(linger = 10000)
public class SolrTestCase extends LuceneTestCase {
- /**
- * <b>DO NOT REMOVE THIS LOGGER</b>
- * <p>
- * For reasons that aren't 100% obvious, the existence of this logger is
neccessary to ensure
- * that the logging framework is properly initialized (even if concrete
subclasses do not
- * themselves initialize any loggers) so that the async logger threads can
be properly shutdown
- * on completion of the test suite
- * </p>
- * @see <a
href="https://issues.apache.org/jira/browse/SOLR-14247";>SOLR-14247</a>
- * @see #shutdownLogger
- */
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
Review comment:
I don't see any reason to remove it. There are other things in what was
the formerly the base class that should be here, there are plenty of things
that will be here, no benifit to removing it - nice to have it there for
someone to use vs not have a logger and have someone decide not to log
something from the base test class because they would rather not go copy paste
one in. I find it odd anyone wanted to remove it to begin with, of all the
classes you expect you might not want to have a logger, the base test class
does not seem like one of them.
Hoss hit that issue because that test class does not have a logger either
and blah blah, if no one was interested in what was happening then, doubt there
is any more interest now.
Anyway, the logger is used, so to remove it, you would have to remove the
current logging. And then you could look into that test, but regardless of if
you put this change in or not you would still not find hossmans fail, so it
would all look good even if this change did nothing. This base class has slowly
limped into almost not being a subtle trap. So there were actually multiple
ways out of hoss's brain teaser, both useful fixes of broken stuff that
affected other things and places as well.
There is a claim, you still want this, hoss's mystery thread leaks or not,
something about overlapping logging output in tests if you want to believe them.
--
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
