Skip to content

Commit

Permalink
Improve performance of ParameterizedTestSpiInstantiator
Browse files Browse the repository at this point in the history
Since Class.getDeclaredConstructors() clones the constructors array AND
makes "child copies" of the constructors, it's usually better to avoid
repeated calls to getDeclaredConstructors() for a single use case. In
addition, it's good to avoid the use of Optional, multiple Streams, and
try-catch blocks if feasible.

This commit reworks ParameterizedTestSpiInstantiator in order to
achieve that by switching to old-school Java constructs like arrays,
for-loops, and if-blocks.

See #4018
See #4025
  • Loading branch information
sbrannen committed Sep 25, 2024
1 parent 48cd00d commit daea6f1
Showing 1 changed file with 24 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,10 @@

package org.junit.jupiter.params;

import static org.junit.platform.commons.util.CollectionUtils.getFirstElement;

import java.lang.reflect.Constructor;
import java.util.Optional;

import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.PreconditionViolationException;
import org.junit.platform.commons.util.Preconditions;
import org.junit.platform.commons.util.ReflectionUtils;

Expand All @@ -28,46 +24,46 @@ class ParameterizedTestSpiInstantiator {

static <T> T instantiate(Class<T> spiInterface, Class<? extends T> implementationClass,
ExtensionContext extensionContext) {

return extensionContext.getExecutableInvoker() //
.invoke(findConstructor(spiInterface, implementationClass));
}

/**
* Find the "best" constructor for the supplied class.
* Find the "best" constructor for the supplied implementation class.
*
* <p>For backward compatibility, it first checks for a default constructor
* which takes precedence over any other constructor. If no default
* constructor is found, it checks for a single constructor and returns it.
* <p>For backward compatibility, it first checks for a single constructor
* and returns that. If there are multiple constructors, it checks for a
* default constructor which takes precedence over any other constructors.
* Otherwise, this method throws an exception stating that it failed to
* find a suitable constructor.
*/
@SuppressWarnings("unchecked")
private static <T, V extends T> Constructor<? extends V> findConstructor(Class<T> spiInterface,
Class<V> implementationClass) {

Preconditions.condition(!ReflectionUtils.isInnerClass(implementationClass),
() -> String.format("The %s [%s] must be either a top-level class or a static nested class",
spiInterface.getSimpleName(), implementationClass.getName()));

return findDefaultConstructor(implementationClass) //
.orElseGet(() -> findSingleConstructor(spiInterface, implementationClass));
}

@SuppressWarnings("unchecked")
private static <T> Optional<Constructor<T>> findDefaultConstructor(Class<T> clazz) {
return getFirstElement(ReflectionUtils.findConstructors(clazz, it -> it.getParameterCount() == 0)) //
.map(it -> (Constructor<T>) it);
}
Constructor<?>[] constructors = implementationClass.getDeclaredConstructors();

private static <T, V extends T> Constructor<V> findSingleConstructor(Class<T> spiInterface,
Class<V> implementationClass) {

try {
return ReflectionUtils.getDeclaredConstructor(implementationClass);
// Single constructor?
if (constructors.length == 1) {
return (Constructor<V>) constructors[0];
}
catch (PreconditionViolationException ex) {
String message = String.format(
"Failed to find constructor for %s [%s]. "
+ "Please ensure that a no-argument or a single constructor exists.",
spiInterface.getSimpleName(), implementationClass.getName());
throw new JUnitException(message);
// Find default constructor.
for (Constructor<?> constructor : constructors) {
if (constructor.getParameterCount() == 0) {
return (Constructor<V>) constructor;
}
}
// Otherwise...
String message = String.format(
"Failed to find constructor for %s [%s]. "
+ "Please ensure that a no-argument or a single constructor exists.",
spiInterface.getSimpleName(), implementationClass.getName());
throw new JUnitException(message);
}

}

1 comment on commit daea6f1

@marcphilipp
Copy link
Member

Choose a reason for hiding this comment

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

LGTM, thanks! 👍

Polished further in d013085

Please sign in to comment.