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

[Daniel Fuchs]

On Mon, 3 Feb 2025 19:26:03 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 two additional commits since the last revision:
> 
>  - Assert that multiple files can be served using the same `FileServerHandler`
>  - Remove redundant `@build` dependencies

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

> 75:             s2 = HttpsServer.create(addr, 0);
> 76:             // Assert that both files share the same parent and can be served from the same `FileServerHandler`
> 77:             assert smallFilePath.getParent().equals(largeFilePath.getParent());

I see what you are doing here. On the one hand I thought that it would be better placed right after line 66, but on the other hand we want it inside the `try { }` so that files will be deleted on `finally` even if it fires. So LGTM.

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

> 143:         fout.close();
> 144: 
> 145:         if (count != filePath.toFile().length()) {

Maybe we could use assertEquals here for better diagnosability

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

> 147:         }
> 148:         Path tempPath = temp.toPath();
> 149:         assert Files.mismatch(filePath, tempPath) < 0;

I would suggest using `assertEquals` with -1 here to ensure that:

1. the test will fail if the files don't match even if asserts are disabled, and
2. we see at which place mismatch was detected in the exception message

test/jdk/com/sun/net/httpserver/Test1.java line 160:

> 158:         }
> 159:         Path tempPath = temp.toPath();
> 160:         assert Files.mismatch(filePath, tempPath) < 0;

Same suggestions WRT assertEquals here

test/jdk/com/sun/net/httpserver/Test12.java line 182:

> 180:                 }
> 181:                 Path tempPath = temp.toPath();
> 182:                 assert Files.mismatch(filePath, tempPath) < 0;

and here as well

test/jdk/com/sun/net/httpserver/Test13.java line 184:

> 182:                 }
> 183:                 Path tempPath = temp.toPath();
> 184:                 assert Files.mismatch(filePath, tempPath) < 0;

ditto

test/jdk/com/sun/net/httpserver/Test9.java line 202:

> 200:                 }
> 201:                 Path tempPath = temp.toPath();
> 202:                 assert Files.mismatch(filePath, tempPath) < 0;

And in all other places where this pattern appears in this PR

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23401#discussion_r1940924335
PR Review Comment: https://git.openjdk.org/jdk/pull/23401#discussion_r1940936508
PR Review Comment: https://git.openjdk.org/jdk/pull/23401#discussion_r1940935266
PR Review Comment: https://git.openjdk.org/jdk/pull/23401#discussion_r1940941328
PR Review Comment: https://git.openjdk.org/jdk/pull/23401#discussion_r1940942729
PR Review Comment: https://git.openjdk.org/jdk/pull/23401#discussion_r1940943725
PR Review Comment: https://git.openjdk.org/jdk/pull/23401#discussion_r1940945015