RFR: 8292215: java/util/stream/boottest/java.base/java/util/stream/SpinedBufferTest.java times out with slowdebug [v2]

[Daniel D. Daugherty]

On Mon, 22 Aug 2022 07:35:28 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Daniel D. Daugherty has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Refactor SpinedBufferTest.java by type.
>
> test/jdk/java/util/stream/boottest/java.base/java/util/stream/SpinedBufferTestCommon.java line 27:
> 
>> 25: import java.util.*;
>> 26: 
>> 27: public class SpinedBufferTestCommon {
> 
> Usually done like this:
> 
> Suggestion:
> 
> public abstract class AbstractSpinedBufferTest {

fixed

> test/jdk/java/util/stream/boottest/java.base/java/util/stream/SpinedBufferTestCommon.java line 30:
> 
>> 28: 
>> 29:     // Create sizes around the boundary of spines
>> 30:     static List<Integer> sizes;
> 
> Pre-existing, but code style:
> 
> Suggestion:
> 
>     static final List<Integer> SIZES;

fixed even though it is pre-existing.

> test/jdk/java/util/stream/boottest/java.base/java/util/stream/SpinedBufferTestCommon.java line 43:
> 
>> 41:         catch (Exception e) {
>> 42:             e.printStackTrace();
>> 43:         }
> 
> The usual way to terminate the clinit "cleanly" is:
> 
> Suggestion:
> 
>         } catch (Exception e) {
>             throw new IllegalStateException(e);
>         }
> 
> 
> This allows you to `final` out the `SIZES` field, as dataflow would definitely show the class initialization fails if field init did not happen.

fixed

> test/jdk/java/util/stream/boottest/java.base/java/util/stream/SpinedBufferTestDouble.java line 35:
> 
>> 33: 
>> 34: @Test
>> 35: public class SpinedBufferTestDouble extends SpinedBufferTestCommon {
> 
> Here and later, I think the common naming convention is having the "Test" at the end, i.e.:
> 
> Suggestion:
> 
> public class SpinedBufferDoubleTest extends SpinedBufferTestCommon {

fixed here and the other tests also.

> test/jdk/java/util/stream/boottest/java.base/java/util/stream/SpinedBufferTestDouble.java line 36:
> 
>> 34: @Test
>> 35: public class SpinedBufferTestDouble extends SpinedBufferTestCommon {
>> 36:     // DoubleSpinedBuffer
> 
> Unnecessary comment, I think?

fixed here and the other tests also.

> test/jdk/java/util/stream/boottest/java.base/java/util/stream/SpinedBufferTestInt.java line 94:
> 
>> 92:         PrimitiveIterator.OfInt it = sb.iterator();
>> 93:         for (int i = 0; i < TEST_SIZE; i++)
>> 94:             list2.add(it.nextInt());
> 
> Pre-existing? Here and later, let's keep the braces on:
> 
> Suggestion:
> 
>         for (int i = 0; i < TEST_SIZE; i++) {
>             list2.add(it.nextInt());
>         }

fixed here and in other tests even though it is pre-existing.

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

PR: https://git.openjdk.org/jdk/pull/9845