RFR: 8308803: Improve java/util/UUID/UUIDTest.java [v2]

Roger Riggs rriggs at openjdk.org
Wed May 31 14:20:57 UTC 2023


On Wed, 31 May 2023 14:05:20 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> My point was that its probably not practical to test (more than once).  
>> If it fails, it will be considered just as you propose and disregarded and in the meantime consumes test cycles in each of the test contexts. Either provide more information about the conditions under which it failed or remove it.
>
> Sorry, I have trouble following the argument here.
> 
> Let me re-iterate: the probability for bona-fide collision is so vanishingly low, the test failure here is a strong signal that something is wrong with the implementation. We can put more guidance in the test comments there, like "This is extremely unlikely to happen. If you see this failing, this highly likely points to the implementation bug, rather than the odd chance."
> 
> What I expect to happen when that test fails, is that it prompts the investigation with multiple stress tests to get a better estimate of the actual collision rate. Assuming we actually see a collision, it is likely to be caused by much higher probability error somewhere in the code. In fact, if this test is _actually noisy_ to the point it becomes a testing problem, this already gives us the signal that actual collision rate is many orders of magnitude higher than math predicts, and this becomes an even _stronger_ signal that random UUIDs are seriously broken for practical use.

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.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14134#discussion_r1211795871


More information about the core-libs-dev mailing list