spuru9 commented on code in PR #27917:
URL: https://github.com/apache/flink/pull/27917#discussion_r3070094676


##########
flink-runtime/src/test/java/org/apache/flink/runtime/testutils/PseudoRandomValueSelectorTest.java:
##########
@@ -109,6 +111,33 @@ void testStableRandomization() {
         assertThat(uniqueValues).hasSize(1);
     }
 
+    /**
+     * Tests that randomize() produces different values for different config 
options within the same
+     * test. Previously, all boolean options got the same value because the 
seed did not include the
+     * config option key.
+     */
+    @Test
+    void testRandomizeDifferentOptionsProduceDifferentValues() {
+        final Integer[] alternatives = IntStream.range(0, 
20).boxed().toArray(Integer[]::new);
+
+        ConfigOption<Integer>[] options = new ConfigOption[100];
+        for (int i = 0; i < options.length; i++) {
+            options[i] = ConfigOptions.key("test.randomize.option." + 
i).intType().noDefaultValue();
+        }
+
+        Configuration conf = new Configuration();
+        for (ConfigOption<Integer> option : options) {
+            randomize(conf, option, alternatives);
+        }
+
+        Set<Integer> uniqueValues = new HashSet<>();
+        for (ConfigOption<Integer> option : options) {
+            uniqueValues.add(conf.get(option));
+        }

Review Comment:
   The three loops in testRandomizeDifferentOptionsProduceDifferentValues could 
be merged into one, which would simplify the test a bit:  
   
   ```suggestion
           final Configuration conf = new Configuration();
           final Set<Integer> uniqueValues = new HashSet<>();                   
                                                                                
                                                      
                                                                                
                                                                                
                                                      
           for (int i = 0; i < 100; i++) {
               ConfigOption<Integer> option =                                   
                                                                                
                                                      
                       ConfigOptions.key("test.randomize.option." + 
i).intType().noDefaultValue();
               randomize(conf, option, alternatives);
               uniqueValues.add(conf.get(option));                              
                                                                                
                                                      
           }
   ```
   Feel free to ignore if you prefer keeping the loops separate for readability



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to