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