RFR: 8015315: Stream.concat methods

[Paul Sandoz]

Hi Henry,

On Jun 29, 2013, at 2:58 AM, Henry Jen <henry.jen at oracle.com> wrote:

> Hi,
> 
> Please review the webrev that add concat static method to Stream and
> primitive Streams.
> 
> http://cr.openjdk.java.net/~henryjen/ccc/8015315.0/webrev/
> 

Non test-code looks good.

Now that LongStream.range uses concat you need to uncomment the tests in RangeTest.


In ConcatOpTest:

You could do the prerequisite assertion tests in the constructor since the map operation and parallel methods are not affected by the type of the stream. Then for each testXXXConcat you can avoid repeating mapToXxx by doing that in a assertXxxConcat method, and the assertion on the characteristics and contents can be shared between all types. See end of email for an example.

--

I notice some exceptions output from testng:

   [testng] java.lang.ClassNotFoundException: org/openjdk/tests/java/util/stream/ConcatOpTest
   [testng] 	at java.lang.Class.forName0(Native Method)
   [testng] 	at java.lang.Class.forName(Class.java:258)
   [testng] 	at org.testng.internal.ClassHelper.getEnclosingClass(ClassHelper.java:427)
   [testng] 	at org.testng.internal.ClassHelper.createInstance1(ClassHelper.java:349)
   [testng] 	at org.testng.internal.ClassHelper.createInstance(ClassHelper.java:299)
   [testng] 	at org.testng.internal.ClassImpl.getDefaultInstance(ClassImpl.java:110)
   [testng] 	at org.testng.internal.ClassImpl.getInstances(ClassImpl.java:186)
   [testng] 	at org.testng.internal.TestNGClassFinder.<init>(TestNGClassFinder.java:120)
   [testng] 	at org.testng.TestRunner.initMethods(TestRunner.java:409)
   [testng] 	at org.testng.TestRunner.init(TestRunner.java:235)
   [testng] 	at org.testng.TestRunner.init(TestRunner.java:205)
   [testng] 	at org.testng.TestRunner.<init>(TestRunner.java:153)
   [testng] 	at org.testng.SuiteRunner$DefaultTestRunnerFactory.newTestRunner(SuiteRunner.java:522)
   [testng] 	at org.testng.SuiteRunner.init(SuiteRunner.java:157)
   [testng] 	at org.testng.SuiteRunner.<init>(SuiteRunner.java:111)
   [testng] 	at org.testng.TestNG.createSuiteRunner(TestNG.java:1273)
   [testng] 	at org.testng.TestNG.createSuiteRunners(TestNG.java:1260)
   [testng] 	at org.testng.TestNG.runSuitesLocally(TestNG.java:1114)
   [testng] 	at org.testng.TestNG.run(TestNG.java:1031)
   [testng] 	at org.testng.TestNG.privateMain(TestNG.java:1338)
   [testng] 	at org.testng.TestNG.main(TestNG.java:1307)

It does not seem to affect test execution and appears to be caused by the inner class used for your clever combination of a data provider and a factory.

--

Just pointing out the following more for interest/education. The tests for unordered are OK, but the concat implementation could if there were any benefit decide to do something different if both streams are unordered e.g. there is no difference if the concatenation is b::a rather than a::b. If so that would break the tests. Let's fix 'em if in the unlikely event we ever get to that point :-)

Paul.

        private <T> void assertConcatContents(Spliterator<T> s, boolean ordered, Spliterator<T> expected) {
            // concat stream cannot guarantee uniqueness
            assertFalse(s.hasCharacteristics(Spliterator.DISTINCT), scenario);
            // concat stream cannot guarantee sorted
            assertFalse(s.hasCharacteristics(Spliterator.SORTED), scenario);
            // concat stream is ordered if bothe are ordered
            assertEquals(s.hasCharacteristics(Spliterator.ORDERED), ordered, scenario);

            // Verify elements
            assertEquals(toBoxedList(s), toBoxedList(expected), scenario);

        }

        private void assertDoubleConcat(Stream<Integer> s1, Stream<Integer> s2, boolean parallel, boolean ordered) {
            DoubleStream result = DoubleStream.concat(s1.mapToDouble(Integer::doubleValue),
                                                      s2.mapToDouble(Integer::doubleValue));
            assertEquals(result.isParallel(), parallel);

            assertConcatContents(result.spliterator(),
                                 ordered,
                                 expected.stream().mapToDouble(Integer::doubleValue).spliterator());
        }

        public void testDoubleConcat() {
            // sequential + sequential -> sequential
            assertDoubleConcat(c1.stream(), c2.stream(), false, true);
            // parallel + parallel -> parallel
            assertDoubleConcat(c1.parallelStream(), c2.parallelStream(), true, true);
            // sequential + parallel -> parallel
            assertDoubleConcat(c1.stream(), c2.parallelStream(), true, true);
            // parallel + sequential -> parallel
            assertDoubleConcat(c1.parallelStream(), c2.stream(), true, true);

            // not ordered
            assertDoubleConcat(c1.stream().unordered(), c2.stream(), false, false);
            assertDoubleConcat(c1.stream(), c2.stream().unordered(), false, false);
            assertDoubleConcat(c1.parallelStream().unordered(), c2.stream().unordered(), true, false);
        }