You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "kirklund (GitHub)" <gi...@apache.org> on 2018/11/26 19:30:22 UTC

[GitHub] [geode] kirklund opened pull request #2896: GEODE-6092: Simplify SingleHopClientExecutorWithLoggingTest

This test should now make a decent role model for other unit tests that
need to verify logging behavior of a class.

Note: The recent changes to Geode logging allowed me to simplify this test even further because Geode no longer adds/removes ConsoleAppender (it's always present now).

[ Full content available at: https://github.com/apache/geode/pull/2896 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund commented on pull request #2896: GEODE-6092: Simplify SingleHopClientExecutorLoggingTest

Posted by "kirklund (GitHub)" <gi...@apache.org>.
I deleted the comments that either of us thought were of dubious value and then I reworded the javadoc on the test class itself.

[ Full content available at: https://github.com/apache/geode/pull/2896 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] kirklund closed pull request #2896: GEODE-6092: Simplify SingleHopClientExecutorLoggingTest

Posted by "kirklund (GitHub)" <gi...@apache.org>.
[ pull request closed by kirklund ]

[ Full content available at: https://github.com/apache/geode/pull/2896 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on pull request #2896: GEODE-6092: Simplify SingleHopClientExecutorLoggingTest

Posted by "mcmellawatt (GitHub)" <gi...@apache.org>.
Not sure this comment is helpful anymore, though I know its not part of your changes here.  Maybe it is from when we didn't use a unified GeodeAwaitility that handles the timeout specification for us?

[ Full content available at: https://github.com/apache/geode/pull/2896 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org

[GitHub] [geode] mcmellawatt commented on issue #2896: GEODE-6092: Simplify SingleHopClientExecutorLoggingTest

Posted by "mcmellawatt (GitHub)" <gi...@apache.org>.
I think this is good conceptually, but I think we identified that Log4J is storing a static reference to System.out which may differ from the one referenced by SystemOutRule on repeated **unit** test runs.  This was not a problem with **integration** tests since each repeated test will get executed in a new JVM.  Maybe we can compromise for now and delete the unnecessary code in the test, but leave it as an integration test?  Or do you think it worth figuring out how to deal with this interaction between Log4J and SystemOutRule?

[ Full content available at: https://github.com/apache/geode/pull/2896 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org