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 <[email protected]> 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 <[email protected]> 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
>
