Hi Henry, On Jun 29, 2013, at 2:58 AM, Henry Jen <henry....@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); }
