mxm commented on code in PR #721:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/721#discussion_r1417134168


##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/autoscaler/KubernetesScalingRealizerTest.java:
##########
@@ -30,12 +31,23 @@
 public class KubernetesScalingRealizerTest {
 
     @Test
-    public void testAutoscalerOverridesVertexIdsAreSorted() {
-
+    public void 
testAutoscalerOverridesStringDoesNotChangeUnlessOverridesChange() {
         KubernetesJobAutoScalerContext ctx =
                 TestingKubernetesAutoscalerUtils.createContext("test", null);
+        FlinkDeployment resource = (FlinkDeployment) ctx.getResource();
+
+        // Create resource with existing parallelism overrides
+        resource.getSpec()
+                .getFlinkConfiguration()
+                .put(PipelineOptions.PARALLELISM_OVERRIDES.key(), "a:1,b:2");

Review Comment:
   Thanks for elaborating further on this. Testing is somewhat of an art. There 
is also an argument for striking a balance between a specific test and one 
which tests all possible code paths (maybe even ones which don't yet exist).
   
   Black box testing is one way of testing. White box testing is also a valid 
approach. It depends on the situation and it is also a matter of taste and how 
confident one feels that the test covers the most important scenarios.
   
   I completely understand your thought, and I've changed the test method to 
address this concern.



-- 
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: issues-unsubscr...@flink.apache.org

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

Reply via email to