Hi Any,

Much better.

I suspect out of laziness when hacking i annotated the test methods with:

  @SuppressWarnings({"unchecked", "rawtypes”})

Can we remove those with an appropriate adjustment to the test method 
signatures?

SpliteratorTraversingAndSplittingTest
—

 897         Object[][] bitStreamTestcases = new Object[][] {
 898                 { "none", IntStream.empty().toArray() },
 899                 { "index 0", IntStream.of(0).toArray() },
 900                 { "index 255", IntStream.of(255).toArray() },
 901                 { "index 0 and 255", IntStream.of(0, 255).toArray() },
 902                 { "index Integer.MAX_VALUE", 
IntStream.of(Integer.MAX_VALUE).toArray() },
 903                 { "index Integer.MAX_VALUE - 1", 
IntStream.of(Integer.MAX_VALUE - 1).toArray() },
 904                 { "index 0 and Integer.MAX_VALUE", IntStream.of(0, 
Integer.MAX_VALUE).toArray() },
 905                 { "every bit", IntStream.range(0, 255).toArray() },
 906                 { "step 2", IntStream.range(0, 255).map(f -> f * 
2).toArray() },
 907                 { "step 3", IntStream.range(0, 255).map(f -> f * 
3).toArray() },
 908                 { "step 5", IntStream.range(0, 255).map(f -> f * 
5).toArray() },
 909                 { "step 7", IntStream.range(0, 255).map(f -> f * 
7).toArray() },
 910                 { "1, 10, 100, 1000", IntStream.of(1, 10, 100, 
1000).toArray() },

We should be careful adding Integer.MAX_VALUE tests for BitStream as the can 
consume lots of memory.

My suggestion is not to add them to this test and instead consider, as a follow 
on issue, moving spliterator testing of BitSet to BitSetStreamTest, where we 
can better control large memory consuming cases.

Paul.

> On 25 Jan 2017, at 03:29, Amy Lu <amy...@oracle.com> wrote:
> 
> Thank you Paul for reviewing.
> 
> Webrev updated: http://cr.openjdk.java.net/~amlu/8169903/webrev.01/
> 
> Note that SpliteratorTestHelper.java now still under 
> test/java/util/stream/bootlib/java.base but changed to "java.util" (instead 
> of java.util.stream). It may be re-located to test/lib/testlibrary, together 
> with other stream-based test libs, later in JDK-8085814.
> 
> Thanks,
> Amy
> 
> 
> On 1/20/17 3:22 AM, Paul Sandoz wrote:
>>> On 19 Jan 2017, at 05:37, Amy Lu <amy...@oracle.com> wrote:
>>> 
>>> This is test-only change, can still go into JDK 9?
>>> 
>> Yes.
>> 
>> It’s verbose but i would prefer if you can explicitly declare each test 
>> rather than having one test deferring to 
>> SpliteratorTestHelper.testSpliterator. So in effect the test methods are 
>> proxies to the equivalent methods on the helper. That way it’s easier to 
>> diagnose test failures.
>> 
>> The expected contents (collection) has also been removed. It’s important in 
>> many cases to be able to pass this independently and not derive the expected 
>> result from traversing the spliterator, as that could mask bugs.
>> 
>> There is no need to create separate classes for primitive tests (you only 
>> added explicit for int traversing and not long and double).
>> 
>> 
>> Independently i wonder if 
>> test/java/util/stream/bootlib/java.base/java/util/stream/SpliteratorTestHelper.java
>>  is the right location. I know it’s used in Streams, but perhaps it and the 
>> provider should be placed within a library?
>> 
>> Paul.
>> 
>>> Thanks,
>>> Amy
>>> 
>>> On 1/19/17 9:34 PM, Amy Lu wrote:
>>>> java/util/Spliterator/SpliteratorTraversingAndSplittingTest.java
>>>> java/util/Spliterator/SpliteratorCollisions.java
>>>> 
>>>> Test functions in above tests are almost duplicate with functions in 
>>>> java/util/stream/SpliteratorTestHelper.java. Test can reuse test functions 
>>>> from SpliteratorTestHelper, but with it’s own DataProvider.
>>>> 
>>>> Please review the patch for refactoring spliterator traversing tests.
>>>> 
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8169903
>>>> webrev: http://cr.openjdk.java.net/~amlu/8169903/webrev.00/
>>>> 
>>>> SpliteratorTestHelper.java has a minor update, added one small testcase 
>>>> that originally from SpliteratorCollisions.testForEach.
>>>> 
>>>> The two skipped tests in SpliteratorCollisions.java are now enabled back, 
>>>> as mentioned bug has already been fixed.
>>>> 256 /* skip this test until 8013649 is fixed
>>>> ...
>>>> 268 */
>>>> 
>>>> This patch also brings back Integer.MAX_VALUE test data which requires big 
>>>> memory (and removed in JDK-8169838), in a separate test.
>>>> 
>>>> Thanks,
>>>> Amy
> 

Reply via email to