[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`.
- apache#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 apache#384 from dongjoon-hyun/SPARK-53872.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>

Author: dongjoon-hyun

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 @@ void testBuilder() {
   @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);

spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/MetricsSystemTest.java

@@ -63,8 +63,8 @@ void testMetricsSystemWithCustomizedSink() {
     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

spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/healthcheck/SentinelManagerTest.java

@@ -109,7 +109,7 @@ void testHandleSentinelResourceReconciliation() throws InterruptedException {
         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 @@ void testHandleSentinelResourceReconciliation() throws InterruptedException {
     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 @@ void sentinelManagerShouldReportHealthyWhenWatchedNamespaceIsReduced()
       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 @@ void sentinelManagerShouldReportHealthyWhenWatchedNamespaceIsReduced()
           "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");
     }
   }

spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/source/KubernetesMetricsInterceptorTest.java

@@ -94,9 +94,9 @@ void testMetricsEnabled() {
           .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();
     }
   }

spark-operator/src/test/java/org/apache/spark/k8s/operator/metrics/source/OperatorJosdkMetricsTest.java

@@ -76,7 +76,7 @@ void testTimeControllerExecution() throws Exception {
     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(

spark-operator/src/test/java/org/apache/spark/k8s/operator/probe/ProbeServiceTest.java

@@ -117,14 +117,14 @@ private void hitHealthyEndpoint() throws IOException, MalformedURLException {
     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 {
     URL u = new URL("http://localhost:" + OPERATOR_PROBE_PORT.getValue() + READYZ);
     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");
   }
 }

spark-submission-worker/src/test/java/org/apache/spark/k8s/operator/SparkAppSubmissionWorkerTest.java

@@ -79,7 +79,7 @@ void buildDriverConfShouldApplySpecAndPropertiesOverride() {
       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 @@ void buildDriverConfShouldApplySpecAndPropertiesOverride() {
           "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]);