RFR(S) : 8242314 : use reproducible random in vmTestbase shared code

[Kim Barrett]

> On Apr 25, 2020, at 9:01 PM, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
> 
> 
>> On Apr 25, 2020, at 4:57 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>> 
>>> On Apr 22, 2020, at 2:03 PM, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>> 
>>> http://cr.openjdk.java.net/~iignatyev/8242314/webrev.00/ 
>>>> 122 lines changed: 71 ins; 19 del; 32 mod;
>>> 
>>> 
>>> Hi all,
>>> 
>>> could you please review this small patch which updates shared vmTestbase  test code to use j.t.l.Utils.getRandomInstance() as a random number generator or as a source of seed values? this patch lays the ground work for updates of :vmTestbase_* test groups which are to be done by remaining subtasks of 8241623[1], such split make it possible to work on/review/test testgroup-specific changes in concurrently.  
>>> 
>>> unfortunately, such extra dependency on /test/lib classes broke the shaky balance and caused some tests to fail due to "split testlibrary" problem (see 8188828[2] for the definition), the solution to that is to remove all dependencies on /test/lib classes from ExecDriver class, so the patch also includes "inlining" of several jdk.test.lib.Utils and Platform methods/fields into ExecDriver.
>>> 
>>> webrev: http://cr.openjdk.java.net/~iignatyev/8242314/webrev.00/ 
>>> testing: all :vmTestbase_* test groups on {linux,windows,macosx}-x64 and solaris-sparcv9
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8242314
>>> 
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8241623
>>> [2] https://bugs.openjdk.java.net/browse/JDK-8188828
>>> 
>>> Thanks,
>>> -- Igor
>> 
>> The test changes look fine.
>> 
>> But I don't feel like I understand the "split library" problem well
>> enough to review the ExecDriver changes.  I will say that I don't like
>> the solution of copying code, and hope there is a better way.  Is
>> there a bug for the problem?  It seems like it could crop up again,
>> perhaps repeatedly, in which case the code copying becomes increasingly
>> bad.  And it might be a good idea to leave some breadcrumbs associated
>> with this code copying workaround, so an eventual better fix could also
>> cleanup the duplication.
>> 
> 
> Hi Kim,
> 
> thanks for look at this!
> 
> "split library" problem occur when a test "have these properties: 
>  - has more than one @library 
>  - depends on class Foo from library A which depends on class Bar from another library B 
>  - has an implicit/explicit build for class Baz from libraries which depends on class Bar"[1]
> in this case A=/vmTestbase, B=/test/lib, Foo=ExecDriver, Bar=jdk.test.lib.Utils, and Baz can be pretty much any of class from jdk.test.lib, e.g. jdk.test.lib.process.ProcessTools.

Yes, I read that already, from JDK-8188828.  But I don't understand
the problem; remember, I'm not really a Java programmer, I just fake
it sometimes :)  In particular, I don't have a good understanding of
Java packaging and modularization and such.  So I'd rather not be
counted as having reviewed that part of the change.

> there is a better way to fix that, which we discussed have discussed in the aftermath of our encounter of this problem (when 8188828 was used as a stopgap solution), and this way is to redesign/simplify and potentially modularize test libraries so it will be easier/saner to build them. unfortunately, we are not there yet, yet we are making progress; this activity is tracked by 8211289[2], 8211290[3], and 8211358[4].
> 
> as of breadcrumbs, I think I left mentions of the copied methods in all the methods, e.g. L#144; and the only copied pieces of code which don't have such breadcrumbs are TEST_CLASS_PATH and TEST_JDK static fields. as they are quite straightforward, I didn't feel that it's need to refer back to the original code. but if you like, I can add comments there as well.

Regarding the breadcrumb comments, I didn't recognize them as such.
Had they been something more than just a fully qualified method name,
i.e. something like "Copied from <fqname>", it would have been clear.
Is there a cleanup CR for later?

> Thanks,
> -- Igor 
> 
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8188828?focusedCommentId=14121228&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14121228
> [2] https://bugs.openjdk.java.net/browse/JDK-8211289
> [3] https://bugs.openjdk.java.net/browse/JDK-8211290
> [4] https://bugs.openjdk.java.net/browse/JDK-8211358