[lworld] RFR: 8377162: [lworld] getResourceAsStream() doesn't work in preview mode for exploded images [v2]
David Beaumont
duke at openjdk.org
Thu Feb 19 11:22:48 UTC 2026
On Wed, 18 Feb 2026 18:25:37 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>> David Beaumont has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>>
>> - Merge branch 'lworld' into jdk_8377162_exploded_modules/squashed
>> - revert unnecessary filter in ModulePatcher
>> - Latest version
>> - Limit awareness of preview resources to system modules only.
>>
>> * limit preview handling
>> * Patch ModuleReaderTest from mainline
>> - Preview mode for exploded system module readers
>>
>> * with better flag plumbing
>> * Simpler approach with linked hash set
>> * Revert weird import change
>> * First cut at a preview aware module reader for exploded builds
>> * Test for partiy of JRT file-system and class resources in preview mode
>
> test/jdk/java/lang/module/ModuleReader/ModuleReaderTest.java line 289:
>
>> 287: @Test
>> 288: public void testModularJar() throws IOException {
>> 289: Path dir = Utils.createTempDirectory("mlib");
>
> Just curious why you changed this as this test should not need to the Utils.
I strongly prefer not having to reason about auxiliary details when reading tests, especially if I'm not familiar with the tests/code involved. Test code is the canonical representation of the expectations for the code under test, and isn't itself tested. Thus, the fewer questions a test raises when reading it, the easier it is to focus on what the test is testing, rather than how the test is setting up its environment etc.
The original version of this test used a mix of "temporary directories created in `user.dir`" and "relative paths from the current working directory". When reading this I had to stop and ask "is there anything special about `user.dir` rather than just using the current working directory?"
Whether a test reads the `user.dir` property, of uses '.' for the directory inside which temporary files are created is a detail of how tests are done, and the knowledge that these are (at the moment) functionally identical might not be something everyone reading the tests knows. So the more it's obvious that "this is just a normal temporary file and where it actually resides isn't important", the less a reader of the code has to pause and spend mental energy thinking about it.
Yes it's a small thing on its own, but consistency across tests for lots of these small details adds up to less "mental friction".
Of course now I look again, I realise that this is a JUnit test, and could be using `@TempDir` to be even more idiomatic.
-------------
PR Review Comment: https://git.openjdk.org/valhalla/pull/2032#discussion_r2827214962
More information about the valhalla-dev
mailing list