On Tue, 10 Mar 2026 16:56:46 GMT, Paul Hübner <[email protected]> wrote:
>> Frederic Parain has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix generation of short values
>
> test/hotspot/jtreg/runtime/valhalla/inlinetypes/field_layout/ValueComparisonTest.java
> line 90:
>
>> 88: Path tempWorkDir;
>> 89: try {
>> 90: tempWorkDir = Files.createTempDirectory(currentDir,
>> "generatedClasses_" + seed);
>
> Sorry, I gave you the wrong name in the earlier comment. I meant `Utils` not
> `Path`.
> Use this instead:
> https://github.com/openjdk/valhalla/blob/432870d43a3ef3ffb181e5909454af58754febc9/test/lib/jdk/test/lib/Utils.java#L808
This code and the one in Utils.java are equivalent. The tests have been updated
to use jdk/lib/Utils code for the sake of integration with JTREG. FYI, most
tests in the runtime directory still use Files.createTemp[Directory|File]
instead of the jdk/lib/Utils version.
> test/hotspot/jtreg/runtime/valhalla/inlinetypes/field_layout/ValueRandomLayoutTest.java
> line 64:
>
>> 62: List<String> argsList = new ArrayList<>();
>> 63: String classpath = System.getProperty("java.class.path") +
>> System.getProperty("path.separator") +
>> 64: Paths.get("").toAbsolutePath() +
>
> Shouldn't this already be included by jtreg somehow? Not sure, but I've not
> seen other similar tests use it.
I was a remain of when the classes were generated in the current directory.
Useless now and removed.
> test/hotspot/jtreg/runtime/valhalla/inlinetypes/field_layout/ValueRandomLayoutTest.java
> line 140:
>
>> 138: Path currentDir = Paths.get("").toAbsolutePath();
>> 139: try {
>> 140: tempWorkDir = Files.createTempDirectory(currentDir,
>> "generatedClasses_" + seed);
>
> See other comment on temp dir creation.
Fixed
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2202#discussion_r2918260779
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2202#discussion_r2918264007
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2202#discussion_r2918264820