> 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 >>>>>> >