XN137 commented on code in PR #2507:
URL: https://github.com/apache/polaris/pull/2507#discussion_r2330641411


##########
integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java:
##########
@@ -136,6 +136,10 @@ public void tearDown() {
     client.cleanUp(authToken);
   }
 
+  private static String newRandomString(int length) {
+    return RandomStringUtils.insecure().next(length, true, true);

Review Comment:
   afaict it is named `insecure()` to make clear that the source of randomness 
is not fit for cryptographic purposes.
   we can also switch to `secure()` instead if you prefer?
   
   > WDYT about making a more substantial change to make the test deterministic 
and improve coverage of input cases
   
   we can do that in a followup PR imo, but if we want to improve the coverage 
compared to randomized input you need to give concrete examples on what inputs 
you think would "improve" that coverage as for me its not clear.
   we can of course make the test inputs deterministic (by picking one 
randomized sample?) but i dont think this would improve coverage.



-- 
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