On Sun, 16 Mar 2025 02:54:36 GMT, Ioi Lam <ik...@openjdk.org> wrote:

> This PR tries to cut down the use of `WhiteBox` in the HotSpot test cases. It 
> modifies `VMProps` to save the set of VM properties into a file called 
> "vm.properties" under Jtreg's work directory. The new API 
> `jdk.test.lib.VMPropsGetter` loads the properties from this file to pass onto 
> individual test cases.
> 
> See `getJtregWorkDir()` for the code that figures out the work directory. It 
> assumes that `VMProps` and all test cases are always executed under
> 
> - `workDir/scratch/` ; or
> - `workDir/scratch/[0-9]+/` 
> 
> This is probably not the right thing to do. I would be better for Jtreg to 
> pass the location of the work directory to the test cases, with a 
> command-line options like `-Djtreg.work.dir=<dir>`.
> 
> To show the benefit of this PR, I modified a few test cases to remove their 
> use of WhiteBox.

Changes requested by lmesnik (Reviewer).

test/lib/jdk/test/lib/VMPropsGetter.java line 53:

> 51:     }
> 52: 
> 53:     static Properties init() {

That is not going to work correctly  for any test that use specific option, 
saying 
@run main/othervm -XX:+UseCompactObjectHeader Test
will return true for 
`if ("true".equals(VMPropsGetter.get("vm.cds.sharing.enabled"))) {`
because CDS are enabled for VMProps but false for 
`if (wb.isSharingEnabled()) {`
because for this particular test the CDS is not supported.

Also, anything that is executed with ProcessTools is going to have wrong 
information.

I think it is not the best way. The fix proposed in the 
https://bugs.openjdk.org/browse/CODETOOLS-7903975
is going to have the same issue.

The VM specific properties should be read only from current VM directly.

I f you want to simplify WB usage, I propose better to rewrite WB APIto  
internal JDK test module available for all tests.  
The WB might be implemented like
`module jdk.jfr/jdk.jfr.internal.test` with` jdk.jfr.internal.test.WhiteBox`
I think internal module jdk.internal.test could be implemented to support WB 
access to JDK.  Additionally it could be protected by VM flags to make it is 
more clear that shouldn't be used in production.

The another alternative is to add tag `@wbEnabled` to jtreg and teach it to run 
jtreg run tests with WB API enabled. I can't be added to the TEST.ROOT to whole 
hotspot tests.

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

PR Review: https://git.openjdk.org/jdk/pull/24071#pullrequestreview-2691487031
PR Review Comment: https://git.openjdk.org/jdk/pull/24071#discussion_r1999302346

Reply via email to