[jira] [Closed] (LANG-1671) Refactor AtomicSafeInitializerTest to improve test design

["Gary D. Gregory (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/LANG-1671?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Gary D. Gregory closed LANG-1671.
---------------------------------
    Resolution: Not A Problem

My comment in the PR:

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.

> 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: 50m
>  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)