dimas-b commented on code in PR #2507:
URL: https://github.com/apache/polaris/pull/2507#discussion_r2322812802
##########
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:
Replacing something "deprecated" with something "insecure" does not look
very nice to me :sweat_smile:
Also (semi-related), using unseeded random input in tests could make them
non-deterministic.
Do we really need random inputs here?
My impression is that we need to ensure that certain chars are permitted or
not and the length is restricted. Using deterministic test parameters is
probably a more natural approach here.
I understand that this PR merely attempts to avoid deprecation, but since
the related test code came to focus here, WDYT about making a more substantial
change to make the test deterministic and improve coverage of input cases?
--
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]
