On Thu, 27 Apr 2023 19:32:02 GMT, John Hendrikx <jhendr...@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
