RFR: 8349151: Refactor test/java/security//cert/CertificateFactory/slowstream.sh to java test [v2]

[Mikhail Yankelevich]

On Fri, 31 Jan 2025 18:01:28 GMT, Matthew Donovan <mdonovan at openjdk.org> wrote:

>> Mikhail Yankelevich has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   cleanup
>
> 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

Done in the next commit

> 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?

Done in the next commit

> 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.

Done in the next commit

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23394#discussion_r1939889619
PR Review Comment: https://git.openjdk.org/jdk/pull/23394#discussion_r1939889552
PR Review Comment: https://git.openjdk.org/jdk/pull/23394#discussion_r1939889676