You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2021/09/21 06:29:01 UTC

[GitHub] [logging-log4j2] wx930910 opened a new pull request #584: LOG4J2-3164: Refactor StoringStatusListener to improve test logic

wx930910 opened a new pull request #584:
URL: https://github.com/apache/logging-log4j2/pull/584


   ### Jira
   
   - [LOG4J2-3164](https://issues.apache.org/jira/browse/LOG4J2-3164)
   
   ### Description
   
   #### Replace 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) by mocking object and improve test design.
   <hr>
   
   ##### 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`.
   
   <hr>
   
   ##### Key changed/added classes in this PR
    * Created mocking object to replace test subclass `StoringStatusListener`, decoupled test from production code.
    * Use ArgumentCapture to replace the new field created in `StoringStatusListener`.
   
   <hr>
   


-- 
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: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [logging-log4j2] wx930910 commented on pull request #584: LOG4J2-3164: Refactor StoringStatusListener to improve test logic

Posted by GitBox <gi...@apache.org>.
wx930910 commented on pull request #584:
URL: https://github.com/apache/logging-log4j2/pull/584#issuecomment-923680213


   If we only want to make sure that the `log(StatusData)` method execution status, we can verify it by using `Mockito.times()` and get rid of the ArgumentCapture. It can make the test condition even more clear.


-- 
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: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [logging-log4j2] wx930910 commented on pull request #584: LOG4J2-3164: Refactor StoringStatusListener to improve test logic

Posted by GitBox <gi...@apache.org>.
wx930910 commented on pull request #584:
URL: https://github.com/apache/logging-log4j2/pull/584#issuecomment-924494400


   @vy Thanks for your comments! Make sence to me, I changed the code corresponding to your advice :+1: 


-- 
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: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [logging-log4j2] vy commented on pull request #584: LOG4J2-3164: Refactor StoringStatusListener to improve test logic

Posted by GitBox <gi...@apache.org>.
vy commented on pull request #584:
URL: https://github.com/apache/logging-log4j2/pull/584#issuecomment-924330636


   Thanks so much for the contribution @wx930910! Mocking is definitely a better approach, since it allows (backward-compatible) evolution of `StatusListener` without breaking tests. Not to mention that the intent is clearer as well.
   
   Regarding the implementation, I think I would prefer a `verify(listener, never())` finalized with a `verifyNoMoreInteractions()`. If you would be kind enough to update your PR, I will be glad to merge it.


-- 
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: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [logging-log4j2] vy merged pull request #584: LOG4J2-3164: Refactor StoringStatusListener to improve test logic

Posted by GitBox <gi...@apache.org>.
vy merged pull request #584:
URL: https://github.com/apache/logging-log4j2/pull/584


   


-- 
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: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org