This is an automated email from the ASF dual-hosted git repository.
dongjoon pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/spark-kubernetes-operator.git
The following commit(s) were added to refs/heads/main by this push:
new 7989344 [SPARK-53872] Revisit `JUnit` assert usage in test cases
7989344 is described below
commit 798934429505f3bae2acc015d1e634389d3cbb66
Author: Dongjoon Hyun <[email protected]>
AuthorDate: Fri Oct 10 14:39:54 2025 -0700
[SPARK-53872] Revisit `JUnit` assert usage in test cases
### What changes were proposed in this pull request?
This PR aims to revisit `JUnit` assertion usage in test cases after
upgrading to `JUnit 6`.
- #383
### Why are the changes needed?
There are three cases:
1. Fix wrong usage of `assertEquals`: The first parameter is the expected
value.
```java
- assertEquals(previousGeneration2, 1L);
+ assertEquals(1L, previousGeneration2);
```
2. `assertNull` is better than `assertEquals(null, *)`.
```java
- assertEquals(null, spec1.runtimeVersions.jdkVersion);
- assertEquals(null, spec1.runtimeVersions.scalaVersion);
- assertEquals(null, spec1.runtimeVersions.sparkVersion);
+ assertNull(spec1.runtimeVersions.jdkVersion);
+ assertNull(spec1.runtimeVersions.scalaVersion);
+ assertNull(spec1.runtimeVersions.sparkVersion);
```
3. `assertInstanceOf` is more intuitive than `assertTrue(... instanceof
...)`.
```java
- assertTrue(constructorArgs.get(conf).get(0) instanceof SparkConf);
+ assertInstanceOf(SparkConf.class, constructorArgs.get(conf).get(0));
```
### Does this PR introduce _any_ user-facing change?
No. This is a test case fix and improvement.
### How was this patch tested?
Pass the CIs.
### Was this patch authored or co-authored using generative AI tooling?
No.
Closes #384 from dongjoon-hyun/SPARK-53872.
Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
---
.../org/apache/spark/k8s/operator/spec/ClusterSpecTest.java | 7 ++++---
.../apache/spark/k8s/operator/metrics/MetricsSystemTest.java | 4 ++--
.../operator/metrics/healthcheck/SentinelManagerTest.java | 12 ++++++------
.../metrics/source/KubernetesMetricsInterceptorTest.java | 4 ++--
.../operator/metrics/source/OperatorJosdkMetricsTest.java | 2 +-
.../apache/spark/k8s/operator/probe/ProbeServiceTest.java | 4 ++--
.../spark/k8s/operator/SparkAppSubmissionWorkerTest.java | 6 +++---
7 files changed, 20 insertions(+), 19 deletions(-)
diff --git
a/spark-operator-api/src/test/java/org/apache/spark/k8s/operator/spec/ClusterSpecTest.java
b/spark-operator-api/src/test/java/org/apache/spark/k8s/operator/spec/ClusterSpecTest.java
index b96455a..5a125bd 100644
---
a/spark-operator-api/src/test/java/org/apache/spark/k8s/operator/spec/ClusterSpecTest.java
+++
b/spark-operator-api/src/test/java/org/apache/spark/k8s/operator/spec/ClusterSpecTest.java
@@ -20,6 +20,7 @@
package org.apache.spark.k8s.operator.spec;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
import org.junit.jupiter.api.Test;
@@ -34,9 +35,9 @@ class ClusterSpecTest {
@Test
void testInitSpecWithDefaults() {
ClusterSpec spec1 = new ClusterSpec();
- assertEquals(null, spec1.runtimeVersions.jdkVersion);
- assertEquals(null, spec1.runtimeVersions.scalaVersion);
- assertEquals(null, spec1.runtimeVersions.sparkVersion);
+ assertNull(spec1.runtimeVersions.jdkVersion);
+ assertNull(spec1.runtimeVersions.scalaVersion);
+ assertNull(spec1.runtimeVersions.sparkVersion);
assertEquals(0, spec1.clusterTolerations.instanceConfig.initWorkers);
assertEquals(0, spec1.clusterTolerations.instanceConfig.minWorkers);
assertEquals(0, spec1.clusterTolerations.instanceConfig.maxWorkers);
diff --git
a/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/MetricsSystemTest.java
b/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/MetricsSystemTest.java
index 838ea19..cf10661 100644
---
a/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/MetricsSystemTest.java
+++
b/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/MetricsSystemTest.java
@@ -63,8 +63,8 @@ class MetricsSystemTest {
Sink mockSink = mockSinkOptional.get();
metricsSystem.stop();
MockSink sink = (MockSink) mockSink;
- assertEquals(sink.getPollPeriod(), 10);
- assertEquals(sink.getTimeUnit(), TimeUnit.SECONDS);
+ assertEquals(10, sink.getPollPeriod());
+ assertEquals(TimeUnit.SECONDS, sink.getTimeUnit());
}
@Test
diff --git
a/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/healthcheck/SentinelManagerTest.java
b/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/healthcheck/SentinelManagerTest.java
index 02b6d35..b9e7e06 100644
---
a/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/healthcheck/SentinelManagerTest.java
+++
b/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/healthcheck/SentinelManagerTest.java
@@ -109,7 +109,7 @@ class SentinelManagerTest {
kubernetesClient.resources(SparkApplication.class).inNamespace(DEFAULT).list();
SparkApplication sparkApplication = crList.getItems().get(0);
Long generation = sparkApplication.getMetadata().getGeneration();
- assertEquals(generation, 1L);
+ assertEquals(1L, generation);
// Spark Reconciler Handle Sentinel Resources at the first time
var sentinelManager = new SentinelManager<SparkApplication>();
@@ -120,12 +120,12 @@ class SentinelManagerTest {
Map<String, String> sparkConf2 = new
HashMap<>(sparkApplication2.getSpec().getSparkConf());
long generation2 = sparkApplication2.getMetadata().getGeneration();
- assertEquals(sparkConf2.get(Constants.SENTINEL_RESOURCE_DUMMY_FIELD), "1");
- assertEquals(generation2, 2L);
+ assertEquals("1", sparkConf2.get(Constants.SENTINEL_RESOURCE_DUMMY_FIELD));
+ assertEquals(2L, generation2);
var state2 =
sentinelManager.getSentinelResources().get(ResourceID.fromResource(mockApp));
long previousGeneration2 = state2.previousGeneration;
assertTrue(sentinelManager.allSentinelsAreHealthy());
- assertEquals(previousGeneration2, 1L);
+ assertEquals(1L, previousGeneration2);
Thread.sleep(Duration.ofSeconds(SENTINEL_RESOURCE_RECONCILIATION_DELAY_SECONDS
* 2).toMillis());
List<SparkApplication> crList3 =
@@ -194,8 +194,8 @@ class SentinelManagerTest {
sentinelManager.handleSentinelResourceReconciliation(sparkApplication1,
kubernetesClient);
sentinelManager.handleSentinelResourceReconciliation(sparkApplication2,
kubernetesClient);
assertEquals(
- sentinelManager.getSentinelResources().size(),
2,
+ sentinelManager.getSentinelResources().size(),
"Sentinel Manager should watch on resources in two namespaces");
assertTrue(
sentinelManager.allSentinelsAreHealthy(), "Sentinel Manager should
report healthy");
@@ -206,8 +206,8 @@ class SentinelManagerTest {
"Sentinel Manager should report healthy after one namespace is "
+ "removed from the watch");
assertEquals(
- sentinelManager.getSentinelResources().size(),
1,
+ sentinelManager.getSentinelResources().size(),
"Sentinel Manager should only watch on one namespace");
}
}
diff --git
a/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/source/KubernetesMetricsInterceptorTest.java
b/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/source/KubernetesMetricsInterceptorTest.java
index 6f71b74..49b1d6e 100644
---
a/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/source/KubernetesMetricsInterceptorTest.java
+++
b/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/source/KubernetesMetricsInterceptorTest.java
@@ -94,9 +94,9 @@ class KubernetesMetricsInterceptorTest {
.forEach(
name -> {
Meter metric = (Meter) metrics2.get(name);
- Assertions.assertEquals(metric.getCount(), 1);
+ Assertions.assertEquals(1, metric.getCount());
});
- Assertions.assertEquals(((Meter)
metrics2.get("http.request")).getCount(), 2);
+ Assertions.assertEquals(2, ((Meter)
metrics2.get("http.request")).getCount());
client.resource(sparkApplication).delete();
}
}
diff --git
a/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/source/OperatorJosdkMetricsTest.java
b/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/source/OperatorJosdkMetricsTest.java
index b7f1b8a..b5e7441 100644
---
a/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/source/OperatorJosdkMetricsTest.java
+++
b/spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/source/OperatorJosdkMetricsTest.java
@@ -76,7 +76,7 @@ class OperatorJosdkMetricsTest {
try {
operatorMetrics.timeControllerExecution(failedExecution);
} catch (Exception e) {
- assertEquals(e.getMessage(), "Foo exception");
+ assertEquals("Foo exception", e.getMessage());
assertEquals(8, metrics.size());
assertTrue(metrics.containsKey("sparkapplication.test-controller.reconcile.failure"));
assertTrue(
diff --git
a/spark-operator/src/test/java/org/apache/spark/k8s/operator/probe/ProbeServiceTest.java
b/spark-operator/src/test/java/org/apache/spark/k8s/operator/probe/ProbeServiceTest.java
index 8afd5ea..f960d7a 100644
---
a/spark-operator/src/test/java/org/apache/spark/k8s/operator/probe/ProbeServiceTest.java
+++
b/spark-operator/src/test/java/org/apache/spark/k8s/operator/probe/ProbeServiceTest.java
@@ -117,7 +117,7 @@ class ProbeServiceTest {
HttpURLConnection connection = (HttpURLConnection) u.openConnection();
connection.setConnectTimeout(100000);
connection.connect();
- assertEquals(connection.getResponseCode(), HTTP_OK, "Health Probe should
return HTTP_OK");
+ assertEquals(HTTP_OK, connection.getResponseCode(), "Health Probe should
return HTTP_OK");
}
private void hitStartedUpEndpoint() throws IOException,
MalformedURLException {
@@ -125,6 +125,6 @@ class ProbeServiceTest {
HttpURLConnection connection = (HttpURLConnection) u.openConnection();
connection.setConnectTimeout(100000);
connection.connect();
- assertEquals(connection.getResponseCode(), HTTP_OK, "operators are not
ready");
+ assertEquals(HTTP_OK, connection.getResponseCode(), "operators are not
ready");
}
}
diff --git
a/spark-submission-worker/src/test/java/org/apache/spark/k8s/operator/SparkAppSubmissionWorkerTest.java
b/spark-submission-worker/src/test/java/org/apache/spark/k8s/operator/SparkAppSubmissionWorkerTest.java
index 4376b28..a1abde1 100644
---
a/spark-submission-worker/src/test/java/org/apache/spark/k8s/operator/SparkAppSubmissionWorkerTest.java
+++
b/spark-submission-worker/src/test/java/org/apache/spark/k8s/operator/SparkAppSubmissionWorkerTest.java
@@ -79,7 +79,7 @@ class SparkAppSubmissionWorkerTest {
assertEquals(6, constructorArgs.get(conf).size());
// validate SparkConf with override
- assertTrue(constructorArgs.get(conf).get(0) instanceof SparkConf);
+ assertInstanceOf(SparkConf.class, constructorArgs.get(conf).get(0));
SparkConf createdConf = (SparkConf) constructorArgs.get(conf).get(0);
assertEquals("bar", createdConf.get("foo"));
assertEquals("5", createdConf.get("spark.executor.instances"));
@@ -90,13 +90,13 @@ class SparkAppSubmissionWorkerTest {
"namespace from CR takes highest precedence");
// validate main resources
- assertTrue(constructorArgs.get(conf).get(2) instanceof
JavaMainAppResource);
+ assertInstanceOf(JavaMainAppResource.class,
constructorArgs.get(conf).get(2));
JavaMainAppResource mainResource = (JavaMainAppResource)
constructorArgs.get(conf).get(2);
assertTrue(mainResource.primaryResource().isEmpty());
assertEquals("foo-class", constructorArgs.get(conf).get(3));
- assertTrue(constructorArgs.get(conf).get(4) instanceof String[]);
+ assertInstanceOf(String[].class, constructorArgs.get(conf).get(4));
String[] capturedArgs = (String[]) constructorArgs.get(conf).get(4);
assertEquals(2, capturedArgs.length);
assertEquals("a", capturedArgs[0]);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
