From 7772f58dbfdfbe808e0ebd9b90a24f9ae47f4b3f Mon Sep 17 00:00:00 2001 From: aman-agrawal <9412470@gmail.com> Date: Fri, 31 May 2024 17:32:52 +0530 Subject: [PATCH 1/2] refactor(artifact_extractor): moved ArtifactExtractorSpec (groovy) unit tests to ArtifactExtractorTest (java) --- .../igor/artifacts/ArtifactExtractor.java | 4 +- .../artifacts/ArtifactExtractorSpec.groovy | 70 ------------------- .../igor/artifacts/ArtifactExtractorTest.java | 51 ++++++++++++-- 3 files changed, 49 insertions(+), 76 deletions(-) delete mode 100644 igor-web/src/test/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactExtractorSpec.groovy diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactExtractor.java b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactExtractor.java index 375afb5a9..85c081357 100644 --- a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactExtractor.java +++ b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactExtractor.java @@ -19,6 +19,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.netflix.spinnaker.igor.build.model.GenericBuild; +import com.netflix.spinnaker.kork.annotations.VisibleForTesting; import com.netflix.spinnaker.kork.artifacts.model.Artifact; import com.netflix.spinnaker.kork.artifacts.parsing.JinjaArtifactExtractor; import java.util.ArrayList; @@ -91,7 +92,8 @@ private JinjaTemplate getTemplateFromProperty(GenericBuild build) { return jinjaTemplateService.getTemplate(messageFormat, templateType); } - private boolean parseCustomFormat(Object customFormat) { + @VisibleForTesting + boolean parseCustomFormat(Object customFormat) { if (customFormat == null) { return false; } diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactExtractorSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactExtractorSpec.groovy deleted file mode 100644 index c2f5f50ff..000000000 --- a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactExtractorSpec.groovy +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Copyright 2018 Google, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License") - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.netflix.spinnaker.igor.artifacts - -import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule -import com.netflix.spinnaker.igor.build.model.GenericBuild -import com.netflix.spinnaker.kork.artifacts.parsing.DefaultJinjavaFactory -import com.netflix.spinnaker.kork.artifacts.parsing.JinjaArtifactExtractor -import spock.lang.Specification -import spock.lang.Subject - -class ArtifactExtractorSpec extends Specification { - def objectMapper = new ObjectMapper() - def jinjaTemplateService = GroovyMock(JinjaTemplateService) - def jinjaArtifactExtractorFactory = new JinjaArtifactExtractor.Factory(new DefaultJinjavaFactory()) - - void setup() { - objectMapper.registerModule(new JavaTimeModule()) - } - - @Subject - def artifactExtractor = new ArtifactExtractor(objectMapper, jinjaTemplateService, jinjaArtifactExtractorFactory) - - def "parses an artifact returns it artifacts"() { - given: - def properties = [ - group: 'test.group', - artifact: 'test-artifact', - version: '1.0', - messageFormat: 'JAR', - customFormat: 'false' - ] - def build = new GenericBuild() - build.properties = properties - - when: - def artifacts = artifactExtractor.extractArtifacts(build) - - then: - artifacts.size() == 1 - artifacts[0].name == "test-artifact-1.0" - } - - def "parseCustomFormat correctly parses booleans and strings"() { - expect: - result == artifactExtractor.parseCustomFormat(customFormat) - - where: - customFormat || result - true || true - false || false - "true" || true - "false" || false - } -} diff --git a/igor-web/src/test/java/com/netflix/spinnaker/igor/artifacts/ArtifactExtractorTest.java b/igor-web/src/test/java/com/netflix/spinnaker/igor/artifacts/ArtifactExtractorTest.java index 12ae4f48a..bf4750d17 100644 --- a/igor-web/src/test/java/com/netflix/spinnaker/igor/artifacts/ArtifactExtractorTest.java +++ b/igor-web/src/test/java/com/netflix/spinnaker/igor/artifacts/ArtifactExtractorTest.java @@ -1,16 +1,24 @@ package com.netflix.spinnaker.igor.artifacts; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import com.netflix.spinnaker.igor.Main; import com.netflix.spinnaker.igor.build.model.GenericBuild; import com.netflix.spinnaker.igor.build.model.GenericGitRevision; import com.netflix.spinnaker.kork.artifacts.model.Artifact; +import com.netflix.spinnaker.kork.artifacts.parsing.DefaultJinjavaFactory; +import com.netflix.spinnaker.kork.artifacts.parsing.JinjaArtifactExtractor; import java.time.Instant; import java.util.List; import java.util.Map; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.test.context.TestPropertySource; @@ -27,16 +35,33 @@ public class ArtifactExtractorTest { @Autowired private ArtifactExtractor artifactExtractor; - @Test - public void should_be_able_to_serialize_jsr310_dates() { - GenericBuild build = new GenericBuild(); - build.setProperties( + @Autowired private JinjaTemplateService jinjaTemplateService; + + private JinjaArtifactExtractor.Factory jinjaArtifactExtractorFactory; + private ObjectMapper objectMapper; + private GenericBuild build; + + @BeforeEach + public void setUp() { + objectMapper = new ObjectMapper(); + objectMapper.registerModule(new JavaTimeModule()); + jinjaArtifactExtractorFactory = new JinjaArtifactExtractor.Factory(new DefaultJinjavaFactory()); + artifactExtractor = + new ArtifactExtractor(objectMapper, jinjaTemplateService, jinjaArtifactExtractorFactory); + + build = new GenericBuild(); + Map properties = Map.of( "group", "test.group", "artifact", "test-artifact", "version", "1.0", "messageFormat", "JAR", - "customFormat", "false")); + "customFormat", "false"); + build.setProperties(properties); + } + + @Test + public void should_be_able_to_serialize_jsr310_dates() { build.setGenericGitRevisions( List.of( GenericGitRevision.builder() @@ -51,4 +76,20 @@ public void should_be_able_to_serialize_jsr310_dates() { assertThat(artifact.getName()).isEqualTo("test-artifact-1.0"); assertThat(artifact.getVersion()).isEqualTo("1.0"); } + + @Test + public void testParsesAnArtifactReturnsItsArtifacts() { + List artifacts = artifactExtractor.extractArtifacts(build); + + assertEquals(1, artifacts.size()); + assertEquals("test-artifact-1.0", artifacts.get(0).getName()); + } + + @ParameterizedTest + @CsvSource({"true, true", "false, false", "'true', true", "'false', false"}) + public void testParseCustomFormatCorrectlyParsesBooleansAndStrings( + Object customFormat, boolean expectedResult) { + boolean result = artifactExtractor.parseCustomFormat(customFormat); + assertThat(result).isEqualTo(expectedResult); + } } From 9ba2ef4e9cabdaa232808cb0b0c24b2446889e00 Mon Sep 17 00:00:00 2001 From: aman-agrawal <9412470@gmail.com> Date: Sat, 1 Jun 2024 07:02:37 +0530 Subject: [PATCH 2/2] refactor(artifact_service): moved ArtifactServiceSpec (groovy) unit tests to ArtifactServiceTest (java) --- .../igor/artifacts/ArtifactServiceSpec.groovy | 105 ------------------ .../igor/artifacts/ArtifactServiceTest.java | 100 +++++++++++++++++ 2 files changed, 100 insertions(+), 105 deletions(-) delete mode 100644 igor-web/src/test/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactServiceSpec.groovy create mode 100644 igor-web/src/test/java/com/netflix/spinnaker/igor/artifacts/ArtifactServiceTest.java diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactServiceSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactServiceSpec.groovy deleted file mode 100644 index e135b4eaf..000000000 --- a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/artifacts/ArtifactServiceSpec.groovy +++ /dev/null @@ -1,105 +0,0 @@ -/* - * - * Copyright 2019 Netflix, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License") - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ -package com.netflix.spinnaker.igor.artifacts - -import com.netflix.spinnaker.kork.web.exceptions.NotFoundException -import spock.lang.Specification -import spock.lang.Subject - -import static org.assertj.core.api.Assertions.assertThat - -class ArtifactServiceSpec extends Specification { - - @Subject - ArtifactServices artifactServices = new ArtifactServices() - - void setup() { - Map services = new HashMap<>() - services.put("artifactory", new TestArtifactService()) - artifactServices.addServices(services) - } - - def "finds matching service"() { - when: - def service = artifactServices.getService("artifactory") - - then: - assertThat(service).isNotNull() - } - - def "does not find a non-matching service"() { - when: - def service = artifactServices.getService("what") - - then: - assertThat(service).isNull() - } - - def "service finds artifact versions"() { - when: - def service = artifactServices.getService("artifactory") - def versions = service.getArtifactVersions("deb","test", null) - - then: - assertThat(versions).isNotNull() - assertThat(versions).isNotEmpty() - versions.size() > 0 - } - - def "service finds only snapshot artifacts"() { - when: - def service = artifactServices.getService("artifactory") - def versions = service.getArtifactVersions("deb","test", "snapshot") - - then: - assertThat(versions).isNotNull() - assertThat(versions).isNotEmpty() - versions.size() == 1 - } - - def "service finds artifact"() { - when: - def service = artifactServices.getService("artifactory") - def artifact = service.getArtifact("deb","test", "v0.4.0") - - then: - assertThat(artifact).isNotNull() - artifact.name.equals("test") - artifact.version.equals("v0.4.0") - } - - def "versions list is empty when no versions found"() { - when: - def service = artifactServices.getService("artifactory") - def versions = service.getArtifactVersions("deb","blah", "") - - then: - assertThat(versions).isNotNull() - assertThat(versions).isEmpty() - } - - def "404 is thrown when artifact not found"() { - when: - def service = artifactServices.getService("artifactory") - service.getArtifact("deb","blah","v0.0.1") - - then: - thrown(NotFoundException) - } - -} diff --git a/igor-web/src/test/java/com/netflix/spinnaker/igor/artifacts/ArtifactServiceTest.java b/igor-web/src/test/java/com/netflix/spinnaker/igor/artifacts/ArtifactServiceTest.java new file mode 100644 index 000000000..a32ca45e7 --- /dev/null +++ b/igor-web/src/test/java/com/netflix/spinnaker/igor/artifacts/ArtifactServiceTest.java @@ -0,0 +1,100 @@ +/* + * + * Copyright 2019 Netflix, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License") + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package com.netflix.spinnaker.igor.artifacts; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.netflix.spinnaker.kork.artifacts.model.Artifact; +import com.netflix.spinnaker.kork.web.exceptions.NotFoundException; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class ArtifactServiceTest { + + private ArtifactServices artifactServices; + + @BeforeEach + public void setUp() { + artifactServices = new ArtifactServices(); + Map services = new HashMap<>(); + services.put("artifactory", new TestArtifactService()); + artifactServices.addServices(services); + } + + @Test + public void findsMatchingService() { + ArtifactService service = artifactServices.getService("artifactory"); + assertThat(service).isNotNull(); + } + + @Test + public void doesNotFindNonMatchingService() { + ArtifactService service = artifactServices.getService("what"); + assertThat(service).isNull(); + } + + @Test + public void serviceFindsArtifactVersions() { + ArtifactService service = artifactServices.getService("artifactory"); + List versions = service.getArtifactVersions("deb", "test", (List) null); + + assertThat(versions).isNotNull(); + assertThat(versions).isNotEmpty(); + assertThat(versions.size()).isGreaterThan(0); + } + + @Test + public void serviceFindsOnlySnapshotArtifacts() { + ArtifactService service = artifactServices.getService("artifactory"); + List versions = service.getArtifactVersions("deb", "test", "snapshot"); + + assertThat(versions).isNotNull(); + assertThat(versions).isNotEmpty(); + assertThat(versions.size()).isEqualTo(1); + } + + @Test + public void serviceFindsArtifact() { + ArtifactService service = artifactServices.getService("artifactory"); + Artifact artifact = service.getArtifact("deb", "test", "v0.4.0"); + + assertThat(artifact).isNotNull(); + assertThat(artifact.getName()).isEqualTo("test"); + assertThat(artifact.getVersion()).isEqualTo("v0.4.0"); + } + + @Test + public void versionsListIsEmptyWhenNoVersionsFound() { + ArtifactService service = artifactServices.getService("artifactory"); + List versions = service.getArtifactVersions("deb", "blah", ""); + + assertThat(versions).isNotNull(); + assertThat(versions).isEmpty(); + } + + @Test + public void notFoundExceptionIsThrownWhenArtifactNotFound() { + ArtifactService service = artifactServices.getService("artifactory"); + assertThrows(NotFoundException.class, () -> service.getArtifact("deb", "blah", "v0.0.1")); + } +}