On Thu, 16 May 2024 10:51:14 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Raffaello Giulietti has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Small documentation changes.
>
> test/jdk/java/util/Random/RandomTestCoverage.java line 179:
>
>> 177: try {
>> 178: rng = factory.create(12345L);
>> 179: } catch (UnsupportedOperationException ignore) {
>
> I think now that we know for sure which algorithm instances don't support
> which type of seed value and since throwing `UnsupportedOperationException`
> is now a specification of the `create(...)` methods, we should probably do
> something like this:
>
>
> diff --git a/test/jdk/java/util/Random/RandomTestCoverage.java
> b/test/jdk/java/util/Random/RandomTestCoverage.java
> index be12d188198..6e5c36e13c3 100644
> --- a/test/jdk/java/util/Random/RandomTestCoverage.java
> +++ b/test/jdk/java/util/Random/RandomTestCoverage.java
> @@ -171,8 +171,37 @@ static void coverFactory(RandomGeneratorFactory factory)
> {
> boolean isSplittable = factory.isSplittable();
>
> coverRandomGenerator(factory.create());
> - coverRandomGenerator(factory.create(12345L));
> - coverRandomGenerator(factory.create(new byte[] {1, 2, 3, 4, 5, 6, 7,
> 8}));
> +
> + String algo = factory.name();
> + // test create(long)
> + switch (algo) {
> + // SecureRandom doesn't have long constructors so we expect
> + // UnsupportedOperationException
> + case "SecureRandom" -> {
> + try {
> + factory.create(12345L);
> + throw new
> AssertionError("RandomGeneratorFactory.create(long) was expected" +
> + "to throw UnsupportedOperationException for " +
> algo + " but didn't");
> + } catch (UnsupportedOperationException uoe) {
> + // expected
> + }
> + }
> + default -> coverRandomGenerator(factory.create(12345L));
> + }
> + // test create(byte[])
> + switch (algo) {
> + // these don't have byte[] constructors so we expect
> UnsupportedOperationException
> + case "Random", "SplittableRandom" -> {
> + try {
> + factory.create(new byte[] {1, 2, 3, 4, 5, 6, 7, 8});
> + throw new
> AssertionError("RandomGeneratorFactory.create(byte[]) was expected" +
> + "to throw UnsupportedOperationException for " +
> algo + " but didn't");
> + } catch (UnsupportedOperationException uoe) {
> + // expected
> + }
> + }
> + default -> coverRandomGenerator(factory.create(new byte[] {1, 2,
> 3, 4, 5, 6, 7, 8}));
> + ...
So you want to be very specific. OK.
> test/jdk/java/util/Random/RandomTestCoverage.java line 195:
>
>> 193: }
>> 194:
>> 195: static void coverDefaults() {
>
> This test method appears to test the calls to `getDefault()` methods on
> `RandomGeneratorFactory` and `RandomGenerator` classes, but it isn't being
> called in the test. We should call this method from `main()` to have test
> coverage for those methods.
Right
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19212#discussion_r1603274191
PR Review Comment: https://git.openjdk.org/jdk/pull/19212#discussion_r1603277648
