From fae5d30a0a88b0745d74f22a710d8422d7a4abd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jiri=20Dan=C4=9Bk?= Date: Thu, 21 Mar 2024 22:10:57 +0100 Subject: [PATCH 1/2] Add AssertJ dependency and rewrite several tests to use it instead of Hamcrest --- pom.xml | 12 ++++++++ .../odh/test/e2e/standard/ModelServingST.java | 14 ++++----- .../test/e2e/standard/PipelineServerST.java | 29 +++++++++---------- .../test/e2e/standard/PipelineV2ServerST.java | 29 +++++++++---------- 4 files changed, 44 insertions(+), 40 deletions(-) diff --git a/pom.xml b/pom.xml index 342c3ec6..45323d50 100644 --- a/pom.xml +++ b/pom.xml @@ -63,6 +63,7 @@ 1.9.21.2 2.25.0 2.12.0 + 3.24.2 @@ -174,6 +175,12 @@ compile ${hamcrest.version} + + org.assertj + assertj-core + ${assertj.version} + test + ch.qos.logback logback-classic @@ -473,6 +480,11 @@ allure-hamcrest runtime + + io.qameta.allure + allure-assertj + runtime + io.qameta.allure allure-okhttp3 diff --git a/src/test/java/io/odh/test/e2e/standard/ModelServingST.java b/src/test/java/io/odh/test/e2e/standard/ModelServingST.java index cdb95ef8..61473269 100644 --- a/src/test/java/io/odh/test/e2e/standard/ModelServingST.java +++ b/src/test/java/io/odh/test/e2e/standard/ModelServingST.java @@ -39,8 +39,6 @@ import io.skodjob.annotations.SuiteDoc; import io.skodjob.annotations.TestDoc; import lombok.SneakyThrows; -import org.hamcrest.Matchers; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.slf4j.Logger; @@ -70,7 +68,7 @@ import static io.odh.test.TestConstants.GLOBAL_TIMEOUT; import static io.odh.test.TestUtils.DEFAULT_TIMEOUT_DURATION; import static io.odh.test.TestUtils.DEFAULT_TIMEOUT_UNIT; -import static org.hamcrest.MatcherAssert.assertThat; +import static org.assertj.core.api.Assertions.assertThat; @SuppressWarnings({"checkstyle:ClassFanOutComplexity"}) @SuiteDoc( @@ -227,11 +225,11 @@ void testMultiModelServerInference() { ServingRuntime processModelServerTemplate(String templateName) { TemplateResource templateResource = kubeClient.templates().inNamespace(OdhConstants.CONTROLLERS_NAMESPACE).withName(templateName); Template template = templateResource.get(); - Assertions.assertNotNull(template); - Assertions.assertEquals(0, template.getParameters().size()); + assertThat(template).isNotNull(); + assertThat(template.getParameters()).isEmpty(); List instances = templateResource.process().getItems(); - Assertions.assertEquals(1, instances.size()); + assertThat(instances).hasSize(1); return instances.stream().map(it -> { GenericKubernetesResource genericKubernetesResource = (GenericKubernetesResource) it; // WORKAROUND(RHOAIENG-4547) ServingRuntime should not have top level `labels` key @@ -274,8 +272,8 @@ void queryModelAndCheckMnistInference(String baseUrl, String modelInputPath, Str .build(); HttpResponse inferResponse = httpClient.send(inferRequest, HttpResponse.BodyHandlers.ofString()); - assertThat(inferResponse.body(), inferResponse.statusCode(), Matchers.is(200)); - assertThat(inferResponse.body(), Matchers.containsString(expectedModelOutput)); + assertThat(inferResponse.statusCode()).as(inferResponse.body()).isEqualTo(200); + assertThat(inferResponse.body()).contains(expectedModelOutput); } private T castResource(KubernetesResource value, Class type) { diff --git a/src/test/java/io/odh/test/e2e/standard/PipelineServerST.java b/src/test/java/io/odh/test/e2e/standard/PipelineServerST.java index b0ec25a2..a9c13b5e 100644 --- a/src/test/java/io/odh/test/e2e/standard/PipelineServerST.java +++ b/src/test/java/io/odh/test/e2e/standard/PipelineServerST.java @@ -40,8 +40,6 @@ import io.skodjob.annotations.Step; import io.skodjob.annotations.SuiteDoc; import io.skodjob.annotations.TestDoc; -import org.hamcrest.Matchers; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.slf4j.Logger; @@ -49,10 +47,9 @@ import java.io.IOException; import java.util.List; -import java.util.stream.Collectors; import java.util.stream.Stream; -import static org.hamcrest.MatcherAssert.assertThat; +import static org.assertj.core.api.Assertions.assertThat; @SuiteDoc( description = @Desc("Verifies simple setup of ODH by spin-up operator, setup DSCI, and setup DSC."), @@ -231,18 +228,18 @@ void testUserCanCreateRunAndDeleteADSPipelineFromDSProject() throws IOException KFPv1Client.Pipeline importedPipeline = kfpv1Client.importPipeline(pipelineTestName, pipelineTestDesc, pipelineTestFilepath); List pipelines = kfpv1Client.listPipelines(); - assertThat(pipelines.stream().map(p -> p.name).collect(Collectors.toList()), Matchers.contains(pipelineTestName)); + assertThat(pipelines).extracting(p -> p.name).contains(pipelineTestName); KFPv1Client.PipelineRun pipelineRun = kfpv1Client.runPipeline(pipelineTestRunBasename, importedPipeline.id, "Immediate"); - Assertions.assertTrue(pipelineRun.pipelineSpec.workflowManifest.contains(pipelineWorkflowName)); + assertThat(pipelineRun.pipelineSpec.workflowManifest).contains(pipelineWorkflowName); kfpv1Client.waitForPipelineRun(pipelineRun.id); List statuses = kfpv1Client.getPipelineRunStatus(); - assertThat(statuses.stream() - .filter(run -> run.id.equals(pipelineRun.id)) - .map(run -> run.status) - .findFirst().get(), Matchers.is("Succeeded")); + assertThat(statuses) + .filteredOn(run -> run.id.equals(pipelineRun.id)) + .extracting(run -> run.status) + .singleElement().isEqualTo("Succeeded"); checkPipelineRunK8sDeployments(prjTitle, pipelineWorkflowName + "-" + pipelineRun.id.substring(0, 5)); @@ -261,16 +258,16 @@ private void checkPipelineRunK8sDeployments(String prjTitle, String workflowName ).map(pod -> pod.list().getItems()).toList(); for (List pods : tektonTaskPods) { - Assertions.assertEquals(1, pods.size()); - Assertions.assertEquals("Succeeded", pods.get(0).getStatus().getPhase()); + assertThat(pods).hasSize(1); + assertThat(pods.get(0).getStatus().getPhase()).isEqualTo("Succeeded"); List containerStatuses = pods.get(0).getStatus().getContainerStatuses(); - Assertions.assertNotEquals(0, containerStatuses.size()); + assertThat(containerStatuses).isNotEmpty(); for (ContainerStatus containerStatus : containerStatuses) { ContainerStateTerminated terminated = containerStatus.getState().getTerminated(); - Assertions.assertNotNull(terminated); - Assertions.assertEquals(0, terminated.getExitCode()); - Assertions.assertEquals("Completed", terminated.getReason()); + assertThat(terminated).isNotNull(); + assertThat(terminated.getExitCode()).isEqualTo(0); + assertThat(terminated.getReason()).isEqualTo("Completed"); } } } diff --git a/src/test/java/io/odh/test/e2e/standard/PipelineV2ServerST.java b/src/test/java/io/odh/test/e2e/standard/PipelineV2ServerST.java index 33319956..fa958276 100644 --- a/src/test/java/io/odh/test/e2e/standard/PipelineV2ServerST.java +++ b/src/test/java/io/odh/test/e2e/standard/PipelineV2ServerST.java @@ -59,8 +59,6 @@ import io.skodjob.annotations.Step; import io.skodjob.annotations.SuiteDoc; import io.skodjob.annotations.TestDoc; -import org.hamcrest.Matchers; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.slf4j.Logger; @@ -69,9 +67,8 @@ import java.io.IOException; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; -import static org.hamcrest.MatcherAssert.assertThat; +import static org.assertj.core.api.Assertions.assertThat; @SuppressWarnings({"checkstyle:ClassFanOutComplexity"}) @SuiteDoc( @@ -302,7 +299,7 @@ void testUserCanOperateDSv2PipelineFromDSProject() throws IOException { KFPv2Client.Pipeline importedPipeline = kfpClient.importPipeline(pipelineTestName, pipelineTestDesc, pipelineTestFilepath); List pipelines = kfpClient.listPipelines(); - assertThat(pipelines.stream().map(p -> p.displayName).collect(Collectors.toList()), Matchers.contains(pipelineTestName)); + assertThat(pipelines).extracting(p -> p.displayName).contains(pipelineTestName); Map parameters = Map.of( "min_max_scaler", false, @@ -314,10 +311,10 @@ void testUserCanOperateDSv2PipelineFromDSProject() throws IOException { kfpClient.waitForPipelineRun(pipelineRun.runId); List statuses = kfpClient.getPipelineRunStatus(); - assertThat(statuses.stream() - .filter(run -> run.runId.equals(pipelineRun.runId)) - .map(run -> run.state) - .findFirst().orElseThrow(), Matchers.is("SUCCEEDED")); + assertThat(statuses) + .filteredOn(run -> run.runId.equals(pipelineRun.runId)) + .extracting(run -> run.state) + .singleElement().isEqualTo("SUCCEEDED"); checkPipelineRunK8sDeployments(prjTitle, pipelineRun.runId); @@ -342,18 +339,18 @@ private static void deletePreexistingPipelinesAndVersions(KFPv2Client kfpClient) @io.qameta.allure.Step private void checkPipelineRunK8sDeployments(String prjTitle, String runId) { List argoTaskPods = client.pods().inNamespace(prjTitle).withLabel("pipeline/runid=" + runId).list().getItems(); - Assertions.assertEquals(7, argoTaskPods.size()); + assertThat(argoTaskPods).hasSize(7); for (Pod pod : argoTaskPods) { - Assertions.assertEquals("Succeeded", pod.getStatus().getPhase()); + assertThat(pod.getStatus().getPhase()).isEqualTo("Succeeded"); List containerStatuses = pod.getStatus().getContainerStatuses(); - Assertions.assertNotEquals(0, containerStatuses.size()); + assertThat(containerStatuses).isNotEmpty(); for (ContainerStatus containerStatus : containerStatuses) { ContainerStateTerminated terminated = containerStatus.getState().getTerminated(); - Assertions.assertNotNull(terminated); - Assertions.assertEquals(0, terminated.getExitCode()); - Assertions.assertEquals("Completed", terminated.getReason()); + assertThat(terminated).isNotNull(); + assertThat(terminated.getExitCode()).isEqualTo(0); + assertThat(terminated.getReason()).isEqualTo("Completed"); } } @@ -369,7 +366,7 @@ private void checkPipelineRunK8sDeployments(String prjTitle, String runId) { List argoNodeNames = argoTaskPods.stream() .map(pod -> pod.getMetadata().getAnnotations().get("workflows.argoproj.io/node-name")) .toList(); - Assertions.assertIterableEquals(expectedNodeNames.stream().sorted().toList(), argoNodeNames.stream().sorted().toList(), argoNodeNames.toString()); + assertThat(argoNodeNames).containsExactlyInAnyOrderElementsOf(expectedNodeNames); } @io.qameta.allure.Step From 52297d3996b83bbde92504424601bf9601c5ab1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jiri=20Dan=C4=9Bk?= Date: Thu, 21 Mar 2024 22:32:11 +0100 Subject: [PATCH 2/2] Introduce soft assertions with AssertJ where appropriate --- .../odh/test/e2e/standard/ModelServingST.java | 11 ++++++++--- .../test/e2e/standard/PipelineServerST.java | 17 +++++++++++------ .../test/e2e/standard/PipelineV2ServerST.java | 19 ++++++++++++------- .../test/e2e/standard/StandardAbstract.java | 2 ++ 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/test/java/io/odh/test/e2e/standard/ModelServingST.java b/src/test/java/io/odh/test/e2e/standard/ModelServingST.java index 61473269..db51aada 100644 --- a/src/test/java/io/odh/test/e2e/standard/ModelServingST.java +++ b/src/test/java/io/odh/test/e2e/standard/ModelServingST.java @@ -39,6 +39,8 @@ import io.skodjob.annotations.SuiteDoc; import io.skodjob.annotations.TestDoc; import lombok.SneakyThrows; +import org.assertj.core.api.SoftAssertions; +import org.assertj.core.api.junit.jupiter.InjectSoftAssertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.slf4j.Logger; @@ -89,6 +91,9 @@ public class ModelServingST extends StandardAbstract { private static final Logger LOGGER = LoggerFactory.getLogger(ModelServingST.class); + @InjectSoftAssertions + private SoftAssertions softly; + private static final String DS_PROJECT_NAME = "test-model-serving"; private final OpenShiftClient kubeClient = (OpenShiftClient) ResourceManager.getKubeClient().getClient(); @@ -229,7 +234,7 @@ ServingRuntime processModelServerTemplate(String templateName) { assertThat(template.getParameters()).isEmpty(); List instances = templateResource.process().getItems(); - assertThat(instances).hasSize(1); + softly.assertThat(instances).hasSize(1); return instances.stream().map(it -> { GenericKubernetesResource genericKubernetesResource = (GenericKubernetesResource) it; // WORKAROUND(RHOAIENG-4547) ServingRuntime should not have top level `labels` key @@ -272,8 +277,8 @@ void queryModelAndCheckMnistInference(String baseUrl, String modelInputPath, Str .build(); HttpResponse inferResponse = httpClient.send(inferRequest, HttpResponse.BodyHandlers.ofString()); - assertThat(inferResponse.statusCode()).as(inferResponse.body()).isEqualTo(200); - assertThat(inferResponse.body()).contains(expectedModelOutput); + softly.assertThat(inferResponse.statusCode()).as(inferResponse.body()).isEqualTo(200); + softly.assertThat(inferResponse.body()).contains(expectedModelOutput); } private T castResource(KubernetesResource value, Class type) { diff --git a/src/test/java/io/odh/test/e2e/standard/PipelineServerST.java b/src/test/java/io/odh/test/e2e/standard/PipelineServerST.java index a9c13b5e..4f18b0ee 100644 --- a/src/test/java/io/odh/test/e2e/standard/PipelineServerST.java +++ b/src/test/java/io/odh/test/e2e/standard/PipelineServerST.java @@ -40,6 +40,8 @@ import io.skodjob.annotations.Step; import io.skodjob.annotations.SuiteDoc; import io.skodjob.annotations.TestDoc; +import org.assertj.core.api.SoftAssertions; +import org.assertj.core.api.junit.jupiter.InjectSoftAssertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.slf4j.Logger; @@ -68,6 +70,9 @@ public class PipelineServerST extends StandardAbstract { private static final Logger LOGGER = LoggerFactory.getLogger(PipelineServerST.class); + @InjectSoftAssertions + private SoftAssertions softly; + private static final String DS_PROJECT_NAME = "test-pipelines"; private final ResourceManager resourceManager = ResourceManager.getInstance(); @@ -258,16 +263,16 @@ private void checkPipelineRunK8sDeployments(String prjTitle, String workflowName ).map(pod -> pod.list().getItems()).toList(); for (List pods : tektonTaskPods) { - assertThat(pods).hasSize(1); - assertThat(pods.get(0).getStatus().getPhase()).isEqualTo("Succeeded"); + softly.assertThat(pods).hasSize(1); + softly.assertThat(pods.get(0).getStatus().getPhase()).isEqualTo("Succeeded"); List containerStatuses = pods.get(0).getStatus().getContainerStatuses(); - assertThat(containerStatuses).isNotEmpty(); + softly.assertThat(containerStatuses).isNotEmpty(); for (ContainerStatus containerStatus : containerStatuses) { ContainerStateTerminated terminated = containerStatus.getState().getTerminated(); - assertThat(terminated).isNotNull(); - assertThat(terminated.getExitCode()).isEqualTo(0); - assertThat(terminated.getReason()).isEqualTo("Completed"); + softly.assertThat(terminated).isNotNull(); + softly.assertThat(terminated.getExitCode()).isEqualTo(0); + softly.assertThat(terminated.getReason()).isEqualTo("Completed"); } } } diff --git a/src/test/java/io/odh/test/e2e/standard/PipelineV2ServerST.java b/src/test/java/io/odh/test/e2e/standard/PipelineV2ServerST.java index fa958276..9e962b1f 100644 --- a/src/test/java/io/odh/test/e2e/standard/PipelineV2ServerST.java +++ b/src/test/java/io/odh/test/e2e/standard/PipelineV2ServerST.java @@ -59,6 +59,8 @@ import io.skodjob.annotations.Step; import io.skodjob.annotations.SuiteDoc; import io.skodjob.annotations.TestDoc; +import org.assertj.core.api.SoftAssertions; +import org.assertj.core.api.junit.jupiter.InjectSoftAssertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.slf4j.Logger; @@ -88,6 +90,9 @@ public class PipelineV2ServerST extends StandardAbstract { private static final Logger LOGGER = LoggerFactory.getLogger(PipelineV2ServerST.class); + @InjectSoftAssertions + private SoftAssertions softly; + private static final String DS_PROJECT_NAME = "test-pipelines"; private final KubernetesClient client = ResourceManager.getKubeClient().getClient(); @@ -339,18 +344,18 @@ private static void deletePreexistingPipelinesAndVersions(KFPv2Client kfpClient) @io.qameta.allure.Step private void checkPipelineRunK8sDeployments(String prjTitle, String runId) { List argoTaskPods = client.pods().inNamespace(prjTitle).withLabel("pipeline/runid=" + runId).list().getItems(); - assertThat(argoTaskPods).hasSize(7); + softly.assertThat(argoTaskPods).hasSize(7); for (Pod pod : argoTaskPods) { - assertThat(pod.getStatus().getPhase()).isEqualTo("Succeeded"); + softly.assertThat(pod.getStatus().getPhase()).isEqualTo("Succeeded"); List containerStatuses = pod.getStatus().getContainerStatuses(); - assertThat(containerStatuses).isNotEmpty(); + softly.assertThat(containerStatuses).isNotEmpty(); for (ContainerStatus containerStatus : containerStatuses) { ContainerStateTerminated terminated = containerStatus.getState().getTerminated(); - assertThat(terminated).isNotNull(); - assertThat(terminated.getExitCode()).isEqualTo(0); - assertThat(terminated.getReason()).isEqualTo("Completed"); + softly.assertThat(terminated).isNotNull(); + softly.assertThat(terminated.getExitCode()).isEqualTo(0); + softly.assertThat(terminated.getReason()).isEqualTo("Completed"); } } @@ -366,7 +371,7 @@ private void checkPipelineRunK8sDeployments(String prjTitle, String runId) { List argoNodeNames = argoTaskPods.stream() .map(pod -> pod.getMetadata().getAnnotations().get("workflows.argoproj.io/node-name")) .toList(); - assertThat(argoNodeNames).containsExactlyInAnyOrderElementsOf(expectedNodeNames); + softly.assertThat(argoNodeNames).containsExactlyInAnyOrderElementsOf(expectedNodeNames); } @io.qameta.allure.Step diff --git a/src/test/java/io/odh/test/e2e/standard/StandardAbstract.java b/src/test/java/io/odh/test/e2e/standard/StandardAbstract.java index a9f85349..a9c5b636 100644 --- a/src/test/java/io/odh/test/e2e/standard/StandardAbstract.java +++ b/src/test/java/io/odh/test/e2e/standard/StandardAbstract.java @@ -12,6 +12,7 @@ import io.odh.test.install.BundleInstall; import io.odh.test.install.InstallTypes; import io.odh.test.install.OlmInstall; +import org.assertj.core.api.junit.jupiter.SoftAssertionsExtension; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.extension.ExtendWith; @@ -26,6 +27,7 @@ @Tag(TestSuite.STANDARD) @ExtendWith(OdhResourceCleaner.class) @ExtendWith(ResourceManagerDeleteHandler.class) +@ExtendWith(SoftAssertionsExtension.class) public abstract class StandardAbstract extends Abstract { private static final Logger LOGGER = LoggerFactory.getLogger(StandardAbstract.class);