On Tue, 30 Sep 2025 08:08:09 GMT, Alan Bateman <[email protected]> wrote:
>> test/hotspot/jtreg/runtime/jni/mutateFinals/MutateFinalsTest.java line 109: >> >>> 107: String instanceMethod = list1.get(rand.nextInt(list1.size())); >>> 108: String staticMethod = list2.get(rand.nextInt(list2.size())); >>> 109: return Stream.of(instanceMethod, staticMethod); >> >> * What is the rationale for choosing a random `pairOf(instanceMutator, >> classMutator)`? >> * Does "instance mutator gets followed by a class mutator" have any >> particular importance for the tests they are used? >> >> I was looking at this argument supplier and thinking of >> `Collections.shuffle()` over a list containing all method names, preferably, >> multiple times. > > The comment in the test description has ".. to avoid starting a child VM to > test every mutation method". I'll see if I can improve this comment to make > it clearer, or maybe your comment is that the method source needs this > comment too? > > The context here is that the test is launching a VM with -Xcheck:jni to check > that it terminates with a "FATAL ERROR in native method". Doing this for > each of 18 cases would be expensive, can be 30+ seconds on macOS systems with > debug builds. To keep the test execution down, the test choses one JNI > function that attempts to mutate a final instance field, and one JNI function > that attempts to mutate a static final field. The ensures the instance + > static implementations are tested on each run. There is template expansion, > and all functions will get exercised with enough runs. I was curious why in particular `instance+static`, but not, say, `static+instance`, or `static1+instance1+static2+static3+instance3`, etc. That is, does checking the static field *after* the instance field have any significance. But AFAIU from your explanation, it does not. I don't have any further remarks. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25115#discussion_r2390371924
