RFR: 8332086: Remove the usage of ServiceLoader in j.u.r.RandomGeneratorFactory [v5]

[Raffaello Giulietti]

On Thu, 16 May 2024 10:51:14 GMT, Jaikiran Pai <jpai at 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