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