TODO update this for Open Source developers PLFM-194
Notes on best practices. This is a work in progress and suggested guidelines, not a rulebook to follow like slaves.
If it isn't tested, it doesn't work. Period. All new features require appropriate unit tests and integration tests as part of the development process. Passing tests should be checked in the same time as new code, and are fully in the scope of code reviews.
We use JUnit as a testing framework and Mockito for testing, mocking, stubbing etc. At the time of writing we use both JUnit5 and JUnit4 for backward compatibility and we are trying to move into the direction of JUnit5 for all new tests. In order to incrementally update the code base without having disrupting PRs we adopt the following simple rules:
To upgrade a test from JUnit4 to JUnit5 in general the following steps might be necessary:
Remove any @Test(expected=SomeException.class) and use:
org.junit.jupiter.api.Assertions.assertThrows(SomeException.class, ()-> { /* Your code here */ }); |
Replace the org.junit.runner.RunWith annotation with the org.junit.jupiter.api.extension.ExtendWith, for example:
@ExtendWith(SpringExtension.class)
public class SomeIntegrationTest {}
@ExtendWith(MockitoExtension.class)
public class SomeUnitTest {} |
If the tests fails with UnnecessaryStubbingException you might need to review your test code (E.g. Check that unnecessary stubbing is not performed in a @BeforeEach, for example stubbing that is not used in all the tests). If it's not possible to fix an old test without spending too much time use the org.mockito.Mockito.lenient() or enable the lenient setting at the class level:
@ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.LENIENT)
public class SomeLenientTest {} |
For mocks where the mocked object is NOT STATEFUL and the return value of the method is NOT specific to ANY PARAMETERS, use Mockitio.lenient().when() to make that specific mock lenient. Do this only in the @BeforeEach setup() method.
@BeforeEach
public void setup(){
lenient().when(mockServiceProvider.getTableServices()).thenReturn(mockTableService);
lenient().when(mockServiceProvider.getWikiService()).thenReturn(mockWikiService);
lenient().when(mockServiceProvider.getEntityService()).thenReturn(mockEntityService);
lenient().when(mockServiceProvider.getDoiServiceV2()).thenReturn(mockDoiServiceV2);
lenient().when(mockServiceProvider.getDiscussionService()).thenReturn(mockDiscussionService);
lenient().when(mockServiceProvider.getDataAccessService()).thenReturn(mockDataAccessService);
} |
Let's do code reviews for all code changes.
Regarding rigor, I don't expect the reviewer to scrutinize the code (the burden is on me to test it) but instead to: