Re: [PR] [FLINK-37478] Adding service labels to flink config [flink]

["ferenc-csaky (via GitHub)" <gi...@apache.org>]

ferenc-csaky commented on code in PR #26301:
URL: https://github.com/apache/flink/pull/26301#discussion_r2184946516


##########
flink-kubernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/parameters/KubernetesJobManagerParametersTest.java:
##########
@@ -109,6 +109,20 @@ void testGetRestServiceAnnotations() {
         assertThat(resultAnnotations).isEqualTo(expectedAnnotations);
     }
 
+    @Test
+    void testGetRestServiceLabels() {
+        final Map<String, String> expectedLabels = new HashMap<>();
+        expectedLabels.put("a1", "v1");
+        expectedLabels.put("a2", "v2");
+
+        flinkConfig.set(KubernetesConfigOptions.REST_SERVICE_LABELS, expectedLabels);
+
+        final Map<String, String> resultLabels =
+                kubernetesJobManagerParameters.getRestServiceLabels();
+
+        assertThat(resultLabels).isEqualTo(expectedLabels);

Review Comment:
   Both asserts can be improved in this test like:
   ```java
   assertThat(resultLabels)
           .containsExactlyInAnyOrderEntriesOf(Map.of("a1", "v1", "a2", "v2"));
   ```



##########
flink-kubernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/decorators/ExternalServiceDecoratorTest.java:
##########
@@ -50,12 +50,21 @@ class ExternalServiceDecoratorTest extends KubernetesJobManagerTestBase {
                 }
             };
 
+    private Map<String, String> customizedLabels =
+            new HashMap<String, String>() {

Review Comment:
   I know it's a copy of the existing class level field, but let's use `Map.of(...)` instead for both.



-- 
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-unsubscribe@flink.apache.org

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