From 9ea112879575f60e9afa91bb5062e7f1deed2e61 Mon Sep 17 00:00:00 2001 From: Tim Yates Date: Thu, 11 Jul 2024 14:42:56 +0100 Subject: [PATCH 1/3] Add more fixture features to aid test setup --- .../services/fixture/SkillCategoryFixture.java | 12 +++++++----- .../checkins/services/fixture/SkillFixture.java | 12 ++++++++---- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/server/src/test/java/com/objectcomputing/checkins/services/fixture/SkillCategoryFixture.java b/server/src/test/java/com/objectcomputing/checkins/services/fixture/SkillCategoryFixture.java index 00324e0d4b..ac2b6c2da4 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/fixture/SkillCategoryFixture.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/fixture/SkillCategoryFixture.java @@ -4,14 +4,16 @@ public interface SkillCategoryFixture extends RepositoryFixture { - default SkillCategory createDefaultSkillCategory() { - SkillCategory skillCategory = new SkillCategory("Languages", "Programming Languages"); + default SkillCategory createSkillCategory(String name, String description) { + SkillCategory skillCategory = new SkillCategory(name, description); return getSkillCategoryRepository().save(skillCategory); } - default SkillCategory createAnotherSkillCategory() { - SkillCategory skillCategory = new SkillCategory("Libraries", "Libraries used"); - return getSkillCategoryRepository().save(skillCategory); + default SkillCategory createDefaultSkillCategory() { + return createSkillCategory("Languages", "Programming Languages"); } + default SkillCategory createAnotherSkillCategory() { + return createSkillCategory("Libraries", "Libraries used"); + } } diff --git a/server/src/test/java/com/objectcomputing/checkins/services/fixture/SkillFixture.java b/server/src/test/java/com/objectcomputing/checkins/services/fixture/SkillFixture.java index 9378147162..60509bdce2 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/fixture/SkillFixture.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/fixture/SkillFixture.java @@ -4,13 +4,17 @@ public interface SkillFixture extends RepositoryFixture { + default Skill createSkill(String name, boolean isPending, String description, boolean isExtraneous) { + return getSkillRepository().save( + new Skill(name, isPending, description, isExtraneous) + ); + } + default Skill createADefaultSkill() { - return getSkillRepository().save(new Skill("Limb regeneration", true, - "Regenerate a lost limb", false)); + return createSkill("Limb regeneration", true, "Regenerate a lost limb", false); } default Skill createASecondarySkill() { - return getSkillRepository().save(new Skill("Limb restoration", true, - "Restore a lost limb", false)); + return createSkill("Limb restoration", true, "Restore a lost limb", false); } } From 966aa25bb6455c82aa1cec474bf3648005230970 Mon Sep 17 00:00:00 2001 From: Tim Yates Date: Thu, 11 Jul 2024 14:44:04 +0100 Subject: [PATCH 2/3] Update controller test to fetch real data from a real database with no mocking --- .../SkillRecordControllerTest.java | 84 +++++++++++++++---- 1 file changed, 70 insertions(+), 14 deletions(-) diff --git a/server/src/test/java/com/objectcomputing/checkins/services/skill_record/SkillRecordControllerTest.java b/server/src/test/java/com/objectcomputing/checkins/services/skill_record/SkillRecordControllerTest.java index 8a4a2563a5..7f36abb5a2 100644 --- a/server/src/test/java/com/objectcomputing/checkins/services/skill_record/SkillRecordControllerTest.java +++ b/server/src/test/java/com/objectcomputing/checkins/services/skill_record/SkillRecordControllerTest.java @@ -2,7 +2,12 @@ import com.objectcomputing.checkins.services.TestContainersSuite; import com.objectcomputing.checkins.services.fixture.RoleFixture; +import com.objectcomputing.checkins.services.fixture.SkillCategoryFixture; +import com.objectcomputing.checkins.services.fixture.SkillCategorySkillFixture; import com.objectcomputing.checkins.services.fixture.SkillFixture; +import com.objectcomputing.checkins.services.skillcategory.SkillCategory; +import com.objectcomputing.checkins.services.skills.Skill; +import io.micronaut.http.HttpHeaders; import io.micronaut.http.HttpRequest; import io.micronaut.http.HttpResponse; import io.micronaut.http.HttpStatus; @@ -10,47 +15,98 @@ import io.micronaut.http.client.annotation.Client; import io.micronaut.http.client.exceptions.HttpClientResponseException; import jakarta.inject.Inject; +import org.apache.commons.csv.CSVFormat; +import org.apache.commons.csv.CSVParser; +import org.apache.commons.csv.CSVRecord; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; -import java.io.File; +import java.io.StringReader; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; import static com.objectcomputing.checkins.services.role.RoleType.Constants.ADMIN_ROLE; import static com.objectcomputing.checkins.services.role.RoleType.Constants.MEMBER_ROLE; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertAll; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; -class SkillRecordControllerTest extends TestContainersSuite implements RoleFixture, SkillFixture { +class SkillRecordControllerTest extends TestContainersSuite implements RoleFixture, SkillCategoryFixture, SkillCategorySkillFixture, SkillFixture { + + private final Map skills = new HashMap<>(); @Inject @Client("/services/skills/records") private HttpClient client; @BeforeEach - void createRolesAndPermissions() { + void buildSkillHierarchy() { createAndAssignRoles(); + + SkillCategory magicCategory = createSkillCategory("Magic", "Magical skills"); + skills.put("conjuring", createSkill("Conjuring", false, "Conjuring skills", false)); + skills.put("divination", createSkill("Divination", true, "Conjuring skills", false)); + createSkillCategorySkill(magicCategory.getId(), skills.get("conjuring").getId()); + createSkillCategorySkill(magicCategory.getId(), skills.get("divination").getId()); + + SkillCategory programmingCategory = createSkillCategory("Programming", "Programming skills"); + skills.put("java", createSkill("Java", false, "Java programming skills", false)); + skills.put("rust", createSkill("Rust", false, "Rust programming skills", true)); + createSkillCategorySkill(programmingCategory.getId(), skills.get("java").getId()); + createSkillCategorySkill(programmingCategory.getId(), skills.get("rust").getId()); } @Test + @SuppressWarnings("resource") void testGetSuccess() { - HttpRequest request = HttpRequest - .GET("/csv") - .basicAuth(ADMIN_ROLE, ADMIN_ROLE); - HttpResponse response = client.toBlocking().exchange(request, File.class); + HttpRequest request = HttpRequest.GET("/csv").basicAuth(ADMIN_ROLE, ADMIN_ROLE); + HttpResponse response = client.toBlocking().exchange(request, String.class); assertEquals(HttpStatus.OK, response.getStatus()); - assertTrue(response.getBody().isPresent()); + String contentDisposition = response.header(HttpHeaders.CONTENT_DISPOSITION); + assertTrue(contentDisposition.startsWith("attachment; filename=skill_records"), "Unexpected content disposition: " + contentDisposition); + assertTrue(contentDisposition.endsWith(".csv"), "Unexpected content disposition: " + contentDisposition); + + String body = response.body(); + + String[] headers = {"name", "description", "extraneous", "pending", "category_name"}; + CSVFormat csvFormat = CSVFormat.DEFAULT + .builder() + .setHeader(headers) + .setQuote('"') + .setSkipHeaderRecord(true) + .build(); + + CSVParser parser = assertDoesNotThrow(() -> csvFormat.parse(new StringReader(body))); + List records = parser.getRecords(); + + assertEquals(skills.size(), records.size()); + + // There's no order guaranteed in the view, so to avoid flakiness we need to check for the presence of each line + List lines = records.stream().map(r -> r.stream().collect(Collectors.joining(","))).toList(); + List expectations = """ + Conjuring,Conjuring skills,false,false,Magic + Divination,Conjuring skills,false,true,Magic + Java,Java programming skills,false,false,Programming + Rust,Rust programming skills,true,false,Programming""" + .lines() + .map(line -> (Executable) () -> assertTrue(lines.contains(line), "Line not found: " + line)) + .toList(); + assertAll(expectations); } @Test void testGetNotAllowed() { - HttpRequest request = HttpRequest - .GET("/csv") - .basicAuth(MEMBER_ROLE, MEMBER_ROLE); + HttpRequest request = HttpRequest.GET("/csv").basicAuth(MEMBER_ROLE, MEMBER_ROLE); HttpClientResponseException responseException = assertThrows(HttpClientResponseException.class, () -> - client.toBlocking().exchange(request, File.class) + client.toBlocking().exchange(request, String.class) ); assertEquals(HttpStatus.FORBIDDEN, responseException.getStatus()); } - } \ No newline at end of file From 6a6fb6cd2c9282b29f6d796e548c8a3a7f7cd2cb Mon Sep 17 00:00:00 2001 From: Tim Yates Date: Thu, 11 Jul 2024 14:44:55 +0100 Subject: [PATCH 3/3] Remove mock-heavy service test I believe the coverage is exactly the same --- .../SkillRecordServicesImplTest.java | 116 ------------------ 1 file changed, 116 deletions(-) delete mode 100644 server/src/test/java/com/objectcomputing/checkins/services/skill_record/SkillRecordServicesImplTest.java diff --git a/server/src/test/java/com/objectcomputing/checkins/services/skill_record/SkillRecordServicesImplTest.java b/server/src/test/java/com/objectcomputing/checkins/services/skill_record/SkillRecordServicesImplTest.java deleted file mode 100644 index 7738829fec..0000000000 --- a/server/src/test/java/com/objectcomputing/checkins/services/skill_record/SkillRecordServicesImplTest.java +++ /dev/null @@ -1,116 +0,0 @@ -package com.objectcomputing.checkins.services.skill_record; - -import com.objectcomputing.checkins.services.TestContainersSuite; -import org.apache.commons.csv.CSVFormat; -import org.apache.commons.csv.CSVParser; -import org.apache.commons.csv.CSVRecord; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.MethodOrderer; -import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.TestMethodOrder; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; - -import java.io.File; -import java.io.FileReader; -import java.io.IOException; -import java.io.Reader; -import java.util.Collections; -import java.util.List; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.reset; -import static org.mockito.Mockito.when; - -@TestMethodOrder(MethodOrderer.OrderAnnotation.class) -class SkillRecordServicesImplTest extends TestContainersSuite { - - @Mock - private SkillRecordRepository skillRecordRepository; - - @Mock - private SkillRecordFileProvider skillRecordFileProvider; - - @InjectMocks - private SkillRecordServicesImpl skillRecordServices; - - private AutoCloseable mockFinalizer; - - @BeforeAll - void initMocks() { - mockFinalizer = MockitoAnnotations.openMocks(this); - } - - @BeforeEach - void resetMocks() { - reset(skillRecordRepository); - } - - @AfterAll - void close() throws Exception { - mockFinalizer.close(); - } - - @Test - @Order(1) - void testFileGeneration() throws IOException { - SkillRecord record1 = new SkillRecord(); - record1.setName("Java"); - record1.setDescription("Various technical skills"); - record1.setExtraneous(true); - record1.setPending(true); - record1.setCategoryName("Languages, Libraries, and Frameworks"); - - when(skillRecordRepository.findAll()).thenReturn(Collections.singletonList(record1)); - File tmpFile = File.createTempFile("foobar",".csv"); - tmpFile.deleteOnExit(); - when(skillRecordFileProvider.provideFile()).thenReturn(tmpFile); - File file = skillRecordServices.generateFile(); - assertNotNull(file); - - Reader fileReader = new FileReader(file); - - String[] headers = { "name", "description", "extraneous", "pending", "category_name" }; - CSVFormat csvFormat = CSVFormat.DEFAULT - .builder() - .setHeader(headers) - .setQuote('"') - .setSkipHeaderRecord(true) - .build(); - - CSVParser parser = csvFormat.parse(fileReader); - List records = parser.getRecords(); - - assertEquals(1, records.size()); - CSVRecord csvRecord = records.get(0); - assertEquals(record1.getName(), csvRecord.get("name")); - assertEquals(record1.getDescription(), csvRecord.get("description")); - assertEquals(record1.isExtraneous(), Boolean.valueOf(csvRecord.get("extraneous"))); - assertEquals(record1.isPending(), Boolean.valueOf(csvRecord.get("pending"))); - assertEquals(record1.getCategoryName(), csvRecord.get("category_name")); - } - - @Test - @Order(2) - void testNoFileGenerated() { - SkillRecord record1 = new SkillRecord(); - record1.setName("Java"); - record1.setDescription("Various technical skills"); - record1.setExtraneous(true); - record1.setPending(true); - record1.setCategoryName("Languages, Libraries, and Frameworks"); - - when(skillRecordRepository.findAll()).thenReturn(Collections.singletonList(record1)); - - when(skillRecordFileProvider.provideFile()).thenThrow(new RuntimeException()); - assertThrows(RuntimeException.class, () -> { - skillRecordServices.generateFile(); - }); - } -} \ No newline at end of file