Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Kotlin Sequence support for @TestFactory methods #3377

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2416,7 +2416,7 @@ generated at runtime by a factory method that is annotated with `@TestFactory`.
In contrast to `@Test` methods, a `@TestFactory` method is not itself a test case but
rather a factory for test cases. Thus, a dynamic test is the product of a factory.
Technically speaking, a `@TestFactory` method must return a single `DynamicNode` or a
`Stream`, `Collection`, `Iterable`, `Iterator`, or array of `DynamicNode` instances.
`Stream`, `Collection`, `Iterable`, `Iterator`, an `Iterator` providing class or array of `DynamicNode` instances.
Instantiable subclasses of `DynamicNode` are `DynamicContainer` and `DynamicTest`.
`DynamicContainer` instances are composed of a _display name_ and a list of dynamic child
nodes, enabling the creation of arbitrarily nested hierarchies of dynamic nodes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private Stream<DynamicNode> toDynamicNodeStream(Object testFactoryMethodResult)

private JUnitException invalidReturnTypeException(Throwable cause) {
String message = String.format(
"@TestFactory method [%s] must return a single %2$s or a Stream, Collection, Iterable, Iterator, or array of %2$s.",
"@TestFactory method [%s] must return a single %2$s or a Stream, Collection, Iterable, Iterator, Iterator-source or array of %2$s.",
getTestMethod().toGenericString(), DynamicNode.class.getName());
return new JUnitException(message, cause);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static org.apiguardian.api.API.Status.INTERNAL;

import java.lang.reflect.Array;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -27,6 +28,7 @@
import java.util.ListIterator;
import java.util.Optional;
import java.util.Set;
import java.util.Spliterator;
import java.util.function.Consumer;
import java.util.stream.Collector;
import java.util.stream.DoubleStream;
Expand All @@ -35,7 +37,9 @@
import java.util.stream.Stream;

import org.apiguardian.api.API;
import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.PreconditionViolationException;
import org.junit.platform.commons.function.Try;

/**
* Collection of utilities for working with {@link Collection Collections}.
Expand Down Expand Up @@ -122,7 +126,7 @@ public static <T> Set<T> toSet(T[] values) {
* returned, so if more control over the returned list is required,
* consider creating a new {@code Collector} implementation like the
* following:
*
* <p>
* <pre class="code">
* public static &lt;T&gt; Collector&lt;T, ?, List&lt;T&gt;&gt; toUnmodifiableList(Supplier&lt;List&lt;T&gt;&gt; listSupplier) {
* return Collectors.collectingAndThen(Collectors.toCollection(listSupplier), Collections::unmodifiableList);
Expand Down Expand Up @@ -162,7 +166,11 @@ public static boolean isConvertibleToStream(Class<?> type) {
|| Iterable.class.isAssignableFrom(type)//
|| Iterator.class.isAssignableFrom(type)//
|| Object[].class.isAssignableFrom(type)//
|| (type.isArray() && type.getComponentType().isPrimitive()));
|| (type.isArray() && type.getComponentType().isPrimitive())//
|| Arrays.stream(type.getMethods())//
.filter(m -> m.getName().equals("iterator"))//
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ If possible, I think we should check whether type implements kotlin.sequences.Sequence here because it's safer. We will need to defensively load the Kotlin type via reflection.

Copy link
Author

@hanszt hanszt May 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed the same logic, the Kotlin language is following for for-loops, namely that anything that provides an iterator, can be iterated over using a for-loop. See also https://kotlinlang.org/docs/control-flow.html#for-loops.

I applied this logic to the conversion from anything that provides an iterator to convert to a stream. This also allows a Kotlin sequence to be converted to a stream. Don't you think it is safe enough to check whether the object to convert to a stream contains an iterator providing method with the name 'iterator' and actually provides a java.util.Iterator?

I'm not sure yet how to check via reflection if we are dealing with a Kotlin Sequence without having a dependency to the Kotlin standard library in the junit-platform-commons module.

.map(Method::getReturnType)//
.anyMatch(returnType -> returnType == Iterator.class));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ How does this work? Isn't the return type kotlin.collections.Iterator, not java.util.Iterator?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works because when Kotlin is used under the Jvm, some types are mapped to Java types. The kotlin.collections-Iterator is one of them and is mapped to java.util.Iterator. See also: https://kotlinlang.org/docs/java-interop.html#mapped-types.

}

/**
Expand All @@ -178,6 +186,7 @@ public static boolean isConvertibleToStream(Class<?> type) {
* <li>{@link Iterator}</li>
* <li>{@link Object} array</li>
* <li>primitive array</li>
* <li>An object that contains a method with name `iterator` returning an Iterator object</li>
* </ul>
*
* @param object the object to convert into a stream; never {@code null}
Expand Down Expand Up @@ -224,8 +233,31 @@ public static Stream<?> toStream(Object object) {
if (object.getClass().isArray() && object.getClass().getComponentType().isPrimitive()) {
return IntStream.range(0, Array.getLength(object)).mapToObj(i -> Array.get(object, i));
}
throw new PreconditionViolationException(
"Cannot convert instance of " + object.getClass().getName() + " into a Stream: " + object);
return tryConvertToStreamByReflection(object);
}

private static Stream<?> tryConvertToStreamByReflection(Object object) {
Preconditions.notNull(object, "Object must not be null");
try {
String name = "iterator";
Method method = object.getClass().getMethod(name);
if (method.getReturnType() == Iterator.class) {
return stream(() -> tryIteratorToSpliterator(object, method), ORDERED, false);
}
else {
throw new PreconditionViolationException(
"Method with name 'iterator' does not return " + Iterator.class.getName());
}
}
catch (NoSuchMethodException | IllegalStateException e) {
throw new PreconditionViolationException(//
"Cannot convert instance of " + object.getClass().getName() + " into a Stream: " + object, e);
}
}

private static Spliterator<?> tryIteratorToSpliterator(Object object, Method method) {
return Try.call(() -> spliteratorUnknownSize((Iterator<?>) method.invoke(object), ORDERED))//
.getOrThrow(e -> new JUnitException("Cannot invoke method " + method.getName() + " onto " + object, e));//
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright 2015-2024 the original author or authors.
*
* All rights reserved. This program and the accompanying materials are
* made available under the terms of the Eclipse Public License v2.0 which
* accompanies this distribution and is available at
*
* https://www.eclipse.org/legal/epl-v20.html
*/
package org.junit.jupiter.api

import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.DynamicTest.dynamicTest
import java.math.BigDecimal
import java.math.BigDecimal.ONE
import java.math.MathContext
import java.math.BigInteger as BigInt
import java.math.RoundingMode as Rounding

/**
* Unit tests for JUnit Jupiter [TestFactory] use in kotlin classes.
*
* @since 5.12
*/
class KotlinDynamicTests {
@Nested
inner class SequenceReturningTestFactoryTests {
@TestFactory
fun `Dynamic tests returned as Kotlin sequence`() =
generateSequence(0) { it + 2 }
.map { dynamicTest("$it should be even") { assertEquals(0, it % 2) } }
.take(10)

@TestFactory
fun `Consecutive fibonacci nr ratios, should converge to golden ratio as n increases`(): Sequence<DynamicTest> {
val scale = 5
val goldenRatio =
(ONE + 5.toBigDecimal().sqrt(MathContext(scale + 10, Rounding.HALF_UP)))
.divide(2.toBigDecimal(), scale, Rounding.HALF_UP)

fun shouldApproximateGoldenRatio(
cur: BigDecimal,
next: BigDecimal
) = next.divide(cur, scale, Rounding.HALF_UP).let {
dynamicTest("$cur / $next = $it should approximate the golden ratio in $scale decimals") {
assertEquals(goldenRatio, it)
}
}
return generateSequence(BigInt.ONE to BigInt.ONE) { (cur, next) -> next to cur + next }
.map { (cur) -> cur.toBigDecimal() }
.zipWithNext(::shouldApproximateGoldenRatio)
.drop(14)
.take(10)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright 2015-2024 the original author or authors.
*
* All rights reserved. This program and the accompanying materials are
* made available under the terms of the Eclipse Public License v2.0 which
* accompanies this distribution and is available at
*
* https://www.eclipse.org/legal/epl-v20.html
*/
package org.junit.jupiter.params.aggregator

import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.Arguments.arguments
import org.junit.jupiter.params.provider.MethodSource
import java.time.Month

/**
* Tests for ParameterizedTest kotlin compatibility
*/
object KotlinParameterizedTests {
@ParameterizedTest
@MethodSource("dataProvidedByKotlinSequence")
fun `a method source can be supplied by a Sequence returning method`(
value: Int,
month: Month
) {
assertEquals(value, month.value)
}

@JvmStatic
private fun dataProvidedByKotlinSequence() =
sequenceOf(
arguments(1, Month.JANUARY),
arguments(3, Month.MARCH),
arguments(8, Month.AUGUST),
arguments(5, Month.MAY),
arguments(12, Month.DECEMBER)
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.Spliterator;
import java.util.Spliterators;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.DoubleStream;
import java.util.stream.IntStream;
Expand Down Expand Up @@ -140,6 +142,7 @@ class StreamConversion {
Collection.class, //
Iterable.class, //
Iterator.class, //
IteratorProvider.class, //
Object[].class, //
String[].class, //
int[].class, //
Expand All @@ -161,10 +164,11 @@ static Stream<Object> objectsConvertibleToStreams() {
Stream.of("cat", "dog"), //
DoubleStream.of(42.3), //
IntStream.of(99), //
LongStream.of(100000000), //
LongStream.of(100_000_000), //
Set.of(1, 2, 3), //
Arguments.of((Object) new Object[] { 9, 8, 7 }), //
new int[] { 5, 10, 15 }//
new int[] { 5, 10, 15 }, //
IteratorProvider.of(new Integer[] { 1, 2, 3, 4, 5 })//
);
}

Expand All @@ -175,6 +179,8 @@ static Stream<Object> objectsConvertibleToStreams() {
Object.class, //
Integer.class, //
String.class, //
IteratorProviderNotUsable.class, //
Spliterator.class, //
int.class, //
boolean.class //
})
Expand Down Expand Up @@ -243,7 +249,7 @@ void toStreamWithLongStream() {
}

@Test
@SuppressWarnings({ "unchecked", "serial" })
@SuppressWarnings({ "unchecked" })
void toStreamWithCollection() {
var collectionStreamClosed = new AtomicBoolean(false);
Collection<String> input = new ArrayList<>() {
Expand Down Expand Up @@ -288,6 +294,24 @@ void toStreamWithIterator() {
assertThat(result).containsExactly("foo", "bar");
}

@Test
@SuppressWarnings("unchecked")
void toStreamWithIteratorProvider() {
final var input = IteratorProvider.of(new String[] { "foo", "bar" });

final var result = (Stream<String>) CollectionUtils.toStream(input);

assertThat(result).containsExactly("foo", "bar");
}

@Test
void throwWhenIteratorNamedMethodDoesNotReturnAnIterator() {
var o = IteratorProviderNotUsable.of(new String[] { "Test" });
var e = assertThrows(PreconditionViolationException.class, () -> CollectionUtils.toStream(o));

assertEquals("Method with name 'iterator' does not return java.util.Iterator", e.getMessage());
}

@Test
@SuppressWarnings("unchecked")
void toStreamWithArray() {
Expand Down Expand Up @@ -356,4 +380,29 @@ public Object convert(Object source, ParameterContext context) throws ArgumentCo
}
}
}

/**
* An interface that has a method with name 'iterator', returning a java.util/Iterator as a return type
*/
private interface IteratorProvider<T> {

@SuppressWarnings("unused")
Iterator<T> iterator();

static <T> IteratorProvider<T> of(T[] elements) {
return () -> Spliterators.iterator(Arrays.spliterator(elements));
}
}

/**
* An interface that has a method with name 'iterator', but does not return java.util/Iterator as a return type
*/
private interface IteratorProviderNotUsable {
@SuppressWarnings("unused")
Object iterator();

static <T> IteratorProviderNotUsable of(T[] elements) {
return () -> Spliterators.iterator(Arrays.spliterator(elements));
}
}
}