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]