> On 8 Feb 2017, at 01:57, Amy Lu <amy...@oracle.com> wrote:
>
> On 1/26/17 1:04 AM, Paul Sandoz wrote:
>> @SuppressWarnings({"unchecked", "rawtypes”})
>>
>> Can we remove those with an appropriate adjustment to the test method
>> signatures?
>>
>
> Fixed.
SpliteratorCollisions
—
232 testForEach(exp, s, (Consumer<HashableInteger> b) -> b);
Do you require the type declaration for the lambda function parameter?
It might be possible to use UnaryOperator.identity() in those cases.
Then i think we are good to go.
>>
>> 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.
>>
>
> To test/java/util/BitSet/BitSetStreamTest.java? Created JDK-8174171.
>
> For now, I reverted the change of adding back Integer.MAX_VALUE, and also
> removed the change of
> < * @requires os.maxMemory >= 2g
> < * @run testng/othervm -Xms512m -Xmx1024m
> SpliteratorTraversingAndSplittingTest
>
Thanks,
Paul.
>
> Please review the updated webrev:
> http://cr.openjdk.java.net/~amlu/8169903/webrev.02/
>
> Thanks,
> Amy
>
>>
>> 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
>>>>>>
>
