RFR: 8343074: test/jdk/com/sun/net/httpserver/docs/test1/largefile.txt could be generated [v5]

Daniel Fuchs dfuchs at openjdk.org
Tue Feb 4 14:20:10 UTC 2025


On Tue, 4 Feb 2025 12:04:47 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:

>> Adds `test.lib.Utils::createTempFileOfSize` to generate `test/jdk/com/sun/net/httpserver/docs` contents at runtime. This directory contains `largefile.txt` of size 2.6MiB showing up as the 4th largest file tracked by git:
>> 
>> 
>> $ git ls-files | while read f; do echo -e $(stat -c %s "$f")"\t$f"; done >/tmp/trackedFileSizes.txt
>> $ sort -n /tmp/trackedFileSizes.txt | tail -n 4
>> 2730088	test/jdk/com/sun/net/httpserver/docs/test1/largefile.txt
>> 2798680	src/java.base/share/data/unicodedata/NormalizationTest.txt
>> 3574947	test/jdk/java/foreign/libTestDowncallStack.c
>> 7128495	test/jdk/java/foreign/libTestUpcallStack.c
>> 
>> 
>> **Other highlights:**
>> 
>> - `jdk.httpclient.test.lib.common.TestUtil` is removed in favor of similar alternatives in `test.lib.Utils` and `test.lib.Asserts`
>> - `test.lib.Asserts::assertFileContentsEqual` is added
>
> Volkan Yazici has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Replace `assert`s with conditionally thrown exceptions

Generally LGTM - still one place calling `assert Files.mismatch(..)...`

test/jdk/com/sun/net/httpserver/SelCacheTest.java line 80:

> 78:             s2 = HttpsServer.create(addr, 0);
> 79:             // Assert that both files share the same parent and can be served from the same `FileServerHandler`
> 80:             assertEquals(smallFilePath.getParent(), largeFilePath.getParent());

That's OK. You could have kept `assert` here as it should not happen and would point to an issue with the logic in the test. But assertEquals will show us both paths if that happens, which is a +.

test/jdk/java/net/httpclient/http2/FixedThreadPoolTest.java line 180:

> 178:                 });
> 179:         response.join();
> 180:         assert Files.mismatch(src, dest) < 0;

Missed call to `assertFileContentsEqual`?

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

PR Review: https://git.openjdk.org/jdk/pull/23401#pullrequestreview-2592973383
PR Review Comment: https://git.openjdk.org/jdk/pull/23401#discussion_r1941241819
PR Review Comment: https://git.openjdk.org/jdk/pull/23401#discussion_r1941256241


More information about the core-libs-dev mailing list