[ https://issues.apache.org/jira/browse/LOG4J2-3164?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Volkan Yazici reassigned LOG4J2-3164: ------------------------------------- Assignee: Volkan Yazici > Refactor StoringStatusListener to improve test logic > ---------------------------------------------------- > > Key: LOG4J2-3164 > URL: https://issues.apache.org/jira/browse/LOG4J2-3164 > Project: Log4j 2 > Issue Type: Improvement > Components: Core > Reporter: Xiao Wang > Assignee: Volkan Yazici > Priority: Minor > > h3. Description > I noticed that there is a test class > [StoringStatusListener|https://github.com/apache/logging-log4j2/blob/20f9a97dbe5928c3b5077bcdd2a22ac92e941655/log4j-core/src/test/java/org/apache/logging/log4j/core/config/plugins/validation/validators/ValidatingPluginWithFailoverTest.java#L86] > implements production interface > [StatusListener|https://github.com/apache/logging-log4j2/blob/20f9a97dbe5928c3b5077bcdd2a22ac92e941655/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusListener.java#L27] > to assist testing > [PluginBuilder.build()|https://github.com/apache/logging-log4j2/blob/20f9a97dbe5928c3b5077bcdd2a22ac92e941655/log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/util/PluginBuilder.java#L117]. > This might not be the best priactice in unit testing and can be improved by > leveraging mocking frameworks. > h3. Current Implementation > * {{StoringStatusListener}} implements {{StatusListener}} and overrides > methods to track the input parameters of {{StatusListener.log(StatusData)}}. > * In test case, the child test class is used in assertion statement to make > sure the log method never been executed. > h3. Proposed Implementation > * Replace {{StoringStatusListener}} with a mocking object created by Mockito. > * Use method stub to control the behavior of {{getStatusLevel()}}. > * Use {{ArgumentCapture}} to capture the input argument for > {{StatusListener.log(StatusData)}} and use it in the assertion statement. > h3. Motivation > * Decouple test class {{StoringStatusListener}} from production interface > {{StatusListener}}. > * Remove the redundant test child class {{StoringStatusListener}} > * Remove the redundant overridden methods {{close()}} > * Make testing logic more explict by directly verify the input argument > through ArgumentCapture. > h3. More Thought > * If we only want to make sure that the {{log(StatusData)}} method was never > been executed, we can verify it by {{Mockito.times()}} and get rid of the > ArgumentCapture. It can make the test condition even more clear. -- This message was sent by Atlassian Jira (v8.3.4#803005)