On Tue, 4 Feb 2025 12:04:47 GMT, Volkan Yazici <vyaz...@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
