Tests for service layer #954

BranetE · 2023-01-19T18:18:48Z

dev

JIRA

Main JIRA ticket

Code reviewers

Summary of issue

Classes ValuesForUserServiceImpl and AzureStorageService weren`t covered by tests at all, ViolationServiceImpl was not fully covered

Summary of change

Made Tests For AzureCloudStrogeService
Added check if file were tying to upload isnt null
Created FileIsNullException and tests for it
Made tests for ValuesForUserServiceImpl
Fixed two tests in ViolationServiceImpl which weren`t working and were disadled

Testing approach

Run tests in /service/src/test/java/greencity/service/ubs

CHECK LIST

СI passed
Сode coverage >=95%
PR is reviewed manually again (to make sure you have 100% ready code)
All reviewers agreed to merge the PR
I've checked new feature as logged in and logged out user if needed
PR meets all conventions

…d Orders For UserServiceImpl

# Conflicts: # service/src/test/java/greencity/service/ubs/OrdersForUserServiceImplTest.java

RomaMocherniuk · 2023-01-23T12:26:12Z

service/src/main/java/greencity/service/ubs/AzureCloudStorageService.java

@@ -40,6 +42,9 @@ public AzureCloudStorageService(@Autowired PropertyResolver propertyResolver) {
     */
    @Override
    public String upload(MultipartFile multipartFile) {
+        if (multipartFile == null) {
+            throw new FileIsNullException(ErrorMessage.FILE_IS_NULL);


please use IllegalArgumentException

And please add exception handler

RomaMocherniuk · 2023-01-23T12:29:18Z

service/src/test/java/greencity/service/ubs/AzureCloudStorageServiceTest.java

+    @BeforeEach
+    void setUp() {
+        MockEnvironment mockEnvironment = new MockEnvironment();
+        mockEnvironment.setProperty("azure.connection.string", " ");


please use some values for properties

RomaMocherniuk · 2023-01-23T12:32:25Z

service/src/test/java/greencity/service/ubs/AzureCloudStorageServiceTest.java

+    private AzureCloudStorageService azureCloudStorageService;
+
+    @Mock
+    BlobContainerClient containerClient;


RomaMocherniuk · 2023-01-23T12:32:32Z

service/src/test/java/greencity/service/ubs/AzureCloudStorageServiceTest.java

+    BlobContainerClient containerClient;
+
+    @Mock
+    BlobClient blobClient;


RomaMocherniuk · 2023-01-23T12:35:43Z

service/src/test/java/greencity/service/ubs/AzureCloudStorageServiceTest.java

+        when(containerClient.getBlobClient(anyString())).thenReturn(blobClient);
+        doReturn("blobUrl").when(blobClient).getBlobUrl();
+        azureCloudStorageService.upload(multipartFile);
+        assertNotNull(multipartFile);


Unnecessary check. multipartFile could not be null
You created it here

RomaMocherniuk · 2023-01-23T12:36:56Z

service/src/test/java/greencity/service/ubs/AzureCloudStorageServiceTest.java

+        doReturn("blobUrl").when(blobClient).getBlobUrl();
+        azureCloudStorageService.upload(multipartFile);
+        assertNotNull(multipartFile);
+        assertNotNull(azureCloudStorageService.getConnectionString());


please check that values are equal to the values you set in the setUp method

RomaMocherniuk · 2023-01-23T12:38:04Z

service/src/test/java/greencity/service/ubs/AzureCloudStorageServiceTest.java

+    void checkDelete() {
+        doReturn(containerClient).when(azureCloudStorageService).containerClient();
+        when(containerClient.getBlobClient(anyString())).thenReturn(blobClient);
+        when(blobClient.exists()).thenReturn(true);


please add test if blobClient.exists() == null and blobClient.exists() == false

RomaMocherniuk · 2023-01-23T12:39:28Z

service/src/test/java/greencity/service/ubs/ValuesForUserTableServiceImplTest.java

+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class ValuesForUserTableServiceImplTest {


Please follow encapsulation object-oriented principle

RomaMocherniuk · 2023-01-23T12:40:54Z

service/src/test/java/greencity/service/ubs/ValuesForUserTableServiceImplTest.java

+    ValuesForUserTableServiceImpl valuesForUserTableService;
+
+    @Test
+    void getAllFields() {


please add test if user does not exist

RomaMocherniuk · 2023-01-23T12:41:14Z

service/src/test/java/greencity/service/ubs/ValuesForUserTableServiceImplTest.java

+        valuesForUserTableService.getAllFields(new CustomerPage(), "column", SortingOrder.ASC, new UserFilterCriteria(),
+            employee.getEmail());
+
+        verify(userTableRepo).findAll(any(UserFilterCriteria.class), anyString(), any(SortingOrder.class),


please verify all mocks

RomaMocherniuk · 2023-01-23T12:42:15Z

service/src/main/java/greencity/service/ubs/AzureCloudStorageService.java

@@ -65,7 +70,8 @@ public void delete(String url) {
        }
    }

-    private BlobContainerClient containerClient() {
+    @VisibleForTesting
+    BlobContainerClient containerClient() {


please add tests for this method too

RomaMocherniuk · 2023-01-23T18:10:50Z

service/src/test/java/greencity/service/ubs/AzureCloudStorageServiceTest.java

+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.NullSource;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.*;


please do not use *

RomaMocherniuk · 2023-01-24T12:00:06Z

service/src/test/java/greencity/service/ubs/AzureCloudStorageServiceTest.java

+@ExtendWith(MockitoExtension.class)
+class AzureCloudStorageServiceTest {
+
+    @BeforeEach


should after all class variables
https://www.cs.rice.edu/~cork/book/node9.html

RomaMocherniuk · 2023-01-24T12:00:34Z

service/src/test/java/greencity/service/ubs/AzureCloudStorageServiceTest.java

+    void setUp() {
+        MockEnvironment mockEnvironment = new MockEnvironment();
+        mockEnvironment.setProperty("azure.connection.string",
+            "DefaultEndpointsProtocol=https;AccountName=csb10032000a548f571;AccountKey=qV2VLVZlzxuEq8zGTgeiVE9puJiELNRPZcB9YgTSjZ3wKdWVA7kPjSOp6ESHlVMTJfHxB6N+iaV2TOlbe1GTvg==;EndpointSuffix=core.windows.net");


please do not use real values for tests

RomaMocherniuk · 2023-01-24T12:07:47Z

service/src/test/java/greencity/service/ubs/ValuesForUserTableServiceImplTest.java

+        when(userRepository.getAllUsersByTariffsInfoId(anyLong())).thenReturn(tariffsInfo);
+        when(userTableRepo.findAll(any(UserFilterCriteria.class), anyString(), any(SortingOrder.class),
+            any(CustomerPage.class), Mockito.<Long>anyList())).thenReturn(new PageImpl<>(List.of(user), pageable, 1L));
+        valuesForUserTableService.getAllFields(new CustomerPage(), "column", SortingOrder.ASC, new UserFilterCriteria(),


please check if the result of valuesForUserTableService.getAllFields contains all expected values

RomaMocherniuk · 2023-01-24T12:11:18Z

service/src/test/java/greencity/service/ubs/ViolationServiceImplTest.java

            () -> violationService.addUserViolation(add, new MultipartFile[2], "abc"));
+        assertEquals(ErrorMessage.EMPLOYEE_NOT_FOUND, ex.getMessage());


please add verify for orderRepository.findById(order.getId())).thenReturn(Optional.ofNullable(order)

RomaMocherniuk · 2023-01-24T12:11:38Z

service/src/test/java/greencity/service/ubs/ViolationServiceImplTest.java

        violationService.addUserViolation(add, new MultipartFile[2], "abc");

        assertEquals(1, user.getViolations());
    }

    @Test
-    @Disabled


please remove import for Disabled

RomaMocherniuk · 2023-01-24T12:11:51Z

service/src/test/java/greencity/service/ubs/ViolationServiceImplTest.java

        when(userRepository.countTotalUsersViolations(1L)).thenReturn(1);
+        when(employeeRepository.findByEmail(anyString())).thenReturn(Optional.ofNullable(employee));
+        when(employeeRepository.findTariffsInfoForEmployee(anyLong())).thenReturn(tariffsInfos);


please add verify for all mocks

RomaMocherniuk · 2023-01-24T12:13:41Z

service/src/test/java/greencity/service/ubs/ValuesForUserTableServiceImplTest.java

+import org.springframework.data.domain.*;
+
+import javax.persistence.EntityNotFoundException;
+import java.util.*;


please do not use *

RomaMocherniuk · 2023-01-24T12:13:47Z

service/src/test/java/greencity/service/ubs/ValuesForUserTableServiceImplTest.java

+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.*;


RomaMocherniuk · 2023-01-27T15:07:34Z

service/src/main/java/greencity/service/ubs/ValuesForUserTableServiceImpl.java

@@ -50,7 +51,8 @@ public PageableDto<UserWithSomeOrderDetailDto> getAllFields(CustomerPage page, S
            users.getPageable().getPageNumber(), users.getTotalPages());
    }

-    private UserWithSomeOrderDetailDto mapToDto(User u) {
+    @VisibleForTesting


why do we need it here?

It is used here https://github.com/ita-social-projects/GreenCityUBS/pull/954/files#diff-f331eed926ccba91320b5e7c37a260a220950a74a51c8ba591f629f1ca867c1eR65

sonarcloud · 2023-01-30T14:33:10Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

BranetE added 2 commits January 19, 2023 19:58

Made tests for ViolationServiceImpl, ValuesForUserTableServiceImpl an…

de942db

…d Orders For UserServiceImpl

Deleted Unused imports

67c5f3b

BranetE requested review from NKorzhik, ABbondar, juseti, JoseCaste and JJJazl January 19, 2023 18:18

BranetE added 2 commits January 19, 2023 20:23

Merge branch 'dev' into tests_for_service_layer

413fd40

# Conflicts: # service/src/test/java/greencity/service/ubs/OrdersForUserServiceImplTest.java

Formatted

a401d59

BranetE self-assigned this Jan 20, 2023

BranetE added the review label Jan 20, 2023

BranetE added 2 commits January 20, 2023 12:06

Merge branch 'dev' into tests_for_service_layer

9645e31

Fixed issues with tests

a206824

BranetE added work in progress and removed review labels Jan 20, 2023

BranetE and others added 12 commits January 20, 2023 15:22

Created tests for AzureCloudStorageService

ad07006

Added check if multipartFile is null and one test

9511661

Increased test coverage percentage for AzureCloudServiceTest

8c7676d

Removed unnecessary exception constructor

d1ced5b

checkstyle fix

c1fe53a

Added checks for exceptions

c0608c9

Update FileIsNullException.java

5989cbf

Fixed format

58e93b4

Tested contentClient

5e67e30

Created test for FileIsNullException

d90eaca

Fixed checkAddUserViolationThrowsException() in ViolationServiceImplTest

98f2ae7

Merge branch 'dev' into tests_for_service_layer

d4727ca

BranetE added review and removed work in progress labels Jan 23, 2023

Removed unnecessary comments

9016ba1

RomaMocherniuk reviewed Jan 23, 2023

View reviewed changes

BranetE added 4 commits January 23, 2023 15:39

fixed some issues

b54037c

Fix

b284bb9

Added IllegalArgumentException to handler and fixed code smell

eeb1fd3

Created test for containerClient()

ee7323d

RomaMocherniuk reviewed Jan 23, 2023

View reviewed changes

BranetE added 2 commits January 24, 2023 12:47

Merge branch 'dev' into tests_for_service_layer

0996600

fixed imports

2eb82d5

RomaMocherniuk reviewed Jan 24, 2023

View reviewed changes

BranetE added 4 commits January 24, 2023 15:07

Fixed fields check in checkUpload method an other

e668ba9

Removed unnecassary imports

38ae02a

Added assertEquals to ValuesForUserTableServiceImplTest

f9c19f6

Changed containerName

0a6ecc6

RomaMocherniuk reviewed Jan 27, 2023

View reviewed changes

Fix

c6eea78

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tests for service layer #954

Tests for service layer #954

BranetE commented Jan 19, 2023 •

edited

Loading

RomaMocherniuk Jan 23, 2023

RomaMocherniuk Jan 23, 2023

RomaMocherniuk Jan 23, 2023

RomaMocherniuk Jan 23, 2023

RomaMocherniuk Jan 23, 2023

RomaMocherniuk Jan 23, 2023

RomaMocherniuk Jan 23, 2023 •

edited

Loading

RomaMocherniuk Jan 23, 2023

RomaMocherniuk Jan 23, 2023

RomaMocherniuk Jan 23, 2023

RomaMocherniuk Jan 23, 2023

RomaMocherniuk Jan 23, 2023

RomaMocherniuk Jan 23, 2023

RomaMocherniuk Jan 24, 2023

RomaMocherniuk Jan 24, 2023

RomaMocherniuk Jan 24, 2023

RomaMocherniuk Jan 24, 2023

RomaMocherniuk Jan 24, 2023

RomaMocherniuk Jan 24, 2023

RomaMocherniuk Jan 24, 2023

RomaMocherniuk Jan 24, 2023

RomaMocherniuk Jan 27, 2023

BranetE Jan 30, 2023

sonarcloud bot commented Jan 30, 2023

		() -> violationService.addUserViolation(add, new MultipartFile[2], "abc"));
		assertEquals(ErrorMessage.EMPLOYEE_NOT_FOUND, ex.getMessage());

Tests for service layer #954

Are you sure you want to change the base?

Tests for service layer #954

Conversation

BranetE commented Jan 19, 2023 • edited Loading

JIRA

Code reviewers

Summary of issue

Summary of change

Testing approach

CHECK LIST

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RomaMocherniuk Jan 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jan 30, 2023

BranetE commented Jan 19, 2023 •

edited

Loading

RomaMocherniuk Jan 23, 2023 •

edited

Loading