wangyang0918 commented on a change in pull request #131:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/131#discussion_r840351874



##########
File path: 
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/controller/FlinkDeploymentControllerTest.java
##########
@@ -397,7 +402,59 @@ public void testUpgradeNotReadyCluster() {
         testUpgradeNotReadyCluster(appCluster, false);
     }
 
-    public void testUpgradeNotReadyCluster(FlinkDeployment appCluster, boolean 
allowUpgrade) {
+    @Test
+    public void verifyReconcileWithBadConfig() {
+
+        FlinkDeployment appCluster = TestUtils.buildApplicationCluster();
+        UpdateControl<FlinkDeployment> updateControl;
+        // Override rest port, and it should be saved in lastReconciledSpec 
once a successful
+        // reconcile() finishes.
+        
appCluster.getSpec().getFlinkConfiguration().put(RestOptions.PORT.key(), 
"8088");
+        updateControl = testController.reconcile(appCluster, context);
+        assertTrue(updateControl.isUpdateStatus());
+        assertEquals(
+                JobManagerDeploymentStatus.DEPLOYING,
+                appCluster.getStatus().getJobManagerDeploymentStatus());
+
+        // Check when the bad config is applied, observe() will change the 
cluster state correctly
+        appCluster.getSpec().getJobManager().setReplicas(-1);
+        // Next reconcile will set error msg and observe with previous 
validated config
+        updateControl = testController.reconcile(appCluster, context);
+        assertEquals(
+                "JobManager replicas should not be configured less than one.",
+                appCluster.getStatus().getReconciliationStatus().getError());
+        assertTrue(updateControl.isUpdateStatus());
+        assertEquals(
+                JobManagerDeploymentStatus.DEPLOYED_NOT_READY,
+                appCluster.getStatus().getJobManagerDeploymentStatus());
+
+        // Check the exception
+        appCluster.getSpec().getJobManager().setReplicas(1);
+        appCluster.getSpec().getJob().setJarURI(null);
+        assertDoesNotThrow(

Review comment:
       I think the test will also fail without `assertDoesNotThrow`. Right?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to