[ 
https://issues.apache.org/jira/browse/LANG-1671?focusedWorklogId=752054&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-752054
 ]

ASF GitHub Bot logged work on LANG-1671:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 03/Apr/22 15:12
            Start Date: 03/Apr/22 15:12
    Worklog Time Spent: 10m 
      Work Description: garydgregory commented on pull request #851:
URL: https://github.com/apache/commons-lang/pull/851#issuecomment-1086888746


   I've looked at this again and decided that while it would be OK to have this 
type of change in principle, I want to use the tests as a simple example in 
this case. I've made adjustments to the Javadoc to reflect this intent. 
   
   The mocking here is fine (again, in principle) but it makes the code much 
harder to follow and presents an additional hurdle to people wanting to 
understand the library. Personally, I prefer to use mocking when it is the only 
solution for testing or the normal way to test requires too much code. I do not 
feel this is the case here.
   
   Thank you for taking the time to provide this PR though.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 752054)
    Time Spent: 40m  (was: 0.5h)

> Refactor AtomicSafeInitializerTest to improve test design
> ---------------------------------------------------------
>
>                 Key: LANG-1671
>                 URL: https://issues.apache.org/jira/browse/LANG-1671
>             Project: Commons Lang
>          Issue Type: Improvement
>          Components: lang.concurrent.*
>            Reporter: Xiao Wang
>            Priority: Minor
>          Time Spent: 40m
>  Remaining Estimate: 0h
>
> h3. Description
> I noticed that there is a test class 
> [AtomicSafeInitializerTestImpl|https://github.com/apache/commons-lang/blob/69c9593cc1da760bb4dbcf32f4ae755c54376b77/src/test/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializerTest.java#L67]
>  extends production class 
> [AtomicSafeInitializer|https://github.com/apache/commons-lang/blob/69c9593cc1da760bb4dbcf32f4ae755c54376b77/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java#L55]
>  to assist testing method 
> [AtomicSafeInitializer.get()|https://github.com/apache/commons-lang/blob/69c9593cc1da760bb4dbcf32f4ae755c54376b77/src/main/java/org/apache/commons/lang3/concurrent/AtomicSafeInitializer.java#L72].
>  This might not be the best priactice in unit testing and can be improved by 
> leveraging mocking frameworks.
> h3. Current Implementation
>  * {{AtomicSafeInitializerTestImpl}} extends {{AtomicSafeInitializer}} and 
> creates a new variable to keep tracking of the method invocation status for 
> {{initialize()}}.
>  * In test cases, after executing {{AtomicSafeInitializer.get()}}, the new 
> variable will be used in assertion statement to check the execution status of 
> {{initialize()}}.
> h3. Proposed Implementation
>  * Replace {{AtomicSafeInitializerTestImpl}} with a mocking object created by 
> Mockito.
>  * Extract the AtomicLong attribute and use the extracted attribute in test 
> case to check method invocation status.
>  * Use method stub to control the behavior of the mocking object.
> h3. Motivation
>  * Decouple test class {{AtomicSafeInitializerTestImpl}} from production 
> interface {{AtomicSafeInitializer}}.
>  * Make test logic more clear by using method stub instead of method 
> overriding.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to