Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/pull-request-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
java: [ 8, 11, 17, 21 ]
java: [ 11, 17, 21 ]
fail-fast: false

steps:
- uses: actions/checkout@v3
- name: Set up JDK 8
- name: Set up JDK 11
uses: actions/setup-java@v3
with:
distribution: 'temurin'
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/xtf-maven-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ jobs:

steps:
- uses: actions/checkout@v1
- name: Set up JDK 8
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: 8
java-version: 11
java-package: jdk
architecture: x64
- name: Check well-formatted code
Expand Down
5 changes: 0 additions & 5 deletions builder/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@
<scope>provided</scope>
</dependency>

<dependency>
<groupId>io.fabric8</groupId>
<artifactId>openshift-server-mock</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

</project>

This file was deleted.

106 changes: 106 additions & 0 deletions builder/src/test/java/cz/xtf/builder/openshift/smoke/BuildersTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package cz.xtf.builder.openshift.smoke;

import java.util.Collections;
import java.util.Map;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

import cz.xtf.builder.builders.ConfigMapBuilder;
import cz.xtf.builder.builders.DeploymentConfigBuilder;
import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.PodSpec;
import io.fabric8.openshift.api.model.DeploymentConfig;

/**
* Unit tests for XTF builder classes.
* Tests verify that builders correctly construct Kubernetes/OpenShift resource objects.
*
* Replaces BasicOpenShiftTest which used OpenShiftServer mock (removed in Fabric8 7.4.0).
*/
public class BuildersTest {

private static final String TEST_RESOURCE_LABEL_NAME_APP = "app";
private static final String TEST_RESOURCE_LABEL_VALUE_APP = "xtf-core-test-openshift-mocked-smoke";
private final static Map<String, String> TEST_RESOURCE_LABELS = Collections.singletonMap(
TEST_RESOURCE_LABEL_NAME_APP, TEST_RESOURCE_LABEL_VALUE_APP);

private static final String TEST_CONFIGMAP_NAME = "test-configmap";
private static final String TEST_DEPLOYMENT_CONFIG_NAME = "test-deployment-config";
private static final Integer TEST_DEPLOYMENT_CONFIG_REPLICAS = 3;

@Test
public void testConfigMapBuilder() {
// arrange
final String dataEntryKey = "test.properties";
final String dataEntryValue = "foo=bar";
final ConfigMap configMap = new ConfigMapBuilder(TEST_CONFIGMAP_NAME)
.addLabels(TEST_RESOURCE_LABELS)
.configEntry(dataEntryKey, dataEntryValue)
.build();

// assert - verify the built ConfigMap structure
Assertions.assertNotNull(configMap, "ConfigMap resource creation failed.");
Assertions.assertEquals(TEST_CONFIGMAP_NAME,
configMap.getMetadata().getName(),
"ConfigMap resource has unexpected name.");
Comment on lines +37 to +46
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Extend builder tests to assert labels (and potentially triggers) to better cover builder behavior

Currently these tests only assert name/data for ConfigMap and a few fields for DeploymentConfig. Since labels are part of the builder contract (and you pass TEST_RESOURCE_LABELS), please also assert that these labels are present in metadata.labels for both resources. For DeploymentConfigBuilder, also verify that the image change trigger configured via onImageChange() appears in .spec.triggers with the expected type, so these tests fully cover the intended builder behavior and can reliably replace BasicOpenShiftTest.

Suggested implementation:

        Assertions.assertEquals(TEST_CONFIGMAP_NAME,
                configMap.getMetadata().getName(),
                "ConfigMap resource has unexpected name.");
        Assertions.assertEquals(TEST_RESOURCE_LABELS,
                configMap.getMetadata().getLabels(),
                "ConfigMap resource has unexpected labels.");

To fully implement the requested coverage for the DeploymentConfigBuilder as per your review comment, make the following changes in the same test class, inside the testDeploymentConfigBuilder (or equivalent) method that builds and asserts the DeploymentConfig:

  1. Assert labels on the DeploymentConfig metadata

    After the existing assertion that checks the DeploymentConfig name (and before or after the replicas assertion), add:

    Assertions.assertEquals(TEST_RESOURCE_LABELS,
            deploymentConfig.getMetadata().getLabels(),
            "DeploymentConfig resource has unexpected labels.");

    This assumes the variable holding the built resource is named deploymentConfig. Adjust the variable name if different.

  2. Assert the presence of the ImageChange trigger

    After the basic structural assertions (name, replicas, etc.), add assertions that verify the ImageChange trigger created by .onImageChange() is present in .spec.triggers:

    Assertions.assertNotNull(deploymentConfig.getSpec(), "DeploymentConfig spec should not be null.");
    Assertions.assertNotNull(deploymentConfig.getSpec().getTriggers(), "DeploymentConfig triggers should not be null.");
    
    boolean hasImageChangeTrigger = deploymentConfig.getSpec().getTriggers()
            .stream()
            .anyMatch(trigger -> "ImageChange".equals(trigger.getType()));
    
    Assertions.assertTrue(hasImageChangeTrigger,
            "DeploymentConfig should contain an ImageChange trigger configured via onImageChange().");

    If your test currently doesn’t import java.util.stream.* or similar, no extra imports are needed because stream() is available on List. Ensure the deploymentConfig type is io.fabric8.openshift.api.model.DeploymentConfig (or compatible) so getSpec().getTriggers() and getType() on the trigger objects are available.

These additions will ensure the tests cover both label propagation and trigger configuration behavior for the builders, so they can reliably replace the previous BasicOpenShiftTest coverage.

Assertions.assertEquals(1,
configMap.getData().entrySet().size(),
String.format("ConfigMap resource has unexpected data size: %d",
configMap.getData().entrySet().size()));

// safe now
final Map.Entry<String, String> dataEntry = configMap.getData().entrySet().stream().findFirst().get();
Assertions.assertEquals(dataEntryKey,
dataEntry.getKey(),
String.format("ConfigMap resource has unexpected data entry key: %s", dataEntry.getKey()));
Assertions.assertEquals(dataEntryValue,
dataEntry.getValue(),
String.format("ConfigMap resource has unexpected data entry value: %s", dataEntry.getValue()));
}

@Test
public void testDeploymentConfigBuilder() {
// arrange
final DeploymentConfig deploymentConfig = new DeploymentConfigBuilder(TEST_DEPLOYMENT_CONFIG_NAME)
.addLabels(TEST_RESOURCE_LABELS)
.setReplicas(TEST_DEPLOYMENT_CONFIG_REPLICAS)
.onImageChange()
.setRecreateStrategy()
.build();

// assert - verify the built DeploymentConfig structure
Assertions.assertNotNull(deploymentConfig, "DeploymentConfig resource creation failed.");
Assertions.assertEquals(TEST_DEPLOYMENT_CONFIG_NAME,
deploymentConfig.getMetadata().getName(),
String.format("DeploymentConfig resource has unexpected name: %s.",
deploymentConfig.getMetadata().getName()));
Assertions.assertEquals(TEST_RESOURCE_LABELS,
deploymentConfig.getMetadata().getLabels(),
"DeploymentConfig resource has unexpected labels.");
Assertions.assertNotNull(deploymentConfig.getSpec(),
String.format("DeploymentConfig resource has null \".spec\"", deploymentConfig.getSpec()));
Assertions.assertEquals(TEST_DEPLOYMENT_CONFIG_REPLICAS,
deploymentConfig.getSpec().getReplicas(),
String.format("DeploymentConfig resource has unexpected \".spec.replicas\": %s.",
deploymentConfig.getSpec().getReplicas()));
Assertions.assertNotNull(deploymentConfig.getSpec().getStrategy(),
String.format("DeploymentConfig resource has null \".spec.strategy\"",
deploymentConfig.getSpec().getStrategy()));
Assertions.assertEquals("Recreate",
deploymentConfig.getSpec().getStrategy().getType(),
String.format("DeploymentConfig resource has unexpected \".spec.strategy\": %s.",
deploymentConfig.getSpec().getStrategy().getType()));
Assertions.assertNotNull(deploymentConfig.getSpec().getTemplate(),
String.format("DeploymentConfig resource has null \".spec.template\"",
deploymentConfig.getSpec().getTemplate()));
Assertions.assertNotNull(deploymentConfig.getSpec().getTemplate().getSpec(),
String.format("DeploymentConfig resource has null \".spec.template.spec\"",
deploymentConfig.getSpec().getTemplate().getSpec()));
PodSpec podSpec = deploymentConfig.getSpec().getTemplate().getSpec();
Assertions.assertEquals(0,
podSpec.getContainers().size(),
String.format("DeploymentConfig resource has unexpected \".spec.template.spec.containers\": %s.",
podSpec.getContainers().size()));
}
}
17 changes: 12 additions & 5 deletions core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@
<groupId>io.fabric8</groupId>
<artifactId>openshift-client</artifactId>
</dependency>
<dependency>
<groupId>io.fabric8</groupId>
<artifactId>openshift-server-mock</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.projectlombok</groupId>
Expand Down Expand Up @@ -77,6 +72,18 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-junit-jupiter</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
Expand Down
Loading