Copilot commented on code in PR #1808:
URL: https://github.com/apache/commons-lang/pull/1808#discussion_r4147274019
##########
src/test/java/org/apache/commons/lang3/RandomStringUtilsTest.java:
##########
@@ -746,6 +746,42 @@ void testRandomAscii(final RandomStringUtils rsu) {
}
}
+ /**
+ * The POSIX {@code [:graph:]} class runs up to and including {@code '~'}
(0x7E), so {@link RandomStringUtils#nextGraph(int)} must be able to
+ * generate it. This test will fail randomly with negligible probability
{@code (1 - (1 - (93/94)^10))^1000}.
Review Comment:
The probability expression in the Javadoc is hard to read and also
hard-codes `1000` rather than referencing `LOOP_COUNT`, so it can become stale
if `LOOP_COUNT` changes. Consider simplifying it to a clearer form (e.g.,
probability of never seeing `~` across all generated characters) and/or
describing it qualitatively without embedding a brittle numeric formula.
##########
src/test/java/org/apache/commons/lang3/RandomStringUtilsTest.java:
##########
@@ -746,6 +746,42 @@ void testRandomAscii(final RandomStringUtils rsu) {
}
}
+ /**
+ * The POSIX {@code [:graph:]} class runs up to and including {@code '~'}
(0x7E), so {@link RandomStringUtils#nextGraph(int)} must be able to
+ * generate it. This test will fail randomly with negligible probability
{@code (1 - (1 - (93/94)^10))^1000}.
+ *
+ * @param rsu The instance to test
+ */
+ @ParameterizedTest
+ @MethodSource("randomProvider")
+ void testRandomGraphIncludesTilde(final RandomStringUtils rsu) {
+ boolean found = false;
+ for (int i = 0; i < LOOP_COUNT && !found; i++) {
+ if (rsu.nextGraph(10).indexOf('~') >= 0) {
+ found = true;
+ }
+ }
+ assertTrue(found, "'~' (0x7E) not generated by nextGraph in " +
LOOP_COUNT + " attempts -- repeated failures indicate a problem");
+ }
+
+ /**
+ * The POSIX {@code [:print:]} class runs up to and including {@code '~'}
(0x7E), so {@link RandomStringUtils#nextPrint(int)} must be able to
+ * generate it. This test will fail randomly with negligible probability
{@code (1 - (1 - (94/95)^10))^1000}.
+ *
+ * @param rsu The instance to test
+ */
+ @ParameterizedTest
+ @MethodSource("randomProvider")
+ void testRandomPrintIncludesTilde(final RandomStringUtils rsu) {
+ boolean found = false;
+ for (int i = 0; i < LOOP_COUNT && !found; i++) {
+ if (rsu.nextPrint(10).indexOf('~') >= 0) {
+ found = true;
+ }
+ }
+ assertTrue(found, "'~' (0x7E) not generated by nextPrint in " +
LOOP_COUNT + " attempts -- repeated failures indicate a problem");
Review Comment:
These tests are inherently probabilistic and can still fail
nondeterministically (flake) on CI even if the production code is correct.
Prefer making the tests deterministic by using an injectable/controllable RNG
(e.g., a custom `Random`/`RandomGenerator` that always returns the max value so
`~` must be produced) or by structuring the test around a deterministic
boundary condition (e.g., verifying behavior with a stubbed RNG) rather than
relying on repeated sampling.
--
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]
