From 59a208e2c91765ec4e82dc8637e8bf90cbf314a9 Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Wed, 19 Aug 2026 13:43:28 +0200 Subject: [PATCH 01/11] refactor(internal): resolve the ripple outside testng-core Marking org.testng.internal reaches four modules, and the packages that were already marked answer for what they receive from it. XmlPackage no longer hands a null package name to PackageUtils: an unnamed tag is now reported the way an unreadable one already was, instead of raising a NullPointerException from inside findClassesInPackage. The other three are checker-visibility fixes with no behaviour change: defaultIfStringEmpty inlines its predicate because a nullness test hidden behind a call does not refine the argument on the other branch, XmlWeaver tests the class directly instead of through a boolean local, and getSkipCausedBy reads the method and the context into locals it already had in hand. --- .../main/java/org/testng/internal/Utils.java | 4 +++- .../main/java/org/testng/xml/XmlPackage.java | 9 ++++++++- .../src/main/java/org/testng/xml/XmlWeaver.java | 3 +-- .../annotations/JDK15AnnotationFinder.java | 2 +- .../java/org/testng/internal/TestResult.java | 17 +++++++++-------- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/testng-core-api/src/main/java/org/testng/internal/Utils.java b/testng-core-api/src/main/java/org/testng/internal/Utils.java index 89883e427..f3d2d3e04 100644 --- a/testng-core-api/src/main/java/org/testng/internal/Utils.java +++ b/testng-core-api/src/main/java/org/testng/internal/Utils.java @@ -286,7 +286,9 @@ public static void writeResourceToFile(File file, String resourceName, Class } public static String defaultIfStringEmpty(@Nullable String s, String defaultValue) { - return isStringEmpty(s) ? defaultValue : s; + // Inlined rather than delegated to isStringEmpty: a nullness check hidden behind a call does + // not refine s on the other branch. + return s == null || s.isEmpty() ? defaultValue : s; } public static boolean isStringBlank(@Nullable String s) { diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlPackage.java b/testng-core-api/src/main/java/org/testng/xml/XmlPackage.java index 6bc633efa..a2851eb4f 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlPackage.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlPackage.java @@ -65,8 +65,15 @@ public List getXmlClasses() { private List initializeXmlClasses() { List result = new ArrayList<>(); + String name = m_name; + if (name == null) { + // A tag carrying no name attribute. Reported the same way as an unreadable + // package below, rather than through the NullPointerException this used to raise. + Utils.log("XmlPackage", 1, "Ignoring a tag that carries no name."); + return result; + } try { - String[] classes = PackageUtils.findClassesInPackage(m_name, m_include, m_exclude); + String[] classes = PackageUtils.findClassesInPackage(name, m_include, m_exclude); int index = 0; for (String className : classes) { diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlWeaver.java b/testng-core-api/src/main/java/org/testng/xml/XmlWeaver.java index 7157853c2..7fbaa35c7 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlWeaver.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlWeaver.java @@ -36,8 +36,7 @@ private static IWeaveXml instantiateIfRequired() { return instance; } Class clazz = ClassHelper.forName(getClassName()); - boolean isValid = clazz != null && IWeaveXml.class.isAssignableFrom(clazz); - if (!isValid) { + if (clazz == null || !IWeaveXml.class.isAssignableFrom(clazz)) { String msg = "In order for " + getClassName() diff --git a/testng-core/src/main/java/org/testng/internal/annotations/JDK15AnnotationFinder.java b/testng-core/src/main/java/org/testng/internal/annotations/JDK15AnnotationFinder.java index ad612e966..3ce9c8ce1 100644 --- a/testng-core/src/main/java/org/testng/internal/annotations/JDK15AnnotationFinder.java +++ b/testng-core/src/main/java/org/testng/internal/annotations/JDK15AnnotationFinder.java @@ -132,7 +132,7 @@ public JDK15AnnotationFinder(IAnnotationTransformer transformer) { throw new IllegalArgumentException( "Java @Annotation class for '" + annotationClass + "' not found."); } - Method m = tm.getConstructorOrMethod().getMethod(); + Method m = tm.getConstructorOrMethod().requireMethod(); Class testClass = m.getDeclaringClass(); if (tm instanceof BaseTestMethod && !((BaseTestMethod) tm).isInstanceInstantiated()) { // Lazy @Factory instance not created yet: a constructor factory produces exactly its diff --git a/testng-runner-api/src/main/java/org/testng/internal/TestResult.java b/testng-runner-api/src/main/java/org/testng/internal/TestResult.java index ab599161b..617ef97fb 100644 --- a/testng-runner-api/src/main/java/org/testng/internal/TestResult.java +++ b/testng-runner-api/src/main/java/org/testng/internal/TestResult.java @@ -427,16 +427,17 @@ public List getSkipCausedBy() { return Collections.unmodifiableList(skippedDueTo); } // Looks like we didn't have any configuration failures. So some upstream method perhaps failed. - if (requireMethod().getMethodsDependedUpon().length == 0) { + ITestNGMethod skippedMethod = requireMethod(); + if (skippedMethod.getMethodsDependedUpon().length == 0) { // Maybe group dependencies exist ? - if (m_method.getGroupsDependedUpon().length == 0) { + if (skippedMethod.getGroupsDependedUpon().length == 0) { return Collections.emptyList(); } - List upstreamGroups = Arrays.asList(m_method.getGroupsDependedUpon()); + List upstreamGroups = Arrays.asList(skippedMethod.getGroupsDependedUpon()); List allFailures = Lists.merge( - m_context.getFailedTests().getAllResults(), - m_context.getFailedButWithinSuccessPercentageTests().getAllResults()); + context.getFailedTests().getAllResults(), + context.getFailedButWithinSuccessPercentageTests().getAllResults()); skippedDueTo = allFailures.stream() .map(ITestResult::getMethod) @@ -451,13 +452,13 @@ public List getSkipCausedBy() { return Collections.unmodifiableList(skippedDueTo); } - List upstreamMethods = Arrays.asList(requireMethod().getMethodsDependedUpon()); + List upstreamMethods = Arrays.asList(skippedMethod.getMethodsDependedUpon()); // So we have dependsOnMethod failures List allFailures = Lists.merge( - m_context.getFailedTests().getAllResults(), - m_context.getFailedButWithinSuccessPercentageTests().getAllResults()); + context.getFailedTests().getAllResults(), + context.getFailedButWithinSuccessPercentageTests().getAllResults()); skippedDueTo = allFailures.stream() .map(ITestResult::getMethod) From 72dd5954af322913d8e57270866734b4aa84efc8 Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Wed, 19 Aug 2026 13:50:45 +0200 Subject: [PATCH 02/11] refactor(internal): state the nullness of the ITestNGMethod implementations BaseTestMethod.m_instance was declared non-null while its own getInstanceId() answered through ofNullable(...).orElse(null) and getFactoryParameterInfo() tested it. Saying so resolves the constructors of TestNGMethod, ConfigurationMethod and FactoryMethod in one move, and carries through to getInstanceId() and IInstanceIdentity. Three latent NullPointerExceptions surfaced while writing the contracts down: - TestNGMethod.clone() wrapped getTestClass() in a NoOpTestClass, which dereferences it on the spot. A method cloned before setTestClass has run threw there; it now propagates the absence, which is what ConfigurationMethod.clone() already did. - TestNGMethodFinder wrote null into m_beforeGroups/m_afterGroups, whose declaration says {}. Every configuration method that is not a group one carried a null array past MethodGroupsHelper, which iterates it. The default is used instead, and the guards that anticipated the null in XmlMethodSelector and ConfigurationMethod become residue. - MethodInstance.SORT_BY_INDEX compared two names without either being guaranteed to have one. BaseTestMethod.getTestClass() stays nullable: setTestClass is called late in the lifecycle by code outside TestNG, so findMethodParameters answers an absent test class with the suite and parameters - what XmlTestUtils computes anyway when no tag matches. The setters of the nullable getters are widened with them. Kotlin only synthesises a mutable property when both halves agree; leaving setTestClass, setMissingGroup, setDescription and setXmlTest non-null would turn four properties into read-only ones for every Kotlin caller. --- .../org/testng/internal/BaseTestMethod.java | 47 +++++++++++++------ .../org/testng/internal/ClonedMethod.java | 10 ++-- .../testng/internal/ConfigurationMethod.java | 2 +- .../org/testng/internal/FactoryMethod.java | 27 +++++++---- .../testng/internal/IInstanceIdentity.java | 7 +-- .../java/org/testng/internal/IObject.java | 4 +- .../org/testng/internal/MethodInstance.java | 6 ++- .../org/testng/internal/TestNGMethod.java | 29 ++++++------ .../testng/internal/TestNGMethodFinder.java | 4 +- .../testng/internal/WrappedTestNGMethod.java | 5 +- 10 files changed, 88 insertions(+), 53 deletions(-) diff --git a/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java b/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java index 654c1354a..741fa1d6e 100644 --- a/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java +++ b/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java @@ -90,7 +90,7 @@ public abstract class BaseTestMethod private int m_interceptedPriority; private @Nullable XmlTest m_xmlTest; - private final IObject.IdentifiableObject m_instance; + private final IObject.@Nullable IdentifiableObject m_instance; private final Map m_testMethodToRetryAnalyzer = new ConcurrentHashMap<>(); protected final ITestObjectFactory m_objectFactory; @@ -100,7 +100,7 @@ public BaseTestMethod( String methodName, ConstructorOrMethod com, IAnnotationFinder annotationFinder, - IObject.IdentifiableObject instance) { + IObject.@Nullable IdentifiableObject instance) { m_objectFactory = objectFactory; m_methodClass = com.getDeclaringClass(); m_method = com; @@ -133,7 +133,7 @@ public Class getRealClass() { /** {@inheritDoc} */ @Override - public void setTestClass(ITestClass tc) { + public void setTestClass(@Nullable ITestClass tc) { if (tc == null) { throw new IllegalArgumentException("test class cannot be null"); } @@ -175,7 +175,7 @@ public boolean isInstanceInstantiated() { } @Override - public UUID getInstanceId() { + public @Nullable UUID getInstanceId() { return Optional.ofNullable(m_instance) .map(IObject.IdentifiableObject::getInstanceId) .orElse(null); @@ -187,6 +187,19 @@ public long[] getInstanceHashCodes() { return IObject.instanceHashCodes(m_testClass); } + /** + * The instance wrapper a clone of this method should carry: the same identity, or {@code null} + * when this method carries no instance at all. + */ + protected IObject.@Nullable IdentifiableObject cloneInstance() { + Object instance = getInstance(); + UUID instanceId = getInstanceId(); + if (instance == null || instanceId == null) { + return null; + } + return new IObject.IdentifiableObject(instance, instanceId); + } + /** * {@inheritDoc} * @@ -464,7 +477,7 @@ protected void initGroups(Class annotationClass) } protected void initBeforeAfterGroups( - Class annotationClass, String[] groups) { + Class annotationClass, String @Nullable [] groups) { String[] groupsAtMethodLevel = calculateGroupsToUseConsideringValuesAndGroupValues(annotationClass, groups); // @BeforeGroups and @AfterGroups annotation cannot be used at Class level. So its always null @@ -472,8 +485,8 @@ protected void initBeforeAfterGroups( initRestOfGroupDependencies(annotationClass); } - private String[] calculateGroupsToUseConsideringValuesAndGroupValues( - Class annotationClass, String[] groups) { + private String @Nullable [] calculateGroupsToUseConsideringValuesAndGroupValues( + Class annotationClass, String @Nullable [] groups) { if (groups == null || groups.length == 0) { ITestOrConfiguration annotation = getAnnotationFinder().findAnnotation(getConstructorOrMethod(), annotationClass); @@ -521,7 +534,7 @@ private void initRestOfGroupDependencies(Class a setMethodsDependedUpon(methodsDependedUpon); } - private static Map> calculateXmlGroupDependencies(XmlTest xmlTest) { + private static Map> calculateXmlGroupDependencies(@Nullable XmlTest xmlTest) { Map> result = new HashMap<>(); if (xmlTest == null) { return result; @@ -654,7 +667,7 @@ public void addMethodDependedUpon(String method) { /** {@inheritDoc} */ @Override - public void setMissingGroup(String group) { + public void setMissingGroup(@Nullable String group) { m_missingGroup = group; } @@ -669,7 +682,7 @@ public int getThreadPoolSize() { public void setThreadPoolSize(int threadPoolSize) {} @Override - public void setDescription(String description) { + public void setDescription(@Nullable String description) { m_description = description; } @@ -847,7 +860,7 @@ public void setInterceptedPriority(int priority) { return m_xmlTest; } - public void setXmlTest(XmlTest xmlTest) { + public void setXmlTest(@Nullable XmlTest xmlTest) { m_xmlTest = xmlTest; } @@ -863,7 +876,13 @@ public Class[] getParameterTypes() { @Override public Map findMethodParameters(XmlTest test) { - return XmlTestUtils.findMethodParameters(test, getTestClass().getName(), getMethodName()); + ITestClass testClass = getTestClass(); + if (testClass == null) { + // No test class bound yet. No tag can match, so XmlTestUtils would return the + // suite and parameters unchanged - which getAllParameters already builds fresh. + return test.getAllParameters(); + } + return XmlTestUtils.findMethodParameters(test, testClass.getName(), getMethodName()); } @Override @@ -873,7 +892,7 @@ public String getQualifiedName() { @Override @Deprecated - public IParameterInfo getFactoryMethodParamsInfo() { + public @Nullable IParameterInfo getFactoryMethodParamsInfo() { return getFactoryParameterInfo(); } @@ -916,7 +935,7 @@ private static boolean isNotParameterisedTest(ITestResult tr) { return Optional.ofNullable(tr.getParameters()).orElse(new Object[0]).length == 0; } - private IRetryAnalyzer computeRetryAnalyzerInstanceToUse(ITestResult tr) { + private @Nullable IRetryAnalyzer computeRetryAnalyzerInstanceToUse(ITestResult tr) { if (m_retryAnalyzer != null) { return m_retryAnalyzer; } diff --git a/testng-core/src/main/java/org/testng/internal/ClonedMethod.java b/testng-core/src/main/java/org/testng/internal/ClonedMethod.java index b92a13cda..3d45aceaa 100644 --- a/testng-core/src/main/java/org/testng/internal/ClonedMethod.java +++ b/testng-core/src/main/java/org/testng/internal/ClonedMethod.java @@ -5,6 +5,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.Callable; import org.jspecify.annotations.Nullable; @@ -127,7 +128,7 @@ public String[] getMethodsDependedUpon() { } @Override - public String getMissingGroup() { + public @Nullable String getMissingGroup() { return null; } @@ -302,7 +303,10 @@ public ClonedMethod clone() { @Override public String toString() { - ConstructorOrMethod m = getConstructorOrMethod(); + // getConstructorOrMethod() answers null, so this has always thrown. Stated rather than hidden. + ConstructorOrMethod m = + Objects.requireNonNull( + getConstructorOrMethod(), "a ClonedMethod has no ConstructorOrMethod"); String cls = m.getDeclaringClass().getName(); return BaseTestMethod.stringify(cls, m).toString(); } @@ -353,7 +357,7 @@ public XmlTest getXmlTest() { } @Override - public ConstructorOrMethod getConstructorOrMethod() { + public @Nullable ConstructorOrMethod getConstructorOrMethod() { return null; } diff --git a/testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java b/testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java index 5905710e5..bac5e791f 100644 --- a/testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java +++ b/testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java @@ -492,7 +492,7 @@ public ConfigurationMethod clone() { getBeforeGroups(), getAfterGroups(), false /* do not call init() */, - new IObject.IdentifiableObject(getInstance(), getInstanceId())); + cloneInstance()); clone.m_testClass = getTestClass(); clone.setDate(getDate()); clone.setGroups(getGroups()); diff --git a/testng-core/src/main/java/org/testng/internal/FactoryMethod.java b/testng-core/src/main/java/org/testng/internal/FactoryMethod.java index 542aef83a..73d0b8b9b 100644 --- a/testng-core/src/main/java/org/testng/internal/FactoryMethod.java +++ b/testng-core/src/main/java/org/testng/internal/FactoryMethod.java @@ -27,22 +27,25 @@ import org.testng.internal.annotations.IAnnotationFinder; import org.testng.internal.invokers.ParameterHolder; import org.testng.xml.XmlTest; +import org.jspecify.annotations.Nullable; +import java.util.Objects; /** This class represents a method annotated with @Factory */ public class FactoryMethod extends BaseTestMethod { - private final IFactoryAnnotation factoryAnnotation; - private final Object m_instance; + private final @Nullable IFactoryAnnotation factoryAnnotation; + private final @Nullable Object m_instance; private final ITestContext m_testContext; - private String m_factoryCreationFailedMessage = null; + private @Nullable String m_factoryCreationFailedMessage = null; private final DataProviderHolder holder; private final boolean m_lazy; - public String getFactoryCreationFailedMessage() { + public @Nullable String getFactoryCreationFailedMessage() { return m_factoryCreationFailedMessage; } - private void init(Object instance, IAnnotationFinder annotationFinder, ConstructorOrMethod com) { + private void init( + @Nullable Object instance, IAnnotationFinder annotationFinder, ConstructorOrMethod com) { IListenersAnnotation annotation = annotationFinder.findAnnotation(com.getDeclaringClass(), IListenersAnnotation.class); if (annotation == null) { @@ -89,10 +92,14 @@ private void init(Object instance, IAnnotationFinder annotationFinder, Construct Utils.checkReturnType(com.getMethod(), Object[].class, IInstanceInfo[].class); Class declaringClass = com.getDeclaringClass(); if (instance != null && !declaringClass.isAssignableFrom(instance.getClass())) { - if (instance instanceof IParameterInfo) { - instance = ((IParameterInfo) instance).getInstance(); + Object mismatched = instance; + if (mismatched instanceof IParameterInfo) { + Object embedded = ((IParameterInfo) mismatched).getInstance(); + if (embedded != null) { + mismatched = embedded; + } } - Class cls = instance.getClass(); + Class cls = mismatched.getClass(); String msg = "Found a default constructor and also a Factory method when working with " + declaringClass.getName() @@ -208,7 +215,9 @@ public IParameterInfo[] invoke() { Iterator parameterIterator = parameterHolder.parameters; try { - List indices = factoryAnnotation.getIndices(); + List indices = + Objects.requireNonNull(factoryAnnotation, "no @Factory annotation on a factory method") + .getIndices(); int position = 0; IFactory factory = new FactoryDescriptor(getConstructorOrMethod(), m_lazy); while (parameterIterator.hasNext()) { diff --git a/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java b/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java index 9b9efe494..dd12affae 100644 --- a/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java +++ b/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java @@ -3,16 +3,17 @@ import java.util.Arrays; import java.util.Objects; import java.util.UUID; +import org.jspecify.annotations.Nullable; public interface IInstanceIdentity { /** * @return - A {@link UUID} that represents a unique id which is associated with - * every test class object. + * every test class object, or {@code null} when the implementation carries no instance. */ - UUID getInstanceId(); + @Nullable UUID getInstanceId(); - static Object getInstanceId(Object object) { + static @Nullable Object getInstanceId(Object object) { if (object instanceof IInstanceIdentity) { return ((IInstanceIdentity) object).getInstanceId(); } diff --git a/testng-core/src/main/java/org/testng/internal/IObject.java b/testng-core/src/main/java/org/testng/internal/IObject.java index 15cab568a..8cd28380f 100644 --- a/testng-core/src/main/java/org/testng/internal/IObject.java +++ b/testng-core/src/main/java/org/testng/internal/IObject.java @@ -32,7 +32,7 @@ public interface IObject { * @param object - The object that should be inspected for its compatibility with {@link IObject}. * @return - An array representing the hash codes of the corresponding instances. */ - static long[] instanceHashCodes(Object object) { + static long[] instanceHashCodes(@Nullable Object object) { return cast(object).map(IObject::getInstanceHashCodes).orElse(new long[] {}); } @@ -65,7 +65,7 @@ static IdentifiableObject[] objects(Object object, boolean create, String errorM * @return - If the incoming object is an instance of {@link IObject} then the cast instance is * wrapped within {@link Optional} else it would be an {@link Optional#empty()} */ - static Optional cast(Object object) { + static Optional cast(@Nullable Object object) { if (object instanceof IObject) { return Optional.of((IObject) object); } diff --git a/testng-core/src/main/java/org/testng/internal/MethodInstance.java b/testng-core/src/main/java/org/testng/internal/MethodInstance.java index 34ef65534..b339085a7 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodInstance.java +++ b/testng-core/src/main/java/org/testng/internal/MethodInstance.java @@ -8,6 +8,7 @@ import org.testng.xml.XmlClass; import org.testng.xml.XmlInclude; import org.testng.xml.XmlTest; +import org.jspecify.annotations.Nullable; public class MethodInstance implements IMethodInstance { private final ITestNGMethod m_method; @@ -43,7 +44,7 @@ public int compare(IMethodInstance o1, IMethodInstance o2) { XmlTest test2 = o2.getMethod().getTestClass().getXmlTest(); // If the two methods are not in the same , we can't compare them - if (!test1.getName().equals(test2.getName())) { + if (!java.util.Objects.equals(test1.getName(), test2.getName())) { return 0; } @@ -83,7 +84,8 @@ public int compare(IMethodInstance o1, IMethodInstance o2) { return result; } - private XmlInclude findXmlInclude(List includedMethods, String methodName) { + private @Nullable XmlInclude findXmlInclude( + List includedMethods, String methodName) { for (XmlInclude xi : includedMethods) { if (xi.getName().equals(methodName)) { return xi; diff --git a/testng-core/src/main/java/org/testng/internal/TestNGMethod.java b/testng-core/src/main/java/org/testng/internal/TestNGMethod.java index 7a6d2690d..547533a8e 100644 --- a/testng-core/src/main/java/org/testng/internal/TestNGMethod.java +++ b/testng-core/src/main/java/org/testng/internal/TestNGMethod.java @@ -35,22 +35,18 @@ public TestNGMethod( IAnnotationFinder finder, XmlTest xmlTest, IObject.@Nullable IdentifiableObject instance) { - this(objectFactory, method, finder, true, xmlTest, instance); + this(objectFactory, method, finder, instance); + init(xmlTest); } + /** Builds the method without initialising it from an {@link XmlTest}; {@link #clone()} copies + * the state across itself. */ private TestNGMethod( ITestObjectFactory objectFactory, Method method, IAnnotationFinder finder, - boolean initialize, - XmlTest xmlTest, IObject.@Nullable IdentifiableObject instance) { super(objectFactory, method.getName(), new ConstructorOrMethod(method), finder, instance); - setXmlTest(xmlTest); - - if (initialize) { - init(xmlTest); - } } /** {@inheritDoc} */ @@ -176,14 +172,17 @@ public BaseTestMethod clone() { m_objectFactory, getConstructorOrMethod().requireMethod(), getAnnotationFinder(), - false, - getXmlTest(), - new IObject.IdentifiableObject(getInstance(), getInstanceId())); + cloneInstance()); + clone.setXmlTest(getXmlTest()); ITestClass tc = getTestClass(); - NoOpTestClass testClass = new NoOpTestClass(tc); - testClass.setBeforeTestMethods(clone(tc.getBeforeTestMethods())); - testClass.setAfterTestMethod(clone(tc.getAfterTestMethods())); - clone.m_testClass = testClass; + if (tc != null) { + // Wrapping a test class this method has not been bound to yet would have thrown here. + // ConfigurationMethod.clone() already propagates the absence rather than wrapping it. + NoOpTestClass testClass = new NoOpTestClass(tc); + testClass.setBeforeTestMethods(clone(tc.getBeforeTestMethods())); + testClass.setAfterTestMethod(clone(tc.getAfterTestMethods())); + clone.m_testClass = testClass; + } clone.setDate(getDate()); clone.setGroups(getGroups()); clone.setGroupsDependedUpon(getGroupsDependedUpon(), Collections.emptyList()); diff --git a/testng-core/src/main/java/org/testng/internal/TestNGMethodFinder.java b/testng-core/src/main/java/org/testng/internal/TestNGMethodFinder.java index 5cb2f4604..42b921521 100644 --- a/testng-core/src/main/java/org/testng/internal/TestNGMethodFinder.java +++ b/testng-core/src/main/java/org/testng/internal/TestNGMethodFinder.java @@ -209,8 +209,8 @@ private ITestNGMethod[] findConfiguration( isBeforeTestMethod, isAfterTestMethod, ignoreFailure, - beforeGroups, - afterGroups); /* @@@ */ + beforeGroups == null ? new String[0] : beforeGroups, + afterGroups == null ? new String[0] : afterGroups); /* @@@ */ } } diff --git a/testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java b/testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java index 0a2295a17..5545dd615 100644 --- a/testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java +++ b/testng-core/src/main/java/org/testng/internal/WrappedTestNGMethod.java @@ -13,6 +13,7 @@ import org.testng.ITestNGMethod; import org.testng.ITestResult; import org.testng.xml.XmlTest; +import org.jspecify.annotations.Nullable; /** * Represents a proxy for an actual instance of {@link ITestNGMethod} but with the exception that it @@ -23,7 +24,7 @@ public class WrappedTestNGMethod implements ITestNGMethod, IInstanceIdentity { private final ITestNGMethod testNGMethod; private final int multiplicationFactor = new Random().nextInt(); - private final UUID uuid; + private final @Nullable UUID uuid; public WrappedTestNGMethod(ITestNGMethod testNGMethod) { this.testNGMethod = testNGMethod; @@ -379,7 +380,7 @@ public String getQualifiedName() { } @Override - public UUID getInstanceId() { + public @Nullable UUID getInstanceId() { return uuid; } From 95544d5fdac33f3418a78887dd469ea8de5bc2b3 Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Wed, 19 Aug 2026 13:55:13 +0200 Subject: [PATCH 03/11] refactor(internal): state the nullness of the parameter assembly Parameters carried the largest share of the diagnostics, behind seven causes rather than twenty-six sites. MethodParameters.context and .testResult are absent on the constructor injection path, which SimpleObjectDispenser reaches before any test context exists. The callees that store the null answer for it - ReflectionRecipes.inject already tested the context internally, MethodMatcherContext was handed a literal null, and CreationAttributes held it in an already nullable field. The one that dereferences it, invokeDataProvider, is given a context asserted at the boundary instead. ConstructorOrMethod gains requireConstructor(), the twin of requireMethod(): the three sites that reach for the constructor have already established the wrapper holds one, and widening IAnnotationFinder.findOptionalValues or ReflectionRecipes.getConstructorParameters to say otherwise would loosen contracts that third parties implement. Two facts had been recorded in a second variable and were lost on the way: the retry analyzer's existence lived in shouldRetry, and the data provider iterator's in thrownException. Both are now read from the value itself. The retry analyzer also gets a message: the dispenser can answer null, and the NullPointerException now names what could not be created. The remaining sites are private helpers whose callers already tested their result, and a nullness test that NullAway cannot follow through a boolean local. --- .../testng/internal/ConstructorOrMethod.java | 19 +++++ .../testng/internal/DataProviderMethod.java | 3 +- .../internal/DataProviderMethodRemovable.java | 3 +- .../java/org/testng/internal/Parameters.java | 70 ++++++++++++------- .../invokers/MethodInvocationHelper.java | 4 +- .../objects/pojo/CreationAttributes.java | 2 +- .../reflect/MethodMatcherContext.java | 7 +- .../internal/reflect/ReflectionRecipes.java | 12 ++-- 8 files changed, 79 insertions(+), 41 deletions(-) diff --git a/testng-core-api/src/main/java/org/testng/internal/ConstructorOrMethod.java b/testng-core-api/src/main/java/org/testng/internal/ConstructorOrMethod.java index b599905df..9c9c819f5 100644 --- a/testng-core-api/src/main/java/org/testng/internal/ConstructorOrMethod.java +++ b/testng-core-api/src/main/java/org/testng/internal/ConstructorOrMethod.java @@ -49,6 +49,10 @@ public Class[] getParameterTypes() { return member instanceof Method ? (Method) member : null; } + /** + * @return the wrapped member if it is a constructor, or {@code null} if it is a method. Prefer + * {@link #requireConstructor()} unless the null is what you are testing for. + */ public @Nullable Constructor getConstructor() { return member instanceof Constructor ? (Constructor) member : null; } @@ -68,6 +72,21 @@ public Method requireMethod() { throw new NullPointerException("Expected a method, but " + member + " is a constructor"); } + /** + * The wrapped member as a {@link Constructor}, for the callers that have already established it + * is not a method. + * + * @return the wrapped constructor + * @throws NullPointerException if this wrapper holds a method -- the same failure the call sites + * saw before, with a message instead of a bare dereference + */ + public Constructor requireConstructor() { + if (member instanceof Constructor) { + return (Constructor) member; + } + throw new NullPointerException("Expected a constructor, but " + member + " is a method"); + } + /** * Makes the wrapped member accessible. When interning is on the handle is shared, so this is * observable through every wrapper of the same member. diff --git a/testng-core/src/main/java/org/testng/internal/DataProviderMethod.java b/testng-core/src/main/java/org/testng/internal/DataProviderMethod.java index da5aff480..d4101257e 100644 --- a/testng-core/src/main/java/org/testng/internal/DataProviderMethod.java +++ b/testng-core/src/main/java/org/testng/internal/DataProviderMethod.java @@ -14,7 +14,8 @@ class DataProviderMethod implements IDataProviderMethod { protected @Nullable Method method; private final IDataProviderAnnotation annotation; - DataProviderMethod(Object instance, Method method, IDataProviderAnnotation annotation) { + DataProviderMethod( + @Nullable Object instance, Method method, IDataProviderAnnotation annotation) { this.instance = instance; this.method = method; this.annotation = annotation; diff --git a/testng-core/src/main/java/org/testng/internal/DataProviderMethodRemovable.java b/testng-core/src/main/java/org/testng/internal/DataProviderMethodRemovable.java index 8faad2347..68d9bce01 100644 --- a/testng-core/src/main/java/org/testng/internal/DataProviderMethodRemovable.java +++ b/testng-core/src/main/java/org/testng/internal/DataProviderMethodRemovable.java @@ -7,7 +7,8 @@ /** Represents an @{@link org.testng.annotations.DataProvider} annotated method. */ class DataProviderMethodRemovable extends DataProviderMethod { - DataProviderMethodRemovable(Object instance, Method method, IDataProviderAnnotation annotation) { + DataProviderMethodRemovable( + @Nullable Object instance, Method method, IDataProviderAnnotation annotation) { super(instance, method, annotation); } diff --git a/testng-core/src/main/java/org/testng/internal/Parameters.java b/testng-core/src/main/java/org/testng/internal/Parameters.java index fb119cbfb..c180c6999 100644 --- a/testng-core/src/main/java/org/testng/internal/Parameters.java +++ b/testng-core/src/main/java/org/testng/internal/Parameters.java @@ -60,6 +60,7 @@ import org.testng.util.Strings; import org.testng.xml.XmlSuite; import org.testng.xml.XmlTest; +import java.util.Objects; /** Methods that bind parameters declared in testng.xml to actual values used to invoke methods. */ public class Parameters { @@ -190,7 +191,7 @@ public static Object[] createConfigurationParameters( name); } - private static Class retrieveConfigAnnotation(Method m) { + private static @Nullable Class retrieveConfigAnnotation(Method m) { return annotationList.stream() .filter(annotation -> m.getAnnotation(annotation) != null) .findAny() @@ -402,7 +403,7 @@ private static Parameter[] extractParameters(ConstructorOrMethod method) { if (method.getMethod() != null) { return ReflectionRecipes.getMethodParameters(method.getMethod()); } - return ReflectionRecipes.getConstructorParameters(method.getConstructor()); + return ReflectionRecipes.getConstructorParameters(method.requireConstructor()); } private static boolean canInject(String annotation) { @@ -509,13 +510,13 @@ private static String prettyFormat(List> classes) { return builder.toString(); } - private static IDataProviderMethod findDataProvider( + private static @Nullable IDataProviderMethod findDataProvider( ITestObjectFactory objectFactory, - Object instance, + @Nullable Object instance, ITestClass clazz, ConstructorOrMethod m, IAnnotationFinder finder, - ITestContext context) { + @Nullable ITestContext context) { IDataProviderMethod result = null; IDataProvidable dp = findDataProviderInfo(clazz, m, finder); @@ -562,11 +563,11 @@ private static IDataProviderMethod findDataProvider( * Find the data provider info (data provider name and class) on either @Test(dataProvider), * @Factory(dataProvider) on a method or @Factory(dataProvider) on a constructor. */ - private static IDataProvidable findDataProviderInfo( + private static @Nullable IDataProvidable findDataProviderInfo( ITestClass clazz, ConstructorOrMethod m, IAnnotationFinder finder) { if (m.getMethod() == null) { // @Factory(dataProvider) on a constructor - return AnnotationHelper.findFactory(finder, m.getConstructor()); + return AnnotationHelper.findFactory(finder, m.requireConstructor()); } // @Test(dataProvider) on a method @@ -629,15 +630,15 @@ private static boolean isDataProviderNameEmpty(ITestAnnotation annotation) { } /** Find a method that has a @DataProvider(name=name) */ - private static IDataProviderMethod findDataProvider( + private static @Nullable IDataProviderMethod findDataProvider( ITestObjectFactory objectFactory, - Object instance, + @Nullable Object instance, ITestClass clazz, IAnnotationFinder finder, String name, - Class dataProviderClass, + @Nullable Class dataProviderClass, boolean isDynamicDataProvider, - ITestContext context) { + @Nullable ITestContext context) { IDataProviderMethod result = null; Class cls = clazz.getRealClass(); @@ -650,8 +651,7 @@ private static IDataProviderMethod findDataProvider( for (Method m : ClassHelper.getAvailableMethods(cls)) { IDataProviderAnnotation dp = finder.findAnnotation(m, IDataProviderAnnotation.class); - boolean proceed = null != dp && name.equals(getDataProviderName(dp, m)); - if (!proceed) { + if (dp == null || !name.equals(getDataProviderName(dp, m))) { continue; } Object instanceToUse = instance; @@ -692,7 +692,7 @@ private static String[] extractOptionalValues( if (consMethod.getMethod() != null) { return finder.findOptionalValues(consMethod.getMethod()); } - return finder.findOptionalValues(consMethod.getConstructor()); + return finder.findOptionalValues(consMethod.requireConstructor()); } private static Object[] createParameters( @@ -751,11 +751,11 @@ public static ParameterHolder handleParameters( ITestObjectFactory objectFactory, ITestNGMethod testMethod, Map allParameterNames, - Object instance, + @Nullable Object instance, MethodParameters methodParams, XmlSuite xmlSuite, IAnnotationFinder annotationFinder, - Object fedInstance, + @Nullable Object fedInstance, DataProviderHolder holder) { return handleParameters( objectFactory, @@ -780,11 +780,11 @@ public static ParameterHolder handleParameters( ITestObjectFactory objectFactory, ITestNGMethod testMethod, Map allParameterNames, - Object instance, + @Nullable Object instance, MethodParameters methodParams, XmlSuite xmlSuite, IAnnotationFinder annotationFinder, - Object fedInstance, + @Nullable Object fedInstance, DataProviderHolder holder, String annotationName) { /* @@ -818,7 +818,11 @@ public static ParameterHolder handleParameters( IObjectDispenser dispenser = Dispenser.newInstance(objectFactory); BasicAttributes basic = new BasicAttributes(testMethod.getTestClass(), retryClass); CreationAttributes attributes = new CreationAttributes(methodParams.context, basic, null); - retry = (IRetryDataProvider) dispenser.dispense(attributes); + retry = + (IRetryDataProvider) + Objects.requireNonNull( + dispenser.dispense(attributes), + "could not instantiate the data provider retry analyzer"); } CloseableIterator initParams = null; @@ -837,7 +841,7 @@ public static ParameterHolder handleParameters( .getInstance(), /* a test instance or null if the data provider is static*/ dataProviderMethod.getMethod(), testMethod, - methodParams.context, + methodParams.requireContext(), fedInstance, annotationFinder); shouldRetry = false; @@ -846,7 +850,8 @@ public static ParameterHolder handleParameters( for (IDataProviderListener each : holder.getListeners()) { each.onDataProviderFailure(testMethod, methodParams.context, e); } - if (shouldRetry) { + if (retry != null) { + // Same condition as shouldRetry, which is true here exactly when retry was created. shouldRetry = retry.retry(dataProviderMethod); thrownException = e; } else { @@ -870,7 +875,8 @@ public static ParameterHolder handleParameters( // FilteredParameters and any interceptors, so the resource can be released later - including // if the setup below (listeners / filtering / interceptors) throws before a ParameterHolder // takes ownership of it. - CloseableIterator closeableSource = initParams; + CloseableIterator closeableSource = + Objects.requireNonNull(initParams, "the data provider produced no iterator"); try { for (IDataProviderListener dataProviderListener : holder.getListeners()) { dataProviderListener.afterDataProviderExecution( @@ -884,7 +890,7 @@ public static ParameterHolder handleParameters( Iterator filteredParameters = new FilteredParameters( - initParams, testMethod, dataProviderMethod.getName(), allIndices); + closeableSource, testMethod, dataProviderMethod.getName(), allIndices); testMethod.setMoreInvocationChecker(filteredParameters::hasNext); for (IDataProviderInterceptor interceptor : holder.getInterceptors()) { @@ -1001,15 +1007,25 @@ public MethodParameters( parameterValues = pv; testResult = tr; } + + /** + * The test context, for the callees that dereference it. The two-argument constructor builds + * parameters for a constructor injection, which happens before any context exists. + */ + ITestContext requireContext() { + return Objects.requireNonNull(context, "these method parameters carry no test context"); + } } private static final class ImmutableDataProvidable implements IDataProvidable { private final String dataProvider; - private final Class dataProviderClass; + private final @Nullable Class dataProviderClass; private final String dataProviderDynamicClass; private ImmutableDataProvidable( - String dataProvider, Class dataProviderClass, String dataProviderDynamicClass) { + String dataProvider, + @Nullable Class dataProviderClass, + String dataProviderDynamicClass) { this.dataProvider = dataProvider; this.dataProviderClass = dataProviderClass; this.dataProviderDynamicClass = @@ -1025,12 +1041,12 @@ public String getDataProvider() { public void setDataProvider(String v) {} @Override - public Class getDataProviderClass() { + public @Nullable Class getDataProviderClass() { return dataProviderClass; } @Override - public void setDataProviderClass(Class v) {} + public void setDataProviderClass(@Nullable Class v) {} @Override public String getDataProviderDynamicClass() { 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 dc29ff891..7a778b707 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 @@ -169,7 +169,7 @@ public static CloseableIterator invokeDataProvider( Method dataProvider, ITestNGMethod method, ITestContext testContext, - Object fedInstance, + @Nullable Object fedInstance, IAnnotationFinder annotationFinder) { List parameters = getParameters(dataProvider, method, testContext, fedInstance, annotationFinder); @@ -206,7 +206,7 @@ private static List getParameters( Method dataProvider, ITestNGMethod method, ITestContext testContext, - Object fedInstance, + @Nullable Object fedInstance, IAnnotationFinder annotationFinder) { // Go through all the parameters declared on this Data Provider and // make sure we have at most one Method and one ITestContext. diff --git a/testng-core/src/main/java/org/testng/internal/objects/pojo/CreationAttributes.java b/testng-core/src/main/java/org/testng/internal/objects/pojo/CreationAttributes.java index 38e778b98..1973958bd 100644 --- a/testng-core/src/main/java/org/testng/internal/objects/pojo/CreationAttributes.java +++ b/testng-core/src/main/java/org/testng/internal/objects/pojo/CreationAttributes.java @@ -13,7 +13,7 @@ public class CreationAttributes { private final @Nullable GuiceContext suiteContext; public CreationAttributes( - ITestContext ctx, BasicAttributes basic, @Nullable DetailedAttributes detailed) { + @Nullable ITestContext ctx, BasicAttributes basic, @Nullable DetailedAttributes detailed) { this.basic = basic; this.detailed = detailed; this.context = ctx; diff --git a/testng-core/src/main/java/org/testng/internal/reflect/MethodMatcherContext.java b/testng-core/src/main/java/org/testng/internal/reflect/MethodMatcherContext.java index 05e959ae5..ee7911c19 100644 --- a/testng-core/src/main/java/org/testng/internal/reflect/MethodMatcherContext.java +++ b/testng-core/src/main/java/org/testng/internal/reflect/MethodMatcherContext.java @@ -4,6 +4,7 @@ import java.lang.reflect.Parameter; import org.testng.ITestContext; import org.testng.ITestResult; +import org.jspecify.annotations.Nullable; /** * Input context for MethodMatchers. @@ -15,7 +16,7 @@ public class MethodMatcherContext { private final Parameter[] methodParameter; private final Object[] arguments; private final ITestContext testContext; - private final ITestResult testResult; + private final @Nullable ITestResult testResult; /** * Constructs a context for MethodMatchers. @@ -29,7 +30,7 @@ public MethodMatcherContext( final Method method, final Object[] arguments, final ITestContext testContext, - final ITestResult testResult) { + final @Nullable ITestResult testResult) { this.method = method; this.methodParameter = ReflectionRecipes.getMethodParameters(method); this.arguments = arguments; @@ -53,7 +54,7 @@ public ITestContext getTestContext() { return testContext; } - public ITestResult getTestResult() { + public @Nullable ITestResult getTestResult() { return testResult; } } diff --git a/testng-core/src/main/java/org/testng/internal/reflect/ReflectionRecipes.java b/testng-core/src/main/java/org/testng/internal/reflect/ReflectionRecipes.java index 462643f8d..bcc6bfdf0 100644 --- a/testng-core/src/main/java/org/testng/internal/reflect/ReflectionRecipes.java +++ b/testng-core/src/main/java/org/testng/internal/reflect/ReflectionRecipes.java @@ -341,8 +341,8 @@ public static Object[] inject( final Set filters, final Object[] args, final @Nullable Method injectionMethod, - final ITestContext context, - final ITestResult testResult) { + final @Nullable ITestContext context, + final @Nullable ITestResult testResult) { return nativelyInject(parameters, filters, args, injectionMethod, context, testResult); } @@ -351,8 +351,8 @@ private static Object[] nativelyInject( final Set filters, final Object[] args, final @Nullable Object injectionMethod, - final ITestContext context, - final ITestResult testResult) { + final @Nullable ITestContext context, + final @Nullable ITestResult testResult) { if (filters == null || filters.isEmpty()) { return args; } @@ -445,8 +445,8 @@ public static Object[] inject( final Set filters, final Object[] args, final Constructor constructor, - final ITestContext context, - final ITestResult testResult) { + final @Nullable ITestContext context, + final @Nullable ITestResult testResult) { return nativelyInject(parameters, filters, args, constructor, context, testResult); } From 683af71fb7d18e4280e3458ee377da25a93e49e0 Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Wed, 19 Aug 2026 14:04:40 +0200 Subject: [PATCH 04/11] refactor(internal): state the nullness of the class finders, helpers and graphs The rest of the package: class discovery, method collection, the dependency graphs and the invokers. Two facts had to stop travelling through a field. Graph.m_independentNodes was built lazily and read back off the field afterwards, so any call in between invalidated what the initialiser had just established; initializeIndependentNodes now hands the map back. ConfigurationGroupMethods.m_afterGroupsMap had the same shape, read from two lambdas that ran after the assignment. MultiMap says what it is: a HashMap-backed multimap. A method that carries no instance has no instance id, and both the instance dependency graph and the per-instance workers already grouped those methods under a null key. Keying them anywhere else would change how they are partitioned. The rest are local: private helpers whose callers already tested their result, lookups whose key came from the very map being read, an AtomicReference holding a fact that is a boolean, and the reflective members that are a method because their call site already established it is not a constructor. Where the value must exist for the caller to work at all, it is asserted at the boundary rather than carried further: TestNGClassFinder and ParameterHandler dispense instances through an object factory, so they say so once instead of threading the absence down to SimpleObjectDispenser. --- .../java/org/testng/collections/MultiMap.java | 12 +++++----- .../org/testng/internal/BaseClassFinder.java | 4 ++-- .../java/org/testng/internal/ClassImpl.java | 22 +++++++++++-------- .../internal/ConfigurationGroupMethods.java | 22 ++++++++++++------- .../org/testng/internal/DynamicGraph.java | 5 +++-- .../testng/internal/DynamicGraphHelper.java | 3 ++- .../org/testng/internal/FactoryMethod.java | 2 +- .../main/java/org/testng/internal/Graph.java | 21 ++++++++++-------- .../java/org/testng/internal/IObject.java | 5 +++-- .../testng/internal/ITestClassConfigInfo.java | 5 +++-- .../testng/internal/MethodGroupsHelper.java | 14 ++++++++---- .../org/testng/internal/MethodHelper.java | 20 +++++++++-------- .../testng/internal/MethodInheritance.java | 5 +++-- .../org/testng/internal/MethodSorting.java | 3 ++- .../java/org/testng/internal/Parameters.java | 6 ++--- .../internal/ScriptSelectorFactory.java | 5 ++++- .../main/java/org/testng/internal/Tarjan.java | 9 ++++++-- .../testng/internal/TestListenerHelper.java | 6 ++--- .../testng/internal/TestNGClassFinder.java | 16 +++++++++----- .../internal/invokers/ParameterHandler.java | 3 ++- .../testng/internal/invokers/TestInvoker.java | 4 ++-- 21 files changed, 117 insertions(+), 75 deletions(-) diff --git a/testng-collections/src/main/java/org/testng/collections/MultiMap.java b/testng-collections/src/main/java/org/testng/collections/MultiMap.java index 7216196c9..6bd338493 100644 --- a/testng-collections/src/main/java/org/testng/collections/MultiMap.java +++ b/testng-collections/src/main/java/org/testng/collections/MultiMap.java @@ -22,7 +22,7 @@ protected MultiMap(boolean isSorted) { protected abstract C createValue(); - public boolean put(K key, V method) { + public boolean put(@Nullable K key, V method) { AtomicBoolean exists = new AtomicBoolean(true); return m_objects .computeIfAbsent( @@ -35,7 +35,7 @@ public boolean put(K key, V method) { && exists.get(); } - public C get(K key) { + public C get(@Nullable K key) { return m_objects.computeIfAbsent(key, k -> createValue()); } @@ -43,7 +43,7 @@ public Set keySet() { return new HashSet<>(m_objects.keySet()); } - public boolean containsKey(K k) { + public boolean containsKey(@Nullable K k) { return m_objects.containsKey(k); } @@ -68,7 +68,7 @@ public int size() { return m_objects.size(); } - public boolean remove(K key, V value) { + public boolean remove(@Nullable K key, V value) { return get(key).remove(value); } @@ -78,7 +78,7 @@ public boolean remove(K key, V value) { * @param key the key to drop. * @return the values that were held, or {@code null} when the key was not present. */ - public @Nullable C removeAll(K key) { + public @Nullable C removeAll(@Nullable K key) { return m_objects.remove(key); } @@ -90,7 +90,7 @@ public Collection values() { return m_objects.values(); } - public boolean putAll(K k, Collection values) { + public boolean putAll(@Nullable K k, Collection values) { boolean result = false; for (V v : values) { result = put(k, v) || result; diff --git a/testng-core/src/main/java/org/testng/internal/BaseClassFinder.java b/testng-core/src/main/java/org/testng/internal/BaseClassFinder.java index ddf774fd4..3aeac6be3 100644 --- a/testng-core/src/main/java/org/testng/internal/BaseClassFinder.java +++ b/testng-core/src/main/java/org/testng/internal/BaseClassFinder.java @@ -32,8 +32,8 @@ protected void putIClass(Class cls, IClass iClass) { protected IClass findOrCreateIClass( ITestContext context, Class cls, - XmlClass xmlClass, - IObject.IdentifiableObject instance, + @Nullable XmlClass xmlClass, + IObject.@Nullable IdentifiableObject instance, IAnnotationFinder annotationFinder, ITestObjectFactory objectFactory) { diff --git a/testng-core/src/main/java/org/testng/internal/ClassImpl.java b/testng-core/src/main/java/org/testng/internal/ClassImpl.java index c831e88ff..7fdb5e9ef 100644 --- a/testng-core/src/main/java/org/testng/internal/ClassImpl.java +++ b/testng-core/src/main/java/org/testng/internal/ClassImpl.java @@ -30,17 +30,17 @@ public class ClassImpl implements IClass, IObject { private final List identifiableObjects = new ArrayList<>(); private final Map, IClass> m_classes; private long @Nullable [] m_instanceHashCodes; - private final IObject.IdentifiableObject m_instance; + private final IObject.@Nullable IdentifiableObject m_instance; private final ITestObjectFactory m_objectFactory; private @Nullable String m_testName = null; - private final XmlClass m_xmlClass; + private final @Nullable XmlClass m_xmlClass; private final ITestContext m_testContext; public ClassImpl( ITestContext context, Class cls, - XmlClass xmlClass, - IObject.IdentifiableObject instance, + @Nullable XmlClass xmlClass, + IObject.@Nullable IdentifiableObject instance, Map, IClass> classes, IAnnotationFinder annotationFinder, ITestObjectFactory objectFactory) { @@ -51,8 +51,9 @@ public ClassImpl( m_annotationFinder = annotationFinder; m_instance = instance; m_objectFactory = objectFactory; - if (IObject.IdentifiableObject.unwrap(instance) instanceof ITest) { - m_testName = ((ITest) instance.getInstance()).getTestName(); + Object unwrapped = IObject.IdentifiableObject.unwrap(instance); + if (unwrapped instanceof ITest) { + m_testName = ((ITest) unwrapped).getTestName(); } if (m_testName == null) { ITestAnnotation annotation = m_annotationFinder.findAnnotation(cls, ITestAnnotation.class); @@ -88,11 +89,12 @@ public XmlTest getXmlTest() { } @Override - public XmlClass getXmlClass() { + public @Nullable XmlClass getXmlClass() { return m_xmlClass; } - private IObject.IdentifiableObject getDefaultInstance(boolean create, String errMsgPrefix) { + private IObject.@Nullable IdentifiableObject getDefaultInstance( + boolean create, String errMsgPrefix) { if (m_defaultInstance == null) { if (m_instance != null) { m_defaultInstance = m_instance; @@ -170,7 +172,9 @@ private static int computeHashCode(IdentifiableObject identifiable) { // derive a stable one from its unique instance id instead. return identifiable.getInstanceId().hashCode(); } - return IParameterInfo.embeddedInstance(instance).hashCode(); + return java.util.Objects.requireNonNull( + IParameterInfo.embeddedInstance(instance), "the factory instance is not available") + .hashCode(); } private DetailedAttributes newDetailedAttributes(boolean create, String errMsgPrefix) { diff --git a/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java b/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java index 443c30f98..919cf84ed 100644 --- a/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java +++ b/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java @@ -81,21 +81,26 @@ public List getAfterGroupMethods(ITestNGMethod testMethod) { Set methodGroups = new HashSet<>(Arrays.asList(testMethod.getGroups())); try (AutoCloseableLock ignore = afterGroups.lock()) { - if (m_afterGroupsMap == null) { - m_afterGroupsMap = initializeAfterGroupsMap(); + Map> afterGroupsMap = m_afterGroupsMap; + if (afterGroupsMap == null) { + afterGroupsMap = initializeAfterGroupsMap(); + m_afterGroupsMap = afterGroupsMap; } + Map> groupsMap = afterGroupsMap; return methodGroups.stream() - .filter(t -> isLastMethodForGroup(t, testMethod)) + .filter(t -> isLastMethodForGroup(groupsMap, t, testMethod)) .map(t -> retrieve(afterGroupsThatHaveAlreadyRun, m_afterGroupsMethods, t)) .flatMap(Collection::stream) - .filter(t -> isAfterGroupAllowedToRunAfterTestMethod(t, methodGroups)) + .filter(t -> isAfterGroupAllowedToRunAfterTestMethod(groupsMap, t, methodGroups)) .collect(Collectors.toList()); } } private boolean isAfterGroupAllowedToRunAfterTestMethod( - ITestNGMethod afterGroupMethod, Set testMethodGroups) { + Map> afterGroupsMap, + ITestNGMethod afterGroupMethod, + Set testMethodGroups) { String[] afterGroupMethodGroups = afterGroupMethod.getAfterGroups(); if (afterGroupMethodGroups.length == 1 || testMethodGroups.containsAll(Arrays.asList(afterGroupMethodGroups))) { @@ -105,7 +110,7 @@ private boolean isAfterGroupAllowedToRunAfterTestMethod( .allMatch( t -> testMethodGroups.contains(t) - || !CollectionUtils.hasElements(m_afterGroupsMap.get(t))); + || !CollectionUtils.hasElements(afterGroupsMap.get(t))); } public void removeBeforeGroups(String[] groups) { @@ -130,8 +135,9 @@ public void removeAfterGroups(Collection groups) { * @return true if the passed method is the last to run for the group. This method is used to * figure out when is the right time to invoke afterGroups methods. */ - private boolean isLastMethodForGroup(String group, ITestNGMethod method) { - List methodsInGroup = m_afterGroupsMap.get(group); + private boolean isLastMethodForGroup( + Map> afterGroupsMap, String group, ITestNGMethod method) { + List methodsInGroup = afterGroupsMap.get(group); if (null == methodsInGroup || methodsInGroup.isEmpty()) { return true; diff --git a/testng-core/src/main/java/org/testng/internal/DynamicGraph.java b/testng-core/src/main/java/org/testng/internal/DynamicGraph.java index b66f75d97..1a5034ebb 100644 --- a/testng-core/src/main/java/org/testng/internal/DynamicGraph.java +++ b/testng-core/src/main/java/org/testng/internal/DynamicGraph.java @@ -14,6 +14,7 @@ import org.jspecify.annotations.Nullable; import org.testng.IDynamicGraph; import org.testng.IExecutionVisualiser; +import java.util.Objects; /** Representation of the graph of methods. */ public class DynamicGraph implements IDynamicGraph { @@ -309,9 +310,9 @@ int getLowestEdgeWeight(Set nodes) { int lowestWeight = Integer.MAX_VALUE; for (T node : intersection) { - Map weightMap = m_outgoingEdges.get(node); - + // The intersection was taken against this very key set, so the lookup cannot miss. // Not catching NoSuchElementException, because that would indicate our graph is corrupt. + Map weightMap = Objects.requireNonNull(m_outgoingEdges.get(node)); lowestWeight = Math.min(lowestWeight, Collections.min(weightMap.values())); } return lowestWeight; diff --git a/testng-core/src/main/java/org/testng/internal/DynamicGraphHelper.java b/testng-core/src/main/java/org/testng/internal/DynamicGraphHelper.java index fe81fd896..5851e0979 100644 --- a/testng-core/src/main/java/org/testng/internal/DynamicGraphHelper.java +++ b/testng-core/src/main/java/org/testng/internal/DynamicGraphHelper.java @@ -17,6 +17,7 @@ import org.testng.xml.XmlSuite; import org.testng.xml.XmlSuite.ParallelMode; import org.testng.xml.XmlTest; +import java.util.concurrent.atomic.AtomicBoolean; public final class DynamicGraphHelper { @@ -33,7 +34,7 @@ public static DynamicGraph createDynamicGraph( // Keep track of whether we have group dependencies. If we do, preserve-order needs // to be ignored since group dependencies create inter-class dependencies which can // end up creating cycles when combined with preserve-order. - final AtomicReference hasDependencies = new AtomicReference<>(false); + final AtomicBoolean hasDependencies = new AtomicBoolean(false); Arrays.stream(methods) .forEach( m -> { diff --git a/testng-core/src/main/java/org/testng/internal/FactoryMethod.java b/testng-core/src/main/java/org/testng/internal/FactoryMethod.java index 73d0b8b9b..5c74809b7 100644 --- a/testng-core/src/main/java/org/testng/internal/FactoryMethod.java +++ b/testng-core/src/main/java/org/testng/internal/FactoryMethod.java @@ -78,7 +78,7 @@ private void init( // constructor outside of this package. FactoryMethod( ConstructorOrMethod com, - IObject.IdentifiableObject identifiable, + IObject.@Nullable IdentifiableObject identifiable, IAnnotationFinder annotationFinder, ITestContext testContext, ITestObjectFactory objectFactory, diff --git a/testng-core/src/main/java/org/testng/internal/Graph.java b/testng-core/src/main/java/org/testng/internal/Graph.java index 9936fbb0f..ff109cbed 100644 --- a/testng-core/src/main/java/org/testng/internal/Graph.java +++ b/testng-core/src/main/java/org/testng/internal/Graph.java @@ -33,7 +33,7 @@ public class Graph { // A map of nodes that are not the predecessors of any node // (not needed for the algorithm but convenient to calculate // the parallel/sequential lists in TestNG). - private Map> m_independentNodes = null; + private @Nullable Map> m_independentNodes = null; public Graph(Comparator> comparator) { this.comparator = comparator; @@ -54,7 +54,7 @@ public Set getPredecessors(T node) { } public boolean isIndependent(T object) { - return m_independentNodes.containsKey(object); + return initializeIndependentNodes().containsKey(object); } private @Nullable Node findNode(T object) { @@ -68,9 +68,9 @@ public void addPredecessor(T tm, T predecessor) { } else { node.addPredecessor(predecessor); // Remove these two nodes from the independent list - initializeIndependentNodes(); - m_independentNodes.remove(predecessor); - m_independentNodes.remove(tm); + Map> independentNodes = initializeIndependentNodes(); + independentNodes.remove(predecessor); + independentNodes.remove(tm); log(() -> " REMOVED " + predecessor + " FROM INDEPENDENT OBJECTS"); } } @@ -81,7 +81,7 @@ private Collection> getNodes() { /** @return All the nodes that don't have any order with each other. */ public Set getIndependentNodes() { - return m_independentNodes.keySet(); + return initializeIndependentNodes().keySet(); } /** @return All the nodes that have an order with each other, sorted in one of the valid sorts. */ @@ -141,9 +141,10 @@ public void topologicalSort() { dumpSortedNodes(sorted); } - private void initializeIndependentNodes() { - if (null == m_independentNodes) { - m_independentNodes = + private Map> initializeIndependentNodes() { + Map> independentNodes = m_independentNodes; + if (null == independentNodes) { + independentNodes = new ArrayList<>(m_nodes.values()) .stream() .sorted(comparator) @@ -153,7 +154,9 @@ private void initializeIndependentNodes() { Function.identity(), (a, b) -> a, Maps::newLinkedHashMap)); + m_independentNodes = independentNodes; } + return independentNodes; } private void dumpSortedNodes(List sorted) { diff --git a/testng-core/src/main/java/org/testng/internal/IObject.java b/testng-core/src/main/java/org/testng/internal/IObject.java index 8cd28380f..2091a5ffd 100644 --- a/testng-core/src/main/java/org/testng/internal/IObject.java +++ b/testng-core/src/main/java/org/testng/internal/IObject.java @@ -42,7 +42,7 @@ static long[] instanceHashCodes(@Nullable Object object) { * @return - An array (can be empty is instance compatibility fails) of {@link IdentifiableObject} * objects. */ - static IdentifiableObject[] objects(Object object, boolean create) { + static IdentifiableObject[] objects(@Nullable Object object, boolean create) { return objects(object, create, ""); } @@ -54,7 +54,8 @@ static IdentifiableObject[] objects(Object object, boolean create) { * @return - An array (can be empty is instance compatibility fails) of {@link IdentifiableObject} * objects. */ - static IdentifiableObject[] objects(Object object, boolean create, String errorMsgPrefix) { + static IdentifiableObject[] objects( + @Nullable Object object, boolean create, String errorMsgPrefix) { return cast(object) .map(it -> it.getObjects(create, errorMsgPrefix)) .orElse(new IdentifiableObject[] {}); diff --git a/testng-core/src/main/java/org/testng/internal/ITestClassConfigInfo.java b/testng-core/src/main/java/org/testng/internal/ITestClassConfigInfo.java index c8695791b..11cefd5e8 100644 --- a/testng-core/src/main/java/org/testng/internal/ITestClassConfigInfo.java +++ b/testng-core/src/main/java/org/testng/internal/ITestClassConfigInfo.java @@ -5,6 +5,7 @@ import java.util.UUID; import org.testng.ITestClass; import org.testng.ITestNGMethod; +import org.jspecify.annotations.Nullable; public interface ITestClassConfigInfo { @@ -23,9 +24,9 @@ public interface ITestClassConfigInfo { * @param instanceId the per-instance id (UUID) of the test class instance * @return All before class methods of instance */ - List getInstanceBeforeClassMethods(UUID instanceId); + List getInstanceBeforeClassMethods(@Nullable UUID instanceId); - List getInstanceAfterClassMethods(UUID instanceId); + List getInstanceAfterClassMethods(@Nullable UUID instanceId); static List allBeforeClassMethods(ITestClass tc) { if (tc instanceof ITestClassConfigInfo) { diff --git a/testng-core/src/main/java/org/testng/internal/MethodGroupsHelper.java b/testng-core/src/main/java/org/testng/internal/MethodGroupsHelper.java index fd93f3a9a..d4a0d910a 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodGroupsHelper.java +++ b/testng-core/src/main/java/org/testng/internal/MethodGroupsHelper.java @@ -20,6 +20,8 @@ import org.testng.internal.annotations.AnnotationHelper; import org.testng.internal.annotations.IAnnotationFinder; import org.testng.internal.collections.Pair; +import org.jspecify.annotations.Nullable; +import java.util.Objects; /** Collections of helper methods to help deal with test methods */ public class MethodGroupsHelper { @@ -41,7 +43,7 @@ static void collectMethodsByGroup( boolean unique) { for (ITestNGMethod tm : methods) { boolean in = false; - Method m = tm.getConstructorOrMethod().getMethod(); + Method m = tm.getConstructorOrMethod().requireMethod(); // // @Test method // @@ -60,7 +62,10 @@ static void collectMethodsByGroup( // @Configuration method // else { - IConfigurationAnnotation annotation = AnnotationHelper.findConfiguration(finder, m); + IConfigurationAnnotation annotation = + Objects.requireNonNull( + AnnotationHelper.findConfiguration(finder, m), + "a configuration method always carries a @Before/@After annotation"); if (annotation.getAlwaysRun()) { if (!unique || MethodGroupsHelper.isMethodAlreadyNotPresent(outIncludedMethods, tm)) { in = true; @@ -85,7 +90,7 @@ static void collectMethodsByGroup( } private static boolean includeMethod( - ITestOrConfiguration annotation, + @Nullable ITestOrConfiguration annotation, RunInfo runInfo, ITestNGMethod tm, boolean forTests, @@ -215,7 +220,8 @@ protected static void findGroupTransitiveClosure( outGroups.addAll(runningGroups.keySet()); } - private static ITestNGMethod findMethodNamed(String tm, List allMethods) { + private static @Nullable ITestNGMethod findMethodNamed( + String tm, List allMethods) { return allMethods.stream() .filter(m -> m.getQualifiedName().equals(tm)) .findFirst() diff --git a/testng-core/src/main/java/org/testng/internal/MethodHelper.java b/testng-core/src/main/java/org/testng/internal/MethodHelper.java index db5dd38d3..498ba65dc 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodHelper.java +++ b/testng-core/src/main/java/org/testng/internal/MethodHelper.java @@ -58,7 +58,7 @@ public static ITestNGMethod[] collectAndOrderMethods( boolean unique, List outExcludedMethods, Comparator comparator) { - AtomicReference results = new AtomicReference<>(); + AtomicReference results = new AtomicReference<>(new ITestNGMethod[0]); List includedMethods = new ArrayList<>(); TimeUtils.computeAndShowTime( "MethodGroupsHelper.collectMethodsByGroup()", @@ -77,7 +77,7 @@ public static ITestNGMethod[] collectAndOrderMethods( results.set( sortMethods(forTests, includedMethods, comparator) .toArray(new ITestNGMethod[] {}))); - return results.get(); + return Objects.requireNonNull(results.get(), "the sorted methods were never published"); } /** @@ -225,7 +225,8 @@ public static ITestNGMethod[] findDependedUponMethods(ITestNGMethod m, ITestNGMe * @param testngMethod TestNG method * @param regExp regex representing a method and/or related class name */ - private static Method findMethodByName(ITestNGMethod testngMethod, String regExp) { + private static @Nullable Method findMethodByName( + ITestNGMethod testngMethod, @Nullable String regExp) { if (regExp == null) { return null; } @@ -275,7 +276,7 @@ public static boolean isEnabled(@Nullable ITestOrConfiguration test) { return null == test || test.getEnabled(); } - public static boolean isAlwaysRun(IConfigurationAnnotation configurationAnnotation) { + public static boolean isAlwaysRun(@Nullable IConfigurationAnnotation configurationAnnotation) { if (null == configurationAnnotation) { return false; } @@ -340,11 +341,11 @@ private static Graph topologicalSort( String[] methodsDependedUpon = m.getMethodsDependedUpon(); if (methodsDependedUpon.length > 0) { ITestNGMethod[] methodsNamed; + Object instanceId = IInstanceIdentity.getInstanceId(m); // Method has instance - if (IInstanceIdentity.getInstanceId(m) != null) { - // Get other methods with the same instance - List instanceMethods = - testInstances.get(IInstanceIdentity.getInstanceId(m)); + List instanceMethods = + instanceId == null ? null : testInstances.get(instanceId); + if (instanceMethods != null) { try { // Search for other methods that depends upon with the same instance methodsNamed = MethodHelper.findDependedUponMethods(m, instanceMethods); @@ -566,7 +567,8 @@ private static boolean isConfigurationMethod(ITestNGMethod tm) { || tm.isAfterMethodConfiguration(); } - protected static String calculateMethodCanonicalName(Class methodClass, String methodName) { + protected static @Nullable String calculateMethodCanonicalName( + Class methodClass, String methodName) { Set methods = ClassHelper.getAvailableMethods(methodClass); // TESTNG-139 return methods.stream() .filter(method -> methodName.equals(method.getName())) diff --git a/testng-core/src/main/java/org/testng/internal/MethodInheritance.java b/testng-core/src/main/java/org/testng/internal/MethodInheritance.java index 4b77602d7..b5866bd28 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodInheritance.java +++ b/testng-core/src/main/java/org/testng/internal/MethodInheritance.java @@ -100,8 +100,9 @@ public static void fixMethodInheritance(ITestNGMethod[] methods, boolean before) l.add(method); } else { Class subClass = findSubClass(map, methodClass); - if (null != subClass) { - l = map.get(subClass); + List subClassMethods = subClass == null ? null : map.get(subClass); + if (null != subClassMethods) { + l = subClassMethods; l.add(method); map.remove(subClass); map.put(methodClass, l); diff --git a/testng-core/src/main/java/org/testng/internal/MethodSorting.java b/testng-core/src/main/java/org/testng/internal/MethodSorting.java index 632e4ad6f..920f1bd0e 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodSorting.java +++ b/testng-core/src/main/java/org/testng/internal/MethodSorting.java @@ -7,6 +7,7 @@ import java.util.UUID; import org.testng.IFactoryInstance; import org.testng.ITestNGMethod; +import org.jspecify.annotations.Nullable; public enum MethodSorting implements Comparator { METHOD_NAMES("methods") { @@ -46,7 +47,7 @@ private int objectEquality(ITestNGMethod a, ITestNGMethod b) { // sorting never forces a lazy @Factory instance to be created during collection. Object one = IInstanceIdentity.getInstanceId(a); Object two = IInstanceIdentity.getInstanceId(b); - if (IInstanceIdentity.isIdentityAware(one, two)) { + if (one != null && two != null && IInstanceIdentity.isIdentityAware(one, two)) { return ((UUID) one).compareTo((UUID) two); } return Integer.compare(Objects.hashCode(one), Objects.hashCode(two)); diff --git a/testng-core/src/main/java/org/testng/internal/Parameters.java b/testng-core/src/main/java/org/testng/internal/Parameters.java index c180c6999..fcb553f38 100644 --- a/testng-core/src/main/java/org/testng/internal/Parameters.java +++ b/testng-core/src/main/java/org/testng/internal/Parameters.java @@ -165,12 +165,12 @@ public static Object[] createInstantiationParameters( public static Object[] createConfigurationParameters( Method m, Map params, - Object[] parameterValues, - ITestNGMethod currentTestMethod, + Object @Nullable [] parameterValues, + @Nullable ITestNGMethod currentTestMethod, IAnnotationFinder finder, XmlSuite xmlSuite, ITestContext ctx, - ITestResult testResult) { + @Nullable ITestResult testResult) { Method currentTestMeth = currentTestMethod != null ? currentTestMethod.getConstructorOrMethod().getMethod() : null; diff --git a/testng-core/src/main/java/org/testng/internal/ScriptSelectorFactory.java b/testng-core/src/main/java/org/testng/internal/ScriptSelectorFactory.java index 2fe45e334..49dfec3f3 100644 --- a/testng-core/src/main/java/org/testng/internal/ScriptSelectorFactory.java +++ b/testng-core/src/main/java/org/testng/internal/ScriptSelectorFactory.java @@ -6,6 +6,7 @@ import javax.script.ScriptEngineFactory; import org.testng.TestNGException; import org.testng.xml.XmlScript; +import java.util.Objects; public final class ScriptSelectorFactory { @@ -39,6 +40,8 @@ public static ScriptMethodSelector getScriptSelector(XmlScript script) { + "https://github.com/cbeust/testng/wiki/Supported-script-engines"); } - return new ScriptMethodSelector(engineFactory.getScriptEngine(), script.getExpression()); + return new ScriptMethodSelector( + engineFactory.getScriptEngine(), + Objects.requireNonNull(script.getExpression(), "the