RFR: 8263549: 8263412 can cause jtreg testlibrary split [v3]

Daniel D.Daugherty dcubed at openjdk.java.net
Sat Mar 13 14:23:08 UTC 2021


On Sat, 13 Mar 2021 06:44:12 GMT, Igor Ignatyev <iignatyev at openjdk.org> wrote:

>> Hi all,
>> 
>> could you please review this dull patch that replaces `ClassFileInstaller` w/ `jdk.test.lib.helpers.ClassFileInstaller` in all jtreg test descriptions to ensure we won't get split testlibrary, and removes `jdk/test/lib/ClassFileInstaller.java` (so it won't be accidentally used).
>> 
>> from JBS:
>>> after JDK-8263412, we might (again) encounter NCDFE b/c parts of testlibraries aren't on the classpath. this happens when jtreg builds `jdk.test.lib.helpers.ClassFileInstaller` as a part of test-specific code, but `ClassFileInstaller` as part of shared testibrary directory in one test, when in the following test, jtreg sees `ClassFileInstaller` in the shared directory, hence javac won't recompile it/its dependencies, but in runtime `jdk.test.lib.helpers.ClassFileInstaller` is nowhere to be found, hence we get NCDFE. 
>> 
>> testing:
>> - [x]  `grep ' ClassFileInstaller[^.]`
>> - [ ] tier1-3
>> 
>> Thanks,
>> -- Igor
>
> Igor Ignatyev has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR.

I downloaded the patch and used Ioi's cmd pattern to scroll through
the changes. I can't honestly say that I looked at every line since 867
changed files would overwhelm anyone's brain...

I did notice a couple of `@run main` instead of `@run driver` calls
to the ClassFileInstaller, but those are pre-existing.

Thumbs up.

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

Marked as reviewed by dcubed (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2985


More information about the security-dev mailing list