On Wed, 31 May 2023 14:46:59 GMT, Aleksey Shipilev <sh...@openjdk.org> wrote:
>> Two thoughts here. >> The random number source (SecureRandom) should have its own tests, UUID has >> a simple dependency on a generator. >> The test for UUID is that it composes the bits into the UUID correctly. The >> randomness of the generator should be factored out. >> >> Second, to raise concern about collisions, then the test should not throw on >> the first detected collision but complete the cycle and provide the simple >> stats on the number of collisions per COUNT (1,000,000). >> >> All those 5 second test runs add up. > >> Two thoughts here. The random number source (SecureRandom) should have its >> own tests, UUID has a simple dependency on a generator. The test for UUID is >> that it composes the bits into the UUID correctly. The randomness of the >> generator should be factored out. > > This test verifies the way UUID uses the SecureRandom. Think about this as > the less of a unit, and more of the integration test. This would be even more > important once things like https://bugs.openjdk.org/browse/JDK-8308804 show > up. > >> Second, to raise concern about collisions, then the test should not throw on >> the first detected collision but complete the cycle and provide the simple >> stats on the number of collisions per COUNT (1,000,000). > > All right, that we can do. See new commit. > >> All those 5 second test runs add up. > > Yes, and it would be sad to waste those 5 seconds on something irrelevant. I > would argue that `UUID.randomUUID` breakage would be very unfortunate for the > real world systems. Spending 5 seconds per test run on it is a good > investment here. We're into the area of diminishing returns. I'd expect the implementation bug mentioned above would be found by far fewer iterations. Thanks for the test change. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14134#discussion_r1212139442
