You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/02/14 14:32:22 UTC
[GitHub] [accumulo] ctubbsii opened a new issue #1512: CombinerTest
shouldn't test by trying to capture logs
ctubbsii opened a new issue #1512: CombinerTest shouldn't test by trying to capture logs
URL: https://github.com/apache/accumulo/issues/1512
The CombinerTest uses a questionable mechanism to test the expected logging behavior. It tries to hijack the log4j system, dynamically reconfiguring the logger with an appender that tries to capture log messages for the test.
This is unreliable and prevents upgrading to log4j2, which does not support such dynamic reconfiguration of logging, and whose APIs have changed significantly from log4j 1.2.
I'm working on this issue, and created this to track it, as part of my larger attempt to migrate to log4j2.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on issue #1512: CombinerTest
shouldn't test by trying to capture logs
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1512: CombinerTest shouldn't test by trying to capture logs
URL: https://github.com/apache/accumulo/issues/1512#issuecomment-588261584
OK I have a fix which removes the log4j and just checks the size of the cache to see if it was logged or not. I think that is sufficient. There is no need to check the string of the logger to make sure the error was logged.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1512: CombinerTest
shouldn't test by trying to capture logs
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1512: CombinerTest shouldn't test by trying to capture logs
URL: https://github.com/apache/accumulo/issues/1512#issuecomment-587942947
@milleruntime I initially assigned it to myself, because I expected to work on it, but if you have a good idea for how to improve it, I'd be happy to pass it off to you. I think you're right that it should be sufficient to check the cache.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1512: CombinerTest
shouldn't test by trying to capture logs
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1512: CombinerTest shouldn't test by trying to capture logs
URL: https://github.com/apache/accumulo/issues/1512#issuecomment-588346560
Awesome! Thanks @milleruntime !
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on issue #1512: CombinerTest
shouldn't test by trying to capture logs
Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #1512: CombinerTest shouldn't test by trying to capture logs
URL: https://github.com/apache/accumulo/issues/1512#issuecomment-587813938
Looked at this test briefly and I think it is overly complicated. I will look at it again but I think we can just remove the log4j stuff and test the cache if the error was logged.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime closed issue #1512: CombinerTest shouldn't
test by trying to capture logs
Posted by GitBox <gi...@apache.org>.
milleruntime closed issue #1512: CombinerTest shouldn't test by trying to capture logs
URL: https://github.com/apache/accumulo/issues/1512
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services