----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36957/#review93956 -----------------------------------------------------------
gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/LogService.java (line 87) <https://reviews.apache.org/r/36957/#comment148399> This PropertyChangeListener is invokes this method once and then this line was invoking it a second time. I'll delete this line. gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/LogService.java (line 150) <https://reviews.apache.org/r/36957/#comment148401> I'll delete all deadcode such as these lines. gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/LogService.java (line 256) <https://reviews.apache.org/r/36957/#comment148402> I discovered that only two filter types were being handled correctly. Appender and AppenderRef filters were not being handled correctly. Basically, if there are any filters defined or if the level is DEBUG or lower then FastLogger must delegate to the underlying logger rather than early-out from the isEnabled checks. gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/log4j/FastLogger.java (line 21) <https://reviews.apache.org/r/36957/#comment148404> I renamed this to delegating to avoid confusion with isDebugEnabled which is a pair of methods on Logger API. gemfire-core/src/test/java/com/gemstone/gemfire/internal/logging/log4j/FastLoggerIntegrationJUnitTest.java (line 37) <https://reviews.apache.org/r/36957/#comment148408> Moved most tests from FastLoggerJUnitTest to here because they were all IntegrationTests. Expanded coverage to include all log4j2 filter types and all boundary/transition points interesting for FastLogger (ex: to/and/from isDelegating). gemfire-core/src/test/java/com/gemstone/gemfire/internal/logging/log4j/FastLoggerWithDefaultConfigJUnitTest.java (line 56) <https://reviews.apache.org/r/36957/#comment148407> Need to rename "debugAvailable" to "delegating" - Kirk Lund On Aug. 3, 2015, 7:58 p.m., Kirk Lund wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36957/ > ----------------------------------------------------------- > > (Updated Aug. 3, 2015, 7:58 p.m.) > > > Review request for geode and Darrel Schneider. > > > Bugs: GEODE-181 and GEODE-89 > https://issues.apache.org/jira/browse/GEODE-181 > https://issues.apache.org/jira/browse/GEODE-89 > > > Repository: geode > > > Description > ------- > > I found GEODE-181 while refactoring the tests in FastLoggerJUnitTest for > GEODE-89. These changes address both tickets. > > Extract testDefaultConfig to FastLoggerWithDefaultConfigJUnitTest. > > Refactor remaining test in FastLoggerJUnitTest to improve coverage and > readability. Introduce use of TemporaryFolder instead of using tmp dir. > Change from UnitTest to IntegrationTest because of file system I/O and > execution speed. > > Add new TestSuite classes for targeted testing of logging and log4j packages. > > > Diffs > ----- > > build.gradle c82e82a > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/LogService.java > 6298cf6 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/log4j/Configurator.java > c7ae945 > > gemfire-core/src/main/java/com/gemstone/gemfire/internal/logging/log4j/FastLogger.java > 21d7965 > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/logging/DistributedSystemLogFileJUnitTest.java > 9c7ba58 > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/logging/LoggingIntegrationTestSuite.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/logging/LoggingUnitTestSuite.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/logging/log4j/FastLoggerIntegrationJUnitTest.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/logging/log4j/FastLoggerJUnitTest.java > 2aab5df > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/logging/log4j/FastLoggerWithDefaultConfigJUnitTest.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/logging/log4j/Log4jIntegrationTestSuite.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/internal/logging/log4j/Log4jUnitTestSuite.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/36957/diff/ > > > Testing > ------- > > test, integrationTest, new TestSuites, rebuild open+closed > > > Thanks, > > Kirk Lund > >
