RFR: JDK-8306990: The guarantees given by Region's floor and ceiling functions should work for larger values [v2]

[Andy Goryachev]

On Thu, 27 Apr 2023 19:32:02 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

>> modules/javafx.graphics/src/test/java/test/javafx/scene/layout/RegionTest.java line 1281:
>> 
>>> 1279:         }
>>> 1280: 
>>> 1281:         random.setSeed(seed);
>> 
>> it might be better to set the seed only once at the beginning.  otherwise the next 2 tests use the same sequence of doubles due to the seed being the same.  
>> or is this intentional?
>
> What are we trying to achieve here I'm wondering? Because if it is to slowly exhaustively test more and more of the problem space during each build, then I really disagree with the methodology.
> 
> We're confident that this is the correct solution at this time.  The test is only there to prevent regressions to the old broken situation. 
> 
> If you argued that it's a poor test, I'd agree; it is a poor test, and has been since the beginning; I can trivially pass the test with a code change that it would not detect as faulty (returning a constant, using round, etc. would all pass this test).

Not sure I quite understand what you are asking, so -

1. the test is good, it's a reasonable approach for a situation with a large space.  An exhaustive tests using floats that I ran passes, no need (I think) to run it on every build.  A similar test using doubles is not practical.  So we are good here.

2. printing seed for tests involving Random.  I think this should be a general rule to print the seed because if/when the test actually fails, we can reproduce with that particular seed.

3. resetting seed at the beginning of each test block.  I am not sure that this is a good idea because we are cutting the amount of "randomness" to a third, **unless** this is intentional.  Is it intentional?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1118#discussion_r1179608103