On Fri, 31 Jan 2025 15:56:29 GMT, Mikhail Yankelevich <d...@openjdk.org> wrote:
> Refactor test/java/security//cert/CertificateFactory/slowstream.sh to java
> test
test/jdk/java/security/cert/CertificateFactory/SlowStream.java line 52:
> 50: while (true) {
> 51: int len = fin.read(buffer);
> 52: if (len < 0) break;
it's clearer to do
`if (len < 0) {
break;
}`
it also matches the style at line 83
test/jdk/java/security/cert/CertificateFactory/SlowStream.java line 60:
> 58: } catch (final Exception e) {
> 59: failed.set(true);
> 60: exception.set(e);
If you're setting this field in both reader and writer threads, is there a
chance that one overwrites the other?
Thinking about debugging this if the test fails... in the event of a error,
should each thread print its stack trace and then set failed to true. Then at
the end of the test, if failed is true, throw a RuntimeException with a generic
message that something failed?
test/jdk/java/security/cert/CertificateFactory/SlowStream.java line 68:
> 66: final var factory =
> CertificateFactory.getInstance("X.509");
> 67: if (factory.generateCertificates(inputStream).size() !=
> 5) {
> 68: throw new Exception("Not all certs read");
To aid debugging a failure, it might help to include the number of certificates
that were read.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23394#discussion_r1937723099
PR Review Comment: https://git.openjdk.org/jdk/pull/23394#discussion_r1937716718
PR Review Comment: https://git.openjdk.org/jdk/pull/23394#discussion_r1937724312
