Skip to content

refactor: declare org.testng.internal.invokers null-marked #3381

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.

Sign up for GitHub

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

Merged
krmahadevan merged 1 commit into from
Aug 17, 2026
Merged

refactor: declare org.testng.internal.invokers null-marked #3381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
70 changes: 47 additions & 23 deletions testng-core/src/main/java/org/testng/internal/invokers/AbstractParallelWorker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -26,16 +28,29 @@ public static AbstractParallelWorker newWorker(
public abstract List<IWorker<ITestNGMethod>> createWorkers(Arguments arguments);

public static class Arguments {
private List<ITestNGMethod> methods;
private IInvoker invoker;
private ConfigurationGroupMethods configMethods;
private ClassMethodMap classMethodMap;
private List<IClassListener> listeners;
private ITestContext testContext;
private IAnnotationFinder finder;

private Arguments() {
// We have a builder. Defeat instantiation via constructors.
private final List<ITestNGMethod> methods;
private final IInvoker invoker;
private final ConfigurationGroupMethods configMethods;
private final ClassMethodMap classMethodMap;
private final List<IClassListener> listeners;
private final ITestContext testContext;
private final IAnnotationFinder finder;

private Arguments(
List<ITestNGMethod> methods,
IInvoker invoker,
ConfigurationGroupMethods configMethods,
ClassMethodMap classMethodMap,
List<IClassListener> 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<ITestNGMethod> getMethods() {
Expand Down Expand Up @@ -67,49 +82,58 @@ public IAnnotationFinder getFinder() {
}

public static class Builder {
private final Arguments instance;

public Builder() {
instance = new Arguments();
}
private @Nullable List<ITestNGMethod> methods;
private @Nullable IInvoker invoker;
private @Nullable ConfigurationGroupMethods configMethods;
private @Nullable ClassMethodMap classMethodMap;
private @Nullable List<IClassListener> listeners;
private @Nullable ITestContext testContext;
private @Nullable IAnnotationFinder finder;

public Builder methods(List<ITestNGMethod> 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<IClassListener> 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));
}
}
}
Expand Down
12 changes: 7 additions & 5 deletions testng-core/src/main/java/org/testng/internal/invokers/Arguments.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<String, String> params;

protected Arguments(Object instance, ITestNGMethod tm, Map<String, String> params) {
protected Arguments(
@Nullable Object instance, @Nullable ITestNGMethod tm, Map<String, String> 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;
}

Expand Down
8 changes: 6 additions & 2 deletions testng-core/src/main/java/org/testng/internal/invokers/ClassBasedParallelWorker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -54,17 +55,20 @@ public List<IWorker<ITestNGMethod>> createWorkers(Arguments arguments) {
params = getParameters(im);
prevClass = c;
}
// prevClass starts out null, so the first iteration always takes the branch above.
Map<String, String> 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);
}
}
Expand Down
73 changes: 44 additions & 29 deletions testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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)) {
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -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(
Expand All @@ -462,32 +472,33 @@ 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);
testResult.setStatus(ITestResult.SKIP);
runConfigurationListeners(testResult, currentTestMethod, false /* after */);
}

private boolean hasConfigFailure(ITestNGMethod currentTestMethod) {
private boolean hasConfigFailure(@Nullable ITestNGMethod currentTestMethod) {
return currentTestMethod != null && m_methodInvocationResults.containsKey(currentTestMethod);
}

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(
Expand Down Expand Up @@ -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 -> {
Expand All @@ -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;
}
Expand All @@ -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<Object> 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<Object> instances = m_classInvocationResults.computeIfAbsent(clazz, k -> new HashSet<>());
Object objectToAdd = instance == null ? NULL_OBJECT : instance;
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Loading
Loading