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 >