diff --git a/testng-core/src/main/java/org/testng/internal/invokers/AbstractParallelWorker.java b/testng-core/src/main/java/org/testng/internal/invokers/AbstractParallelWorker.java index ad45ce99e6..4ef4679509 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/AbstractParallelWorker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/AbstractParallelWorker.java @@ -4,6 +4,8 @@ import java.util.Collections; import java.util.LinkedList; import java.util.List; +import java.util.Objects; +import org.jspecify.annotations.Nullable; import org.testng.ClassMethodMap; import org.testng.IClassListener; import org.testng.ITestContext; @@ -26,16 +28,29 @@ public static AbstractParallelWorker newWorker( public abstract List> createWorkers(Arguments arguments); public static class Arguments { - private List methods; - private IInvoker invoker; - private ConfigurationGroupMethods configMethods; - private ClassMethodMap classMethodMap; - private List listeners; - private ITestContext testContext; - private IAnnotationFinder finder; - - private Arguments() { - // We have a builder. Defeat instantiation via constructors. + private final List methods; + private final IInvoker invoker; + private final ConfigurationGroupMethods configMethods; + private final ClassMethodMap classMethodMap; + private final List listeners; + private final ITestContext testContext; + private final IAnnotationFinder finder; + + private Arguments( + List methods, + IInvoker invoker, + ConfigurationGroupMethods configMethods, + ClassMethodMap classMethodMap, + List listeners, + ITestContext testContext, + IAnnotationFinder finder) { + this.methods = methods; + this.invoker = invoker; + this.configMethods = configMethods; + this.classMethodMap = classMethodMap; + this.listeners = listeners; + this.testContext = testContext; + this.finder = finder; } public List getMethods() { @@ -67,49 +82,58 @@ public IAnnotationFinder getFinder() { } public static class Builder { - private final Arguments instance; - - public Builder() { - instance = new Arguments(); - } + private @Nullable List methods; + private @Nullable IInvoker invoker; + private @Nullable ConfigurationGroupMethods configMethods; + private @Nullable ClassMethodMap classMethodMap; + private @Nullable List listeners; + private @Nullable ITestContext testContext; + private @Nullable IAnnotationFinder finder; public Builder methods(List methods) { - instance.methods = methods; + this.methods = methods; return this; } public Builder invoker(IInvoker invoker) { - instance.invoker = invoker; + this.invoker = invoker; return this; } public Builder configMethods(ConfigurationGroupMethods configMethods) { - instance.configMethods = configMethods; + this.configMethods = configMethods; return this; } public Builder classMethodMap(ClassMethodMap classMethodMap) { - instance.classMethodMap = classMethodMap; + this.classMethodMap = classMethodMap; return this; } public Builder listeners(Collection listeners) { - instance.listeners = new LinkedList<>(listeners); + this.listeners = new LinkedList<>(listeners); return this; } public Builder testContext(ITestContext testContext) { - instance.testContext = testContext; + this.testContext = testContext; return this; } public Builder finder(IAnnotationFinder finder) { - instance.finder = finder; + this.finder = finder; return this; } public Arguments build() { - return instance; + return new Arguments( + Objects.requireNonNull(methods), + Objects.requireNonNull(invoker), + Objects.requireNonNull(configMethods), + Objects.requireNonNull(classMethodMap), + Objects.requireNonNull(listeners), + Objects.requireNonNull(testContext), + Objects.requireNonNull(finder)); } } } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/Arguments.java b/testng-core/src/main/java/org/testng/internal/invokers/Arguments.java index 4eb20310b8..e2913e8f7e 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/Arguments.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/Arguments.java @@ -1,25 +1,27 @@ package org.testng.internal.invokers; import java.util.Map; +import org.jspecify.annotations.Nullable; import org.testng.ITestNGMethod; public class Arguments { - protected final Object instance; - protected final ITestNGMethod tm; + protected final @Nullable Object instance; + protected final @Nullable ITestNGMethod tm; protected final Map params; - protected Arguments(Object instance, ITestNGMethod tm, Map params) { + protected Arguments( + @Nullable Object instance, @Nullable ITestNGMethod tm, Map params) { this.instance = instance; this.tm = tm; this.params = params; } - public Object getInstance() { + public @Nullable Object getInstance() { return instance; } - public ITestNGMethod getTestMethod() { + public @Nullable ITestNGMethod getTestMethod() { return tm; } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ClassBasedParallelWorker.java b/testng-core/src/main/java/org/testng/internal/invokers/ClassBasedParallelWorker.java index 1153356a36..427b86686e 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ClassBasedParallelWorker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ClassBasedParallelWorker.java @@ -6,6 +6,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; import org.testng.IMethodInstance; @@ -54,17 +55,20 @@ public List> createWorkers(Arguments arguments) { params = getParameters(im); prevClass = c; } + // prevClass starts out null, so the first iteration always takes the branch above. + Map currentParams = Objects.requireNonNull(params); if (shouldRunSequentially(c, sequentialClasses)) { if (!processedClasses.contains(c)) { processedClasses.add(c); // Sequential class: all methods in one worker - TestMethodWorker worker = createTestMethodWorker(arguments, methodInstances, params, c); + TestMethodWorker worker = + createTestMethodWorker(arguments, methodInstances, currentParams, c); result.add(worker); } } else { // Parallel class: each method in its own worker TestMethodWorker worker = - createTestMethodWorker(arguments, Collections.singletonList(im), params, c); + createTestMethodWorker(arguments, Collections.singletonList(im), currentParams, c); result.add(worker); } } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java index d8d8ce3aee..b3840dc953 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java @@ -9,8 +9,10 @@ import java.util.Collection; import java.util.HashSet; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import org.jspecify.annotations.Nullable; import org.testng.ConfigurationNotInvokedException; import org.testng.IClass; import org.testng.IConfigurable; @@ -87,17 +89,20 @@ public IConfiguration getConfiguration() { * least one of these methods failed. */ public boolean hasConfigurationFailureFor( - ITestNGMethod testNGMethod, String[] groups, IClass testClass, Object instance) { + @Nullable ITestNGMethod testNGMethod, + String[] groups, + IClass testClass, + @Nullable Object instance) { return hasConfigurationFailureFor(null, testNGMethod, groups, testClass, instance); } @Override public boolean hasConfigurationFailureFor( - ITestNGMethod configMethod, - ITestNGMethod testNGMethod, + @Nullable ITestNGMethod configMethod, + @Nullable ITestNGMethod testNGMethod, String[] groups, IClass testClass, - Object instance) { + @Nullable Object instance) { boolean result = false; Class cls = testClass.getRealClass(); @@ -124,8 +129,13 @@ public boolean hasConfigurationFailureFor( } // if method is BeforeClass, currentTestMethod will be null if ((m_continueOnFailedConfiguration || annotationFound) && hasConfigFailure(testNGMethod)) { - Object key = TestNgMethodUtils.getMethodInvocationToken(testNGMethod, instance); - result = m_methodInvocationResults.get(testNGMethod).contains(key); + // hasConfigFailure() is false for a null method, and a set of arguments that carries a test + // method carries its instance too, so both are present on this branch. + Object key = + TestNgMethodUtils.getMethodInvocationToken( + Objects.requireNonNull(testNGMethod), Objects.requireNonNull(instance)); + // hasConfigFailure() has just established that the map holds this key. + result = Objects.requireNonNull(m_methodInvocationResults.get(testNGMethod)).contains(key); } else if (!(m_continueOnFailedConfiguration || annotationFound)) { for (Class clazz : m_classInvocationResults.keySet()) { if (clazz.isAssignableFrom(cls) && m_classInvocationResults.get(clazz).contains(instance)) { @@ -241,6 +251,8 @@ public void invokeConfigurations(ConfigMethodArguments arguments) { if (null == arguments.getTestClass()) { arguments.setTestClass(tm.getTestClass()); } + // Defaulted just above, so it is set from here on. + IClass testClass = Objects.requireNonNull(arguments.getTestClass()); ITestResult testResult = TestResult.newContextAwareTestResult(tm, m_testContext); testResult.setStatus(ITestResult.STARTED); @@ -251,7 +263,8 @@ public void invokeConfigurations(ConfigMethodArguments arguments) { if (inst == null) { inst = arguments.getInstance(); } - Class objectClass = inst.getClass(); + // Either the configuration method carries its own instance or the caller supplied one. + Class objectClass = Objects.requireNonNull(inst).getClass(); ConstructorOrMethod method = tm.getConstructorOrMethod(); // Only run the configuration if @@ -276,11 +289,7 @@ public void invokeConfigurations(ConfigMethodArguments arguments) { continue; } if (hasConfigurationFailureFor( - tm, - arguments.getTestMethod(), - tm.getGroups(), - arguments.getTestClass(), - arguments.getInstance()) + tm, arguments.getTestMethod(), tm.getGroups(), testClass, arguments.getInstance()) && !alwaysRun) { log(3, "Skipping " + Utils.detailedMethodName(tm, true)); InvokedMethod invokedMethod = new InvokedMethod(System.currentTimeMillis(), testResult); @@ -438,7 +447,8 @@ private IConfigurable computeConfigurableInstance( : m_configuration.getConfigurable(); } - private void runConfigurationListeners(ITestResult tr, ITestNGMethod tm, boolean before) { + private void runConfigurationListeners( + ITestResult tr, @Nullable ITestNGMethod tm, boolean before) { ListenerComparator comparator = m_configuration.getListenerComparator(); if (before) { TestListenerHelper.runPreConfigurationListeners( @@ -462,8 +472,8 @@ private void handleConfigurationSkip( ITestNGMethod tm, ITestResult testResult, IConfigurationAnnotation annotation, - ITestNGMethod currentTestMethod, - Object instance, + @Nullable ITestNGMethod currentTestMethod, + @Nullable Object instance, XmlSuite suite) { recordConfigurationInvocationFailed( tm, testResult.getTestClass(), annotation, currentTestMethod, instance, suite); @@ -471,7 +481,7 @@ private void handleConfigurationSkip( runConfigurationListeners(testResult, currentTestMethod, false /* after */); } - private boolean hasConfigFailure(ITestNGMethod currentTestMethod) { + private boolean hasConfigFailure(@Nullable ITestNGMethod currentTestMethod) { return currentTestMethod != null && m_methodInvocationResults.containsKey(currentTestMethod); } @@ -479,15 +489,16 @@ private void handleConfigurationFailure( Throwable ite, ITestNGMethod tm, ITestResult testResult, - IConfigurationAnnotation annotation, - ITestNGMethod currentTestMethod, - Object instance, + @Nullable IConfigurationAnnotation annotation, + @Nullable ITestNGMethod currentTestMethod, + @Nullable Object instance, XmlSuite suite) { Throwable cause = ite.getCause() != null ? ite.getCause() : ite; if (isSkipExceptionAndSkip(cause)) { testResult.setThrowable(cause); - handleConfigurationSkip(tm, testResult, annotation, currentTestMethod, instance, suite); + handleConfigurationSkip( + tm, testResult, Objects.requireNonNull(annotation), currentTestMethod, instance, suite); return; } Utils.log( @@ -523,7 +534,7 @@ private static boolean isConfigMethodEligibleForScrutiny(ITestNGMethod tm) { } /** @return true if this class or a parent class failed to initialize. */ - private boolean classConfigurationFailed(Class cls, Object instance) { + private boolean classConfigurationFailed(Class cls, @Nullable Object instance) { return m_classInvocationResults.entrySet().stream() .anyMatch( classSetEntry -> { @@ -537,7 +548,7 @@ private boolean classConfigurationFailed(Class cls, Object instance) { } private static void copyAttributesFromNativelyInjectedTestResult( - Object[] source, ITestResult target) { + Object[] source, @Nullable ITestResult target) { if (source == null || target == null) { return; } @@ -547,17 +558,21 @@ private static void copyAttributesFromNativelyInjectedTestResult( .ifPresent(eachSource -> TestResult.copyAttributes((ITestResult) eachSource, target)); } - private void setMethodInvocationFailure(ITestNGMethod method, Object instance) { + private void setMethodInvocationFailure( + @Nullable ITestNGMethod method, @Nullable Object instance) { if (method == null) { return; } Set instances = m_methodInvocationResults.computeIfAbsent(method, k -> new HashSet<>()); - instances.add(TestNgMethodUtils.getMethodInvocationToken(method, instance)); + // Both come from one set of arguments, and a set that carries a test method carries its + // instance too, so a non-null method means a non-null instance. + instances.add( + TestNgMethodUtils.getMethodInvocationToken(method, Objects.requireNonNull(instance))); } private final AutoCloseableLock internalLock = new AutoCloseableLock(); - private void setClassInvocationFailure(Class clazz, Object instance) { + private void setClassInvocationFailure(Class clazz, @Nullable Object instance) { try (AutoCloseableLock ignore = internalLock.lock()) { Set instances = m_classInvocationResults.computeIfAbsent(clazz, k -> new HashSet<>()); Object objectToAdd = instance == null ? NULL_OBJECT : instance; @@ -573,8 +588,8 @@ private void recordConfigurationInvocationFailed( ITestNGMethod tm, IClass testClass, IConfigurationAnnotation annotation, - ITestNGMethod currentTestMethod, - Object instance, + @Nullable ITestNGMethod currentTestMethod, + @Nullable Object instance, XmlSuite suite) { // If beforeTestClass or afterTestClass failed, mark either the config method's // entire class as failed, or the class under tests as failed, depending on @@ -630,7 +645,7 @@ else if (annotation.getBeforeTest() || annotation.getAfterTest()) { } } - private static Object computeInstance(Object instance, Object inst, ITestNGMethod tm) { + private static Object computeInstance(@Nullable Object instance, Object inst, ITestNGMethod tm) { if (instance == null || !tm.getConstructorOrMethod().getDeclaringClass().isAssignableFrom(instance.getClass())) { return inst; @@ -671,7 +686,7 @@ private static boolean canIgnoreConfigFailure(ITestNGMethod method) { return method.isIgnoreFailure(); } - private boolean canIgnoreConfigFailure(IClass testClass, ITestNGMethod configMethod) { + private boolean canIgnoreConfigFailure(IClass testClass, @Nullable ITestNGMethod configMethod) { boolean instanceMatch = testClass instanceof ITestClass; if (!instanceMatch) { return false; diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ConfigMethodArguments.java b/testng-core/src/main/java/org/testng/internal/invokers/ConfigMethodArguments.java index 728fc25974..86a7e989b4 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ConfigMethodArguments.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ConfigMethodArguments.java @@ -2,6 +2,8 @@ import java.util.Collection; import java.util.Map; +import java.util.Objects; +import org.jspecify.annotations.Nullable; import org.testng.IClass; import org.testng.ITestNGMethod; import org.testng.ITestResult; @@ -9,20 +11,20 @@ public class ConfigMethodArguments extends MethodArguments { - private IClass testClass; + private @Nullable IClass testClass; private final ITestNGMethod[] allMethods; private final XmlSuite suite; - private final ITestResult testMethodResult; + private final @Nullable ITestResult testMethodResult; private ConfigMethodArguments( - IClass testClass, - ITestNGMethod currentTestMethod, + @Nullable IClass testClass, + @Nullable ITestNGMethod currentTestMethod, ITestNGMethod[] allMethods, XmlSuite suite, Map params, - Object[] parameterValues, - Object instance, - ITestResult testMethodResult) { + Object @Nullable [] parameterValues, + @Nullable Object instance, + @Nullable ITestResult testMethodResult) { super(instance, currentTestMethod, params, parameterValues); this.testClass = testClass; this.allMethods = allMethods; @@ -30,7 +32,7 @@ private ConfigMethodArguments( this.testMethodResult = testMethodResult; } - public IClass getTestClass() { + public @Nullable IClass getTestClass() { return testClass; } @@ -42,7 +44,7 @@ public XmlSuite getSuite() { return suite; } - public ITestResult getTestMethodResult() { + public @Nullable ITestResult getTestMethodResult() { return testMethodResult; } @@ -52,14 +54,14 @@ public void setTestClass(IClass testClass) { public static class Builder { - private IClass testClass; - private ITestNGMethod currentTestMethod; - private ITestNGMethod[] allMethods; - private XmlSuite suite; - private Map params; - private Object[] parameterValues; - private Object instance; - private ITestResult testMethodResult; + private @Nullable IClass testClass; + private @Nullable ITestNGMethod currentTestMethod; + private ITestNGMethod @Nullable [] allMethods; + private @Nullable XmlSuite suite; + private @Nullable Map params; + private Object @Nullable [] parameterValues; + private @Nullable Object instance; + private @Nullable ITestResult testMethodResult; public Builder forTestClass(IClass testClass) { this.testClass = testClass; @@ -93,7 +95,7 @@ public Builder usingParameters(Map params) { return this; } - public Builder usingParameterValues(Object[] parameterValues) { + public Builder usingParameterValues(Object @Nullable [] parameterValues) { this.parameterValues = parameterValues; return this; } @@ -112,9 +114,9 @@ public ConfigMethodArguments build() { return new ConfigMethodArguments( testClass, currentTestMethod, - allMethods, - suite, - params, + Objects.requireNonNull(allMethods), + Objects.requireNonNull(suite), + Objects.requireNonNull(params), parameterValues, instance, testMethodResult); diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ExceptionUtils.java b/testng-core/src/main/java/org/testng/internal/invokers/ExceptionUtils.java index cf585025ad..32479616f1 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ExceptionUtils.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ExceptionUtils.java @@ -1,6 +1,7 @@ package org.testng.internal.invokers; import java.util.Set; +import org.jspecify.annotations.Nullable; import org.testng.IInvokedMethod; import org.testng.ITestContext; import org.testng.ITestNGMethod; @@ -12,7 +13,7 @@ private ExceptionUtils() { // Utility class. Defeat instantiation. } - static Throwable getExceptionDetails(ITestContext context, Object instance) { + static @Nullable Throwable getExceptionDetails(ITestContext context, Object instance) { Set configResults = context.getFailedConfigurations().getAllResults(); if (configResults.isEmpty()) { configResults = context.getSkippedConfigurations().getAllResults(); @@ -41,7 +42,7 @@ private static boolean sameInstance(ITestResult configResult, Object instance) { return instance.equals(configResult.getInstance()); } - private static Throwable getConfigFailureException(ITestContext context) { + private static @Nullable Throwable getConfigFailureException(ITestContext context) { for (IInvokedMethod method : context.getSuite().getAllInvokedMethods()) { ITestNGMethod m = method.getTestMethod(); if (m.isBeforeSuiteConfiguration() && (!method.getTestResult().isSuccess())) { diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ExpectedExceptionsHolder.java b/testng-core/src/main/java/org/testng/internal/invokers/ExpectedExceptionsHolder.java index d073548330..8346287e5d 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ExpectedExceptionsHolder.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ExpectedExceptionsHolder.java @@ -1,6 +1,7 @@ package org.testng.internal.invokers; import java.util.Arrays; +import org.jspecify.annotations.Nullable; import org.testng.IExpectedExceptionsHolder; import org.testng.ITestNGMethod; import org.testng.TestException; @@ -70,7 +71,7 @@ public Throwable wrongException(Throwable ite) { } } - public TestException noException(ITestNGMethod testMethod) { + public @Nullable TestException noException(ITestNGMethod testMethod) { if (hasNoExpectedClasses()) { return null; } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/GroupConfigMethodArguments.java b/testng-core/src/main/java/org/testng/internal/invokers/GroupConfigMethodArguments.java index 53c3fb6c8b..7813630bb2 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/GroupConfigMethodArguments.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/GroupConfigMethodArguments.java @@ -1,6 +1,8 @@ package org.testng.internal.invokers; import java.util.Map; +import java.util.Objects; +import org.jspecify.annotations.Nullable; import org.testng.ITestNGMethod; import org.testng.internal.ConfigurationGroupMethods; import org.testng.xml.XmlSuite; @@ -22,16 +24,32 @@ public ConfigurationGroupMethods getGroupMethods() { return groupMethods; } + /** + * A group configuration is always tied to the test method that triggered it, so this narrows the + * inherited contract back. See {@link TestMethodArguments#getTestMethod()} for why the base is + * nullable at all. + */ + @Override + public ITestNGMethod getTestMethod() { + return Objects.requireNonNull(super.getTestMethod()); + } + + /** Always present, for the same reason as {@link #getTestMethod()}. */ + @Override + public Object getInstance() { + return Objects.requireNonNull(super.getInstance()); + } + public XmlSuite getSuite() { return getTestMethod().getXmlTest().getSuite(); } public static class Builder { - private ITestNGMethod testMethod; - private ConfigurationGroupMethods groupMethods; - private Map params; - private Object instance; + private @Nullable ITestNGMethod testMethod; + private @Nullable ConfigurationGroupMethods groupMethods; + private @Nullable Map params; + private @Nullable Object instance; public Builder forTestMethod(ITestNGMethod testMethod) { this.testMethod = testMethod; @@ -54,7 +72,11 @@ public Builder forInstance(Object instance) { } public GroupConfigMethodArguments build() { - return new GroupConfigMethodArguments(testMethod, groupMethods, params, instance); + return new GroupConfigMethodArguments( + Objects.requireNonNull(testMethod), + Objects.requireNonNull(groupMethods), + Objects.requireNonNull(params), + Objects.requireNonNull(instance)); } } } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/IConfigInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/IConfigInvoker.java index 2b88b35896..6bf36cfe3e 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/IConfigInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/IConfigInvoker.java @@ -1,5 +1,6 @@ package org.testng.internal.invokers; +import org.jspecify.annotations.Nullable; import org.testng.IClass; import org.testng.ITestNGMethod; import org.testng.internal.IConfiguration; @@ -7,14 +8,23 @@ public interface IConfigInvoker { boolean hasConfigurationFailureFor( - ITestNGMethod testNGMethod, String[] groups, IClass testClass, Object instance); - + @Nullable ITestNGMethod testNGMethod, + String[] groups, + IClass testClass, + @Nullable Object instance); + + /** + * @param configMethod the configuration method being scrutinised, or null to ask about the class + * as a whole + * @param testNGMethod null when the configuration is a class or suite level one, which has no + * current test method + */ boolean hasConfigurationFailureFor( - ITestNGMethod configMethod, - ITestNGMethod testNGMethod, + @Nullable ITestNGMethod configMethod, + @Nullable ITestNGMethod testNGMethod, String[] groups, IClass testClass, - Object instance); + @Nullable Object instance); void invokeBeforeGroupsConfigurations(GroupConfigMethodArguments arguments); diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ITestInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/ITestInvoker.java index ed12554a90..0c3b72a7cb 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ITestInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ITestInvoker.java @@ -6,6 +6,7 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import org.jspecify.annotations.Nullable; import org.testng.IInvokedMethod; import org.testng.ITestContext; import org.testng.ITestNGMethod; @@ -43,12 +44,19 @@ FailureContext retryFailed( void runTestResultListener(ITestResult tr); default ITestResult registerSkippedTestResult( - ITestNGMethod testMethod, long start, Throwable throwable) { + ITestNGMethod testMethod, long start, @Nullable Throwable throwable) { return registerSkippedTestResult(testMethod, start, throwable, null); } + /** + * @param source the result to copy attributes and parameters from, or null when the skip has no + * originating result + */ ITestResult registerSkippedTestResult( - ITestNGMethod testMethod, long start, Throwable throwable, ITestResult source); + ITestNGMethod testMethod, + long start, + @Nullable Throwable throwable, + @Nullable ITestResult source); void invokeListenersForSkippedTestResult(ITestResult r, IInvokedMethod invokedMethod); diff --git a/testng-core/src/main/java/org/testng/internal/invokers/InvokeMethodRunnable.java b/testng-core/src/main/java/org/testng/internal/invokers/InvokeMethodRunnable.java index abb8a99a3b..5005c8cac2 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/InvokeMethodRunnable.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/InvokeMethodRunnable.java @@ -2,6 +2,7 @@ import java.util.Optional; import java.util.concurrent.Callable; +import org.jspecify.annotations.Nullable; import org.testng.IHookable; import org.testng.ITestNGMethod; import org.testng.ITestResult; @@ -12,7 +13,7 @@ public class InvokeMethodRunnable implements Callable { private final ITestNGMethod m_method; private final Object m_instance; private final Object[] m_parameters; - private final IHookable m_hookable; + private final @Nullable IHookable m_hookable; private final ITestResult m_testResult; /** @@ -26,7 +27,7 @@ public InvokeMethodRunnable( ITestNGMethod thisMethod, Object instance, Object[] parameters, - IHookable hookable, + @Nullable IHookable hookable, ITestResult testResult) { m_method = thisMethod; m_instance = instance; diff --git a/testng-core/src/main/java/org/testng/internal/invokers/MethodArguments.java b/testng-core/src/main/java/org/testng/internal/invokers/MethodArguments.java index c1bea743dd..c7c9d31780 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/MethodArguments.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/MethodArguments.java @@ -1,19 +1,23 @@ package org.testng.internal.invokers; import java.util.Map; +import org.jspecify.annotations.Nullable; import org.testng.ITestNGMethod; public class MethodArguments extends Arguments { - protected final Object[] parameterValues; + protected final Object @Nullable [] parameterValues; protected MethodArguments( - Object instance, ITestNGMethod tm, Map params, Object[] parameterValues) { + @Nullable Object instance, + @Nullable ITestNGMethod tm, + Map params, + Object @Nullable [] parameterValues) { super(instance, tm, params); this.parameterValues = parameterValues; } - public Object[] getParameterValues() { + public Object @Nullable [] getParameterValues() { return parameterValues; } } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/MethodInvocationHelper.java b/testng-core/src/main/java/org/testng/internal/invokers/MethodInvocationHelper.java index dfbbc9b1f4..3d59739f46 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/MethodInvocationHelper.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/MethodInvocationHelper.java @@ -8,6 +8,7 @@ import java.util.Collection; import java.util.Iterator; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -16,9 +17,9 @@ import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.stream.Stream; +import org.jspecify.annotations.Nullable; import org.testng.IConfigurable; import org.testng.IConfigureCallBack; import org.testng.IHookCallBack; @@ -256,7 +257,7 @@ protected static boolean invokeHookable( final ITestResult testResult) throws Throwable { final Throwable[] error = new Throwable[1]; - AtomicReference wasCalled = new AtomicReference<>(false); + AtomicBoolean wasCalled = new AtomicBoolean(false); IHookCallBack callback = new IHookCallBack() { @@ -305,7 +306,7 @@ protected static boolean invokeWithTimeout( Object instance, Object[] parameterValues, ITestResult testResult, - IHookable hookable) + @Nullable IHookable hookable) throws InterruptedException, ThreadExecutionException { if (ThreadUtil.isTestNGThread() && testResult.getTestContext().getCurrentXmlTest().getParallel() @@ -324,7 +325,7 @@ private static boolean invokeWithTimeoutWithNoExecutor( Object instance, Object[] parameterValues, ITestResult testResult, - IHookable hookable) { + @Nullable IHookable hookable) { Consumer failureMarker = t -> { @@ -392,7 +393,7 @@ private static boolean invokeWithTimeoutWithNewExecutor( Object instance, Object[] parameterValues, ITestResult testResult, - IHookable hookable) + @Nullable IHookable hookable) throws InterruptedException, ThreadExecutionException { ExecutorService exec = ThreadUtil.createExecutor(configuration, 1, tm.getMethodName()); @@ -467,7 +468,9 @@ private static boolean invokeWithTimeoutWithNewExecutor( testResult.setStatus(ITestResult.SUCCESS); // if no exception till here then SUCCESS. return flag; } catch (ExecutionException e) { - throw new ThreadExecutionException(e.getCause()); + // FutureTask never completes exceptionally without a cause, and the only handler of this + // wrapper reads the cause straight back out. + throw new ThreadExecutionException(Objects.requireNonNull(e.getCause())); } } @@ -488,7 +491,7 @@ private static String buildNeverStartedMessage(ITestNGMethod tm, long startupGra + "size."; } - private static StackTraceElement[] getRunningMethodStackTrace(ExecutorService exec) { + private static StackTraceElement @Nullable [] getRunningMethodStackTrace(ExecutorService exec) { if (!(exec instanceof ThreadPoolExecutor)) { return null; } @@ -517,7 +520,7 @@ protected static boolean invokeConfigurable( final ITestResult testResult) throws Throwable { final Throwable[] error = new Throwable[1]; - AtomicReference wasCalled = new AtomicReference<>(false); + AtomicBoolean wasCalled = new AtomicBoolean(false); IConfigureCallBack callback = new IConfigureCallBack() { diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ParameterHandler.java b/testng-core/src/main/java/org/testng/internal/invokers/ParameterHandler.java index c60484f48c..eb7961db12 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ParameterHandler.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ParameterHandler.java @@ -4,6 +4,7 @@ import java.util.Map; import java.util.Optional; +import org.jspecify.annotations.Nullable; import org.testng.DataProviderHolder; import org.testng.IDataProviderMethod; import org.testng.ITestContext; @@ -47,7 +48,7 @@ ParameterBag createParameters( Map parameters, Map allParameterNames, ITestContext testContext, - Object fedInstance) { + @Nullable Object fedInstance) { return handleParameters( testMethod, testMethod.getInstance(), @@ -63,7 +64,7 @@ private ParameterBag handleParameters( Map allParameterNames, Map parameters, ITestContext testContext, - Object fedInstance) { + @Nullable Object fedInstance) { XmlSuite suite = testContext.getCurrentXmlTest().getSuite(); try { MethodParameters methodParams = @@ -104,8 +105,8 @@ private ParameterBag handleParameters( * TestResult} containing the cause */ static class ParameterBag { - final ParameterHolder parameterHolder; - final ITestResult errorResult; + final @Nullable ParameterHolder parameterHolder; + final @Nullable ITestResult errorResult; boolean bubbleUpFailures = false; ParameterBag(ParameterHolder parameterHolder) { diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ParameterHolder.java b/testng-core/src/main/java/org/testng/internal/invokers/ParameterHolder.java index 9dbc7cbbc8..dadab3636f 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/ParameterHolder.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/ParameterHolder.java @@ -1,6 +1,7 @@ package org.testng.internal.invokers; import java.util.Iterator; +import org.jspecify.annotations.Nullable; import org.testng.IDataProviderMethod; import org.testng.internal.collections.CloseableIterator; @@ -27,7 +28,7 @@ public enum ParameterOrigin { * original resource is released regardless of how the exposed iterator was wrapped or how much of * it was consumed. */ - private final CloseableIterator closeableSource; + private final @Nullable CloseableIterator closeableSource; public ParameterHolder( Iterator parameters, ParameterOrigin origin, IDataProviderMethod dph) { @@ -38,7 +39,7 @@ public ParameterHolder( Iterator parameters, ParameterOrigin origin, IDataProviderMethod dph, - CloseableIterator closeableSource) { + @Nullable CloseableIterator closeableSource) { super(); this.parameters = parameters; this.origin = origin; diff --git a/testng-core/src/main/java/org/testng/internal/invokers/SuiteRunnerMap.java b/testng-core/src/main/java/org/testng/internal/invokers/SuiteRunnerMap.java index 083e5993bd..d0bf623177 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/SuiteRunnerMap.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/SuiteRunnerMap.java @@ -3,6 +3,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; +import org.jspecify.annotations.Nullable; import org.testng.ISuite; import org.testng.TestNGException; import org.testng.xml.XmlSuite; @@ -19,7 +20,7 @@ public void put(XmlSuite xmlSuite, ISuite suite) { m_map.put(name, suite); } - public ISuite get(XmlSuite xmlSuite) { + public @Nullable ISuite get(XmlSuite xmlSuite) { return m_map.get(xmlSuite.getName()); } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java index 2b1ae6bcb3..775c1a04b3 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java @@ -22,6 +22,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import java.util.stream.Collectors; +import org.jspecify.annotations.Nullable; import org.testng.DataProviderHolder; import org.testng.IClassListener; import org.testng.IDataProviderListener; @@ -344,7 +345,7 @@ private DataProviderHolder buildDataProviderHolder() { * @param testMethod test method being checked for * @return error message or null if dependencies have been run successfully */ - private String checkDependencies(ITestNGMethod testMethod) { + private @Nullable String checkDependencies(ITestNGMethod testMethod) { // If this method is marked alwaysRun, no need to check for its dependencies if (testMethod.isAlwaysRun()) { return null; @@ -687,8 +688,11 @@ private boolean shouldRetryTestMethod( // pass both paramValues and paramIndex to be thread safe in case parallel=true + dataprovider. private ITestResult invokeMethod( TestMethodArguments arguments, XmlSuite suite, FailureContext failureContext) { + // Every route in here rebuilds the arguments with the values for this one invocation; the + // template object the invocation loop starts from never reaches this method. + Object[] parameterValues = Objects.requireNonNull(arguments.getParameterValues()); TestResult testResult = - TestResult.newTestResult(arguments.getParameterValues(), arguments.getParametersIndex()); + TestResult.newTestResult(parameterValues, arguments.getParametersIndex()); testResult.setHost(m_testContext.getHost()); GroupConfigMethodArguments cfgArgs = @@ -783,14 +787,13 @@ private ITestResult invokeMethod( willfullyIgnored = !MethodInvocationHelper.invokeHookable( arguments.getInstance(), - arguments.getParameterValues(), + parameterValues, hookableInstance, thisMethod, testResult); } else { // Not a IHookable, invoke directly - MethodInvocationHelper.invokeMethod( - thisMethod, arguments.getInstance(), arguments.getParameterValues()); + MethodInvocationHelper.invokeMethod(thisMethod, arguments.getInstance(), parameterValues); } if (!willfullyIgnored) { setTestStatus(testResult, ITestResult.SUCCESS); @@ -802,7 +805,7 @@ private ITestResult invokeMethod( m_configuration, arguments.getTestMethod(), arguments.getInstance(), - arguments.getParameterValues(), + parameterValues, testResult, hookableInstance); } @@ -820,7 +823,8 @@ private ITestResult invokeMethod( testResult.setThrowable(ite.getCause()); setTestStatus(testResult, ITestResult.FAILURE); } catch (ThreadExecutionException tee) { // wrapper for TestNGRuntimeException - Throwable cause = tee.getCause(); + // The wrapper is only ever constructed around a cause that is already known to be present. + Throwable cause = Objects.requireNonNull(tee.getCause()); if (TestNGRuntimeException.class.equals(cause.getClass())) { testResult.setThrowable(cause.getCause()); } else { @@ -866,9 +870,7 @@ private ITestResult invokeMethod( // instance a @Factory produced is a separate axis, recorded by FailedReporter as the // factory-instances attribute; writing it here used to overwrite the row index and make the // regenerated suite re-run the wrong ones. - if (testResult.getThrowable() != null - && arguments.getParameterValues().length > 0 - && !willRetryMethod) { + if (testResult.getThrowable() != null && parameterValues.length > 0 && !willRetryMethod) { arguments.getTestMethod().addFailedInvocationNumber(arguments.getParametersIndex()); } @@ -941,7 +943,10 @@ private void runConfigMethods( @Override public ITestResult registerSkippedTestResult( - ITestNGMethod testMethod, long start, Throwable throwable, ITestResult source) { + ITestNGMethod testMethod, + long start, + @Nullable Throwable throwable, + @Nullable ITestResult source) { ITestResult result = TestResult.newEndTimeAwareTestResult(testMethod, m_testContext, throwable, start); if (source != null) { diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestMethodArguments.java b/testng-core/src/main/java/org/testng/internal/invokers/TestMethodArguments.java index bfee95da69..1abf6083e9 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestMethodArguments.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/TestMethodArguments.java @@ -1,6 +1,8 @@ package org.testng.internal.invokers; import java.util.Map; +import java.util.Objects; +import org.jspecify.annotations.Nullable; import org.testng.ITestClass; import org.testng.ITestNGMethod; import org.testng.internal.ConfigurationGroupMethods; @@ -14,9 +16,9 @@ public class TestMethodArguments extends MethodArguments { private final ConfigurationGroupMethods groupMethods; private TestMethodArguments( - Object instance, - ITestNGMethod tm, - Object[] parameterValues, + @Nullable Object instance, + @Nullable ITestNGMethod tm, + Object @Nullable [] parameterValues, int parametersIndex, Map params, ITestClass testClass, @@ -51,17 +53,33 @@ public ITestClass getTestClass() { return testClass; } + /** + * The inherited getter is nullable only to serve {@link ConfigMethodArguments}, which stands for + * suite and test level configurations that have no current test method. A test method invocation + * always has one, so this narrows the contract back. + */ + @Override + public ITestNGMethod getTestMethod() { + return Objects.requireNonNull(super.getTestMethod()); + } + + /** Always present, for the same reason as {@link #getTestMethod()}. */ + @Override + public Object getInstance() { + return Objects.requireNonNull(super.getInstance()); + } + public static class Builder { - private Object instance; - private ITestNGMethod tm; - private Object[] parameterValues; + private @Nullable Object instance; + private @Nullable ITestNGMethod tm; + private Object @Nullable [] parameterValues; private int parametersIndex; - private Map params; - private ITestClass testClass; - private ITestNGMethod[] beforeMethods; - private ITestNGMethod[] afterMethods; - private ConfigurationGroupMethods groupMethods; + private @Nullable Map params; + private @Nullable ITestClass testClass; + private ITestNGMethod @Nullable [] beforeMethods; + private ITestNGMethod @Nullable [] afterMethods; + private @Nullable ConfigurationGroupMethods groupMethods; public Builder usingInstance(Object instance) { this.instance = instance; @@ -73,7 +91,7 @@ public Builder forTestMethod(ITestNGMethod tm) { return this; } - public Builder withParameterValues(Object[] parameterValues) { + public Builder withParameterValues(Object @Nullable [] parameterValues) { this.parameterValues = parameterValues; return this; } @@ -126,11 +144,11 @@ public TestMethodArguments build() { tm, parameterValues, parametersIndex, - params, - testClass, - beforeMethods, - afterMethods, - groupMethods); + Objects.requireNonNull(params), + Objects.requireNonNull(testClass), + Objects.requireNonNull(beforeMethods), + Objects.requireNonNull(afterMethods), + Objects.requireNonNull(groupMethods)); } } } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java b/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java index 1fe3a96b66..f0e6a89030 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/TestMethodWorker.java @@ -12,7 +12,7 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; -import javax.annotation.Nonnull; +import org.jspecify.annotations.Nullable; import org.testng.ClassMethodMap; import org.testng.IClassListener; import org.testng.IMethodInstance; @@ -49,7 +49,7 @@ public class TestMethodWorker implements IWorker { private final Map m_parameters; private final List m_testResults = new ArrayList<>(); private final ConfigurationGroupMethods m_groupMethods; - private final ClassMethodMap m_classMethodMap; + private final @Nullable ClassMethodMap m_classMethodMap; private final ITestContext m_testContext; private final List m_listeners; private long currentThreadId; @@ -66,7 +66,7 @@ public TestMethodWorker( List testMethods, Map parameters, ConfigurationGroupMethods groupMethods, - ClassMethodMap classMethodMap, + @Nullable ClassMethodMap classMethodMap, ITestContext testContext, List listeners) { this.m_testInvoker = testInvoker; @@ -191,8 +191,9 @@ private boolean canInvokeBeforeClassMethods() { /** Invoke the @BeforeClass methods if not done already */ protected void invokeBeforeClassMethods(ITestClass testClass, IMethodInstance mi) { + // Guarded by canInvokeBeforeClassMethods(). Map> invokedBeforeClassMethods = - m_classMethodMap.getInvokedBeforeClassMethods(); + Objects.requireNonNull(m_classMethodMap).getInvokedBeforeClassMethods(); Set instances = invokedBeforeClassMethods.computeIfAbsent(testClass, key -> ConcurrentHashMap.newKeySet()); Object instance = mi.getInstance(); @@ -274,7 +275,7 @@ private void invokeAfterClassConfigurations( * when the method is not identity-aware (in which case the per-instance config lookup yields * none). */ - private static UUID instanceIdOf(IMethodInstance mi) { + private static @Nullable UUID instanceIdOf(IMethodInstance mi) { ITestNGMethod method = mi.getMethod(); return method instanceof IInstanceIdentity ? ((IInstanceIdentity) method).getInstanceId() @@ -285,7 +286,7 @@ private static UUID instanceIdOf(IMethodInstance mi) { * @return - The throwable raised while lazily constructing the instance this method is bound to, * or {@code null} if there is none (eager instance, successful construction, or no factory). */ - private static Throwable lazyInstantiationFailure(IMethodInstance mi) { + private static @Nullable Throwable lazyInstantiationFailure(IMethodInstance mi) { // A lazy-instantiation detail, so it is read from the internal factory metadata rather than // from the public IFactoryInstance the method hands out. ITestNGMethod method = mi.getMethod(); @@ -327,7 +328,7 @@ public List getTasks() { } @Override - public int compareTo(@Nonnull IWorker other) { + public int compareTo(IWorker other) { if (m_methodInstances.isEmpty()) { return 0; } diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestNgMethodUtils.java b/testng-core/src/main/java/org/testng/internal/invokers/TestNgMethodUtils.java index 8f4fa3dfa5..83ef2535a8 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestNgMethodUtils.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/TestNgMethodUtils.java @@ -8,6 +8,7 @@ import java.util.Optional; import java.util.function.BiPredicate; import java.util.function.Predicate; +import org.jspecify.annotations.Nullable; import org.testng.IClass; import org.testng.ITestClass; import org.testng.ITestNGMethod; @@ -84,8 +85,8 @@ static ITestNGMethod[] filterAfterTestMethods( /** @return Only the ITestNGMethods applicable for this testClass */ static ITestNGMethod[] filterMethods( - Object instance, - IClass testClass, + @Nullable Object instance, + @Nullable IClass testClass, ITestNGMethod[] methods, BiPredicate predicate) { List vResult = new ArrayList<>(); @@ -104,7 +105,7 @@ static ITestNGMethod[] filterMethods( return vResult.toArray(new ITestNGMethod[0]); } - private static boolean isSameInstance(ITestNGMethod tm, Object instance) { + private static boolean isSameInstance(ITestNGMethod tm, @Nullable Object instance) { if (instance == null) { return true; } diff --git a/testng-runner-api/src/main/java/org/testng/internal/invokers/package-info.java b/testng-runner-api/src/main/java/org/testng/internal/invokers/package-info.java new file mode 100644 index 0000000000..87c672b214 --- /dev/null +++ b/testng-runner-api/src/main/java/org/testng/internal/invokers/package-info.java @@ -0,0 +1,5 @@ +/** Runs test and configuration methods: argument assembly, timeouts, listeners and results. */ +@NullMarked +package org.testng.internal.invokers; + +import org.jspecify.annotations.NullMarked;