On Sat, 20 Aug 2022 02:41:50 GMT, Daniel D. Daugherty <dcu...@openjdk.org>
wrote:
>> Split the java/util/stream/SpinedBufferTest.java test into two parts:
>> - java/util/stream/SpinedBufferTest1.java has the first 6 test cases
>> - java/util/stream/SpinedBufferTes2.java has the second 6 test cases
>>
>> I couldn't figure out a way to set a larger timeout value for the entirety
>> of java/util/stream/SpinedBufferTest.java and I saw that other folks solved
>> this problem with testng tests by splitting the test into more parts.
>>
>> This fix is being tested in my jdk-20+10 stress testing run.
>
> Daniel D. Daugherty has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Refactor SpinedBufferTest.java by type.
Looks much better, I have only a few Java stylistic nits.
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 {
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;
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.
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 {
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?
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());
}
-------------
PR: https://git.openjdk.org/jdk/pull/9845
