From c65538970c358c4204b1b7dad2a5f589a9c428f9 Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Tue, 18 Aug 2026 09:05:01 +0200 Subject: [PATCH 1/7] refactor(internal): use jspecify @Nullable instead of spotbugs Utils carries eight @Nullable annotations from javax.annotation, which spotbugs provides. The class is published API, and spotbugs is a compileOnly dependency: the annotations are dropped from the artifact, so no consumer and no consumer's checker can read the contract they state. JSpecify is declared as a regular api dependency for exactly that reason, which testng.java-library.gradle.kts records. NullAway matches either annotation by simple name, so nothing changes for the check itself. --- testng-core-api/src/main/java/org/testng/internal/Utils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ce5b3bb99..9d8e232d3 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 @@ -19,7 +19,7 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; -import javax.annotation.Nullable; +import org.jspecify.annotations.Nullable; import org.testng.ITestNGMethod; import org.testng.TestNGException; import org.testng.log4testng.Logger; From 4cf279087bb571340bea542e217b0799828d7f3f Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Tue, 18 Aug 2026 09:06:12 +0200 Subject: [PATCH 2/7] refactor(internal): drop the redundant @Nonnull on compareTo MethodSelectorDescriptor and TestResult annotate their compareTo parameter with javax.annotation.@Nonnull. Comparable.compareTo is unannotated in the JDK, so the annotation constrains nothing a caller can act on, and spotbugs is a compileOnly dependency here: it never reaches the published artifact. The build runs no spotbugs analysis either -- only the annotation jar is on the classpath. Once the package is null-marked non-null becomes the default, so converting these to jspecify @NonNull would restate the default rather than add information. They are removed instead. TestResult held the only javax.annotation reference in testng-runner-api, so that module's spotbugs dependency goes with it. testng-core and testng-core-api keep theirs: four files outside this package still use javax.annotation. --- .../java/org/testng/internal/MethodSelectorDescriptor.java | 3 +-- .../src/main/java/org/testng/internal/TestResult.java | 3 +-- testng-runner-api/testng-runner-api-build.gradle.kts | 1 - 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/testng-core/src/main/java/org/testng/internal/MethodSelectorDescriptor.java b/testng-core/src/main/java/org/testng/internal/MethodSelectorDescriptor.java index 8ed68c98e..1079cfa0b 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodSelectorDescriptor.java +++ b/testng-core/src/main/java/org/testng/internal/MethodSelectorDescriptor.java @@ -1,7 +1,6 @@ package org.testng.internal; import java.util.List; -import javax.annotation.Nonnull; import org.testng.IMethodSelector; import org.testng.ITestNGMethod; @@ -25,7 +24,7 @@ public MethodSelectorDescriptor(IMethodSelector selector, int priority) { } @Override - public int compareTo(@Nonnull MethodSelectorDescriptor other) { + public int compareTo(MethodSelectorDescriptor other) { return m_priority - other.m_priority; } 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 888e5f43a..f573c534e 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 @@ -10,7 +10,6 @@ import java.util.UUID; import java.util.regex.Pattern; import java.util.stream.Collectors; -import javax.annotation.Nonnull; import org.testng.IAttributes; import org.testng.IClass; import org.testng.IFactoryInstance; @@ -349,7 +348,7 @@ public void setContext(ITestContext context) { } @Override - public int compareTo(@Nonnull ITestResult comparison) { + public int compareTo(ITestResult comparison) { return Long.compare(getStartMillis(), comparison.getStartMillis()); } diff --git a/testng-runner-api/testng-runner-api-build.gradle.kts b/testng-runner-api/testng-runner-api-build.gradle.kts index a1b835e71..c664be963 100644 --- a/testng-runner-api/testng-runner-api-build.gradle.kts +++ b/testng-runner-api/testng-runner-api-build.gradle.kts @@ -4,5 +4,4 @@ plugins { dependencies { api(projects.testngCoreApi) - compileOnly("com.github.spotbugs:spotbugs:4.10.3") } From 16e5cb192d6836c9cffbcb54e2514ec94f056484 Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Tue, 18 Aug 2026 23:27:07 +0200 Subject: [PATCH 3/7] refactor(internal): state the nullness contracts of org.testng.internal Prepares org.testng.internal for @NullMarked by writing down, in the signatures, the contracts the bodies already implement. The package-info that turns the check on lands in the follow-up, so nothing here changes what the build verifies -- NullAway stays inert for this package. What the annotations record, with the caller that proves each one: - ConstructorOrMethod.getMethod()/getConstructor() answer null by design; the wrapper holds either a method or a constructor. A new requireMethod() gives the sixteen call sites that only ever see a test or configuration method a non-null accessor, so they stop restating an invariant the wrapper knows. It throws NullPointerException, the same failure those sites already produced from the dereference that followed, now with a message. - IParameterInfo.getInstance() is null for a lazy implementation whose construction failed: LazyParameterInfo memoizes the failure instead of rethrowing it, and getInstantiationFailure() reports it. - ITestResult.getMethod() is already read through Optional.ofNullable in ITestResult itself; TestResult carries no method when built by newTestResult(Object[], int) to hold parameters only. - Configuration's five optional collaborators are null until TestNG configures them, and every caller tests for it -- SuiteRunner checks getObjectFactory() == null, TestRunner checks getListenerFactory() != null. No default is invented for them: one would make those branches dead. Setters follow their getters so a Kotlin consumer keeps a mutable property rather than a read-only one, as XMLReporterConfig.setOutputDirectory did. Behaviour-neutral restructurings that remove the need for an annotation: - Configuration chains its constructors instead of sharing an init() helper. - PackageUtils builds the classpath cache locally and publishes it once; the old code stored the array before filling it, so a concurrent reader could observe null elements. The field is volatile to finish that. - TestResult reads the resolved method through one local, and asserts it in an accessor rather than at each of the seven members that use it. - BaseTestMethod initialises m_retryAnalyzerClass to DisabledRetryAnalyzer, which is what its setter already normalised null to. - ConfigurationGroupMethods answers an absent group with an empty list, which is what it already answered when the group was being tracked. - A null test written through a boolean local is inlined where the checker needs it, in TestInvoker and ConfigInvoker. TestInvoker no longer tests getTestClass() for null before invoking: the accessor now raises that diagnostic itself, with the same message. --- .../testng/internal/AutoCloseableLock.java | 3 +- .../java/org/testng/internal/ClassHelper.java | 3 +- .../testng/internal/ConstructorOrMethod.java | 26 ++++- .../org/testng/internal/IParameterInfo.java | 16 +++- .../internal/KeyAwareAutoCloseableLock.java | 3 +- .../org/testng/internal/PackageUtils.java | 17 ++-- .../org/testng/internal/PropertyUtils.java | 10 +- .../org/testng/internal/ReporterConfig.java | 3 +- .../org/testng/internal/RuntimeBehavior.java | 3 +- .../main/java/org/testng/internal/Utils.java | 26 ++--- .../org/testng/internal/BaseClassFinder.java | 3 +- .../org/testng/internal/BaseTestMethod.java | 36 +++---- .../java/org/testng/internal/ClassImpl.java | 7 +- .../org/testng/internal/ClassInfoMap.java | 3 +- .../org/testng/internal/ClonedMethod.java | 3 +- .../org/testng/internal/Configuration.java | 37 ++++--- .../internal/ConfigurationGroupMethods.java | 15 +-- .../testng/internal/ConfigurationMethod.java | 33 +++---- .../internal/DataProviderMethodRemovable.java | 5 +- .../internal/DefaultListenerFactory.java | 3 +- .../org/testng/internal/DynamicGraph.java | 2 + .../java/org/testng/internal/ExitCode.java | 4 +- .../testng/internal/FilteredParameters.java | 3 +- .../main/java/org/testng/internal/Graph.java | 7 +- .../org/testng/internal/IConfiguration.java | 16 +++- .../java/org/testng/internal/IObject.java | 3 +- .../testng/internal/LazyParameterInfo.java | 5 +- .../internal/ListenerOrderDeterminer.java | 5 +- .../org/testng/internal/MethodHelper.java | 5 +- .../testng/internal/MethodInheritance.java | 3 +- .../org/testng/internal/NoOpTestClass.java | 16 ++-- .../java/org/testng/internal/Parameters.java | 17 ++-- .../main/java/org/testng/internal/Tarjan.java | 3 +- .../testng/internal/TestListenerHelper.java | 9 +- .../testng/internal/TestMethodContainer.java | 3 +- .../testng/internal/TestNGClassFinder.java | 12 ++- .../org/testng/internal/TestNGMethod.java | 13 +-- .../testng/internal/XmlMethodSelector.java | 3 +- .../internal/invokers/ConfigInvoker.java | 11 +-- .../invokers/InvokeMethodRunnable.java | 4 +- .../invokers/MethodInvocationHelper.java | 2 +- .../internal/invokers/MethodRunner.java | 4 +- .../internal/invokers/ParameterHandler.java | 13 ++- .../internal/invokers/ParameterHolder.java | 6 +- .../testng/internal/invokers/TestInvoker.java | 7 +- .../org/testng/reporters/FailedReporter.java | 4 +- .../java/org/testng/internal/Attributes.java | 5 +- .../internal/LiteWeightTestNGMethod.java | 3 +- .../java/org/testng/internal/TestResult.java | 96 +++++++++++-------- .../main/java/org/testng/internal/Yaml.java | 16 ++-- .../java/org/testng/internal/YamlParser.java | 3 +- .../java/org/testng/internal/YamlSchema.java | 16 +++- 52 files changed, 343 insertions(+), 231 deletions(-) diff --git a/testng-core-api/src/main/java/org/testng/internal/AutoCloseableLock.java b/testng-core-api/src/main/java/org/testng/internal/AutoCloseableLock.java index c5e821ba0..15ca115ea 100644 --- a/testng-core-api/src/main/java/org/testng/internal/AutoCloseableLock.java +++ b/testng-core-api/src/main/java/org/testng/internal/AutoCloseableLock.java @@ -3,6 +3,7 @@ import java.io.Closeable; import java.util.Objects; import java.util.concurrent.locks.ReentrantLock; +import org.jspecify.annotations.Nullable; /** * A simple abstraction over {@link ReentrantLock} that can be used in conjunction with @@ -27,7 +28,7 @@ public void close() { } @Override - public boolean equals(Object object) { + public boolean equals(@Nullable Object object) { if (this == object) { return true; } diff --git a/testng-core-api/src/main/java/org/testng/internal/ClassHelper.java b/testng-core-api/src/main/java/org/testng/internal/ClassHelper.java index 1db07de40..6662bcd82 100644 --- a/testng-core-api/src/main/java/org/testng/internal/ClassHelper.java +++ b/testng-core-api/src/main/java/org/testng/internal/ClassHelper.java @@ -14,6 +14,7 @@ import java.util.Vector; import java.util.function.BiConsumer; import java.util.stream.Collectors; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; import org.testng.annotations.IFactoryAnnotation; import org.testng.internal.annotations.IAnnotationFinder; @@ -67,7 +68,7 @@ static List appendContextualClassLoaders(List currentL * @param className the class name to be loaded. * @return the class or null if the class is not found. */ - public static Class forName(final String className) { + public static @Nullable Class forName(final String className) { List allClassLoaders = appendContextualClassLoaders(classLoaders); for (ClassLoader classLoader : allClassLoaders) { 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 f43229444..b599905df 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 @@ -3,6 +3,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Executable; import java.lang.reflect.Method; +import org.jspecify.annotations.Nullable; /** * Wraps either a method or a constructor. @@ -40,14 +41,33 @@ public Class[] getParameterTypes() { return member.getParameterTypes(); // the JDK returns a fresh copy each call } - public Method getMethod() { + /** + * @return the wrapped member if it is a method, or {@code null} if it is a constructor. Prefer + * {@link #requireMethod()} unless the null is what you are testing for. + */ + public @Nullable Method getMethod() { return member instanceof Method ? (Method) member : null; } - public Constructor getConstructor() { + public @Nullable Constructor getConstructor() { return member instanceof Constructor ? (Constructor) member : null; } + /** + * The wrapped member as a {@link Method}, for the callers that only ever see a test or a + * configuration method. + * + * @return the wrapped method + * @throws NullPointerException if this wrapper holds a constructor -- the same failure the call + * sites saw before, with a message instead of a bare dereference + */ + public Method requireMethod() { + if (member instanceof Method) { + return (Method) member; + } + throw new NullPointerException("Expected a method, but " + member + " is a constructor"); + } + /** * Makes the wrapped member accessible. When interning is on the handle is shared, so this is * observable through every wrapper of the same member. @@ -57,7 +77,7 @@ public void makeAccessible() { } @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { if (this == o) { return true; } diff --git a/testng-core-api/src/main/java/org/testng/internal/IParameterInfo.java b/testng-core-api/src/main/java/org/testng/internal/IParameterInfo.java index 7a16c8c00..c6d35326a 100644 --- a/testng-core-api/src/main/java/org/testng/internal/IParameterInfo.java +++ b/testng-core-api/src/main/java/org/testng/internal/IParameterInfo.java @@ -1,11 +1,17 @@ package org.testng.internal; +import org.jspecify.annotations.Nullable; import org.testng.IFactoryInstance; /** Represents the ability to retrieve the parameters associated with a factory method. */ public interface IParameterInfo { - /** @return - The actual instance associated with a factory method */ + /** + * @return - The actual instance associated with a factory method, or null if a lazy + * implementation's construction failed -- the failure is memoized rather than rethrown, and + * {@link #getInstantiationFailure()} reports it. + */ + @Nullable Object getInstance(); /** @@ -25,7 +31,7 @@ public interface IParameterInfo { * for an implementation that does not provide one. Reading it never instantiates a lazy * instance. */ - default IFactoryInstance getFactoryInstance() { + default @Nullable IFactoryInstance getFactoryInstance() { return null; } @@ -35,7 +41,7 @@ default IFactoryInstance getFactoryInstance() { * created) instance; lazy implementations know it up-front (the declaring class of a * constructor factory) and can answer without triggering construction. */ - default Class getTargetClass() { + default @Nullable Class getTargetClass() { Object instance = getInstance(); return instance == null ? null : instance.getClass(); } @@ -64,11 +70,11 @@ default boolean isInstanceInstantiated() { * failure to the affected instance's methods without the failure being re-thrown on every * access. Always {@code null} for eager implementations. */ - default Throwable getInstantiationFailure() { + default @Nullable Throwable getInstantiationFailure() { return null; } - static Object embeddedInstance(Object original) { + static @Nullable Object embeddedInstance(Object original) { if (original instanceof IParameterInfo) { return ((IParameterInfo) original).getInstance(); } diff --git a/testng-core-api/src/main/java/org/testng/internal/KeyAwareAutoCloseableLock.java b/testng-core-api/src/main/java/org/testng/internal/KeyAwareAutoCloseableLock.java index 0f09e3dc0..9c5f0bd99 100644 --- a/testng-core-api/src/main/java/org/testng/internal/KeyAwareAutoCloseableLock.java +++ b/testng-core-api/src/main/java/org/testng/internal/KeyAwareAutoCloseableLock.java @@ -3,6 +3,7 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; +import org.jspecify.annotations.Nullable; /** * A simple abstraction over {@link java.util.concurrent.locks.ReentrantLock} that can be used when @@ -36,7 +37,7 @@ public void close() { } @Override - public boolean equals(Object object) { + public boolean equals(@Nullable Object object) { if (this == object) { return true; } diff --git a/testng-core-api/src/main/java/org/testng/internal/PackageUtils.java b/testng-core-api/src/main/java/org/testng/internal/PackageUtils.java index 65c032b87..84ca869f5 100644 --- a/testng-core-api/src/main/java/org/testng/internal/PackageUtils.java +++ b/testng-core-api/src/main/java/org/testng/internal/PackageUtils.java @@ -17,6 +17,7 @@ import java.util.function.Function; import java.util.stream.Stream; import java.util.stream.StreamSupport; +import org.jspecify.annotations.Nullable; import org.testng.internal.protocols.Input; import org.testng.internal.protocols.Processor; import org.testng.internal.protocols.UnhandledIOException; @@ -29,7 +30,7 @@ * @author Cedric Beust */ public class PackageUtils { - private static String[] testClassPaths; + private static volatile String @Nullable [] testClassPaths; /** The additional class loaders to find classes in. */ private static final Collection classLoaders = new ConcurrentLinkedDeque<>(); @@ -79,9 +80,10 @@ public static String[] findClassesInPackage( .toArray(String[]::new); } - private static String[] getTestClasspath() { - if (null != testClassPaths) { - return testClassPaths; + private static String @Nullable [] getTestClasspath() { + String[] cached = testClassPaths; + if (null != cached) { + return cached; } String testClasspath = RuntimeBehavior.getTestClasspath(); @@ -90,7 +92,7 @@ private static String[] getTestClasspath() { } String[] classpathFragments = Utils.split(testClasspath, File.pathSeparator); - testClassPaths = new String[classpathFragments.length]; + String[] paths = new String[classpathFragments.length]; for (int i = 0; i < classpathFragments.length; i++) { String path; @@ -105,10 +107,11 @@ private static String[] getTestClasspath() { } } - testClassPaths[i] = path.replace('\\', '/'); + paths[i] = path.replace('\\', '/'); } - return testClassPaths; + testClassPaths = paths; + return paths; } private static Function> asURLs(String packageDir) { diff --git a/testng-core-api/src/main/java/org/testng/internal/PropertyUtils.java b/testng-core-api/src/main/java/org/testng/internal/PropertyUtils.java index 29b53b18b..f1a13e7ec 100644 --- a/testng-core-api/src/main/java/org/testng/internal/PropertyUtils.java +++ b/testng-core-api/src/main/java/org/testng/internal/PropertyUtils.java @@ -8,6 +8,7 @@ import java.beans.PropertyDescriptor; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; import org.testng.log4testng.Logger; @@ -21,7 +22,8 @@ public class PropertyUtils { private static final Logger LOGGER = Logger.getLogger(PropertyUtils.class); @SuppressWarnings("unchecked") - public static T convertType(Class type, String value, String paramName) { + public static @Nullable T convertType( + Class type, @Nullable String value, String paramName) { try { if (value == null || NULL_VALUE.equalsIgnoreCase(value)) { if (type.isPrimitive()) { @@ -93,7 +95,7 @@ public static void setProperty(Object instance, String name, String value) { setPropertyRealValue(instance, name, realValue); } - public static Class getPropertyType(Class instanceClass, String propertyName) { + public static @Nullable Class getPropertyType(Class instanceClass, String propertyName) { if (instanceClass == null) { LOGGER.warn( "Cannot retrieve property class for " + propertyName + ". Target instance class is null"); @@ -105,7 +107,7 @@ public static Class getPropertyType(Class instanceClass, String propertyNa return propDesc.getPropertyType(); } - private static PropertyDescriptor getPropertyDescriptor( + private static @Nullable PropertyDescriptor getPropertyDescriptor( Class targetClass, String propertyName) { PropertyDescriptor result = null; if (targetClass == null) { @@ -127,7 +129,7 @@ private static PropertyDescriptor getPropertyDescriptor( return result; } - public static void setPropertyRealValue(Object instance, String name, Object value) { + public static void setPropertyRealValue(Object instance, String name, @Nullable Object value) { if (instance == null) { LOGGER.warn( "Cannot set property " + name + " with value " + value + ". Target instance is null"); diff --git a/testng-core-api/src/main/java/org/testng/internal/ReporterConfig.java b/testng-core-api/src/main/java/org/testng/internal/ReporterConfig.java index 7021d5303..c6060d8e0 100644 --- a/testng-core-api/src/main/java/org/testng/internal/ReporterConfig.java +++ b/testng-core-api/src/main/java/org/testng/internal/ReporterConfig.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.List; +import org.jspecify.annotations.Nullable; /** Stores the information regarding the configuration of a pluggable report listener. */ public class ReporterConfig { @@ -43,7 +44,7 @@ public String serialize() { return sb.toString(); } - public static ReporterConfig deserialize(String inputString) { + public static @Nullable ReporterConfig deserialize(String inputString) { if (Utils.isStringEmpty(inputString)) { return null; diff --git a/testng-core-api/src/main/java/org/testng/internal/RuntimeBehavior.java b/testng-core-api/src/main/java/org/testng/internal/RuntimeBehavior.java index 0fec19bb9..9c98a977a 100644 --- a/testng-core-api/src/main/java/org/testng/internal/RuntimeBehavior.java +++ b/testng-core-api/src/main/java/org/testng/internal/RuntimeBehavior.java @@ -4,6 +4,7 @@ import java.util.List; import java.util.Optional; import java.util.TimeZone; +import org.jspecify.annotations.Nullable; /** This class houses handling all JVM arguments by TestNG */ public final class RuntimeBehavior { @@ -118,7 +119,7 @@ public static String orderMethodsBasedOn() { return System.getProperty("testng.order"); } - public static String getTestClasspath() { + public static @Nullable String getTestClasspath() { return System.getProperty(TEST_CLASSPATH); } 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 9d8e232d3..9f7699aec 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 @@ -67,7 +67,7 @@ public static void setVerbose(int n) { } public static void writeUtf8File( - @Nullable String outputDir, String fileName, XMLStringBuffer xsb, String prefix) { + @Nullable String outputDir, String fileName, XMLStringBuffer xsb, @Nullable String prefix) { try { final File outDir = outputDir != null ? new File(outputDir) : new File("").getAbsoluteFile(); if (!outDir.exists()) { @@ -223,7 +223,7 @@ public static void log(String msg) { * @param level the logging level of the message. * @param msg the message to log to System.out. */ - public static void log(String cls, int level, String msg) { + public static void log(String cls, int level, @Nullable String msg) { // Why this coupling on a static member of getVerbose()? if (getVerbose() >= level) { if (!cls.isEmpty()) { @@ -243,7 +243,7 @@ public static void warn(String warnMsg) { } /* Tokenize the string using the separator. */ - public static String[] split(String string, String sep) { + public static String[] split(@Nullable String string, String sep) { if (string == null || string.isEmpty()) { return new String[0]; } @@ -285,23 +285,23 @@ public static void writeResourceToFile(File file, String resourceName, Class } } - public static String defaultIfStringEmpty(String s, String defaultValue) { + public static String defaultIfStringEmpty(@Nullable String s, String defaultValue) { return isStringEmpty(s) ? defaultValue : s; } - public static boolean isStringBlank(String s) { + public static boolean isStringBlank(@Nullable String s) { return s == null || s.trim().isEmpty(); } - public static boolean isStringEmpty(String s) { + public static boolean isStringEmpty(@Nullable String s) { return s == null || s.isEmpty(); } - public static boolean isStringNotBlank(String s) { + public static boolean isStringNotBlank(@Nullable String s) { return !isStringBlank(s); } - public static boolean isStringNotEmpty(String s) { + public static boolean isStringNotEmpty(@Nullable String s) { return !isStringEmpty(s); } @@ -357,7 +357,8 @@ private enum StackTraceType { FULL } - public static String escapeHtml(String s) { + /** Escapes the five characters that must not appear literally in HTML or XML text. */ + public static @Nullable String escapeHtml(@Nullable String s) { if (s == null) { return null; } @@ -377,7 +378,8 @@ public static String escapeHtml(String s) { return result.toString(); } - public static String escapeUnicode(String s) { + /** Replaces every character the JVM does not define with the Unicode replacement character. */ + public static @Nullable String escapeUnicode(@Nullable String s) { if (s == null) { return null; } @@ -538,7 +540,7 @@ public static String join(List objects, String separator) { } /* Make sure that either we have an instance or if not, that the method is static */ - public static void checkInstanceOrStatic(Object instance, Method method) { + public static void checkInstanceOrStatic(@Nullable Object instance, @Nullable Method method) { if (instance == null && method != null && !Modifier.isStatic(method.getModifiers())) { throw new TestNGException( "Can't invoke " @@ -548,7 +550,7 @@ public static void checkInstanceOrStatic(Object instance, Method method) { } } - public static void checkReturnType(Method method, Class... returnTypes) { + public static void checkReturnType(@Nullable Method method, Class... returnTypes) { if (method == null) { return; } 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 8656a0311..ddf774fd4 100644 --- a/testng-core/src/main/java/org/testng/internal/BaseClassFinder.java +++ b/testng-core/src/main/java/org/testng/internal/BaseClassFinder.java @@ -2,6 +2,7 @@ import java.util.LinkedHashMap; import java.util.Map; +import org.jspecify.annotations.Nullable; import org.testng.IClass; import org.testng.ITestClassFinder; import org.testng.ITestContext; @@ -18,7 +19,7 @@ public abstract class BaseClassFinder implements ITestClassFinder { private final Map, IClass> m_classes = new LinkedHashMap<>(); @Override - public IClass getIClass(Class cls) { + public @Nullable IClass getIClass(Class cls) { return m_classes.get(cls); } 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 2ab3463be..1ae39ba04 100644 --- a/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java +++ b/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java @@ -18,6 +18,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Pattern; import java.util.stream.Collectors; +import org.jspecify.annotations.Nullable; import org.testng.IClass; import org.testng.IFactoryInstance; import org.testng.IRetryAnalyzer; @@ -45,11 +46,11 @@ public abstract class BaseTestMethod * The test class on which the test method was found. Note that this is not necessarily the * declaring class. */ - protected ITestClass m_testClass; + protected @Nullable ITestClass m_testClass; protected final Class m_methodClass; protected final ConstructorOrMethod m_method; - private String m_signature; + private @Nullable String m_signature; protected String m_id = ""; protected long m_date = -1; protected final IAnnotationFinder m_annotationFinder; @@ -63,8 +64,8 @@ public abstract class BaseTestMethod private final String m_methodName; // If a depends on group is not found - private String m_missingGroup; - private String m_description = null; + private @Nullable String m_missingGroup; + private @Nullable String m_description = null; protected AtomicInteger m_currentInvocationCount = new AtomicInteger(0); private int m_parameterInvocationCount = 1; // Set on the per-invocation clones created for a parallel (threadPoolSize > 1) @@ -72,9 +73,9 @@ public abstract class BaseTestMethod // lastTimeOnly @AfterMethod are run once - as a barrier - around the whole pool // instead of inside each parallel invocation, so the clones must not run them. private boolean m_skipFirstAndLastTimeOnlyConfigs; - private Callable m_moreInvocationChecker; - private IRetryAnalyzer m_retryAnalyzer = null; - private Class m_retryAnalyzerClass = null; + private @Nullable Callable m_moreInvocationChecker; + private @Nullable IRetryAnalyzer m_retryAnalyzer = null; + private Class m_retryAnalyzerClass = DisabledRetryAnalyzer.class; private boolean m_skipFailedInvocations = true; private long m_invocationTimeOut = 0L; @@ -88,7 +89,7 @@ public abstract class BaseTestMethod private int m_priority; private int m_interceptedPriority; - private XmlTest m_xmlTest; + private @Nullable XmlTest m_xmlTest; private final IObject.IdentifiableObject m_instance; private final Map m_testMethodToRetryAnalyzer = new ConcurrentHashMap<>(); @@ -126,7 +127,7 @@ public Class getRealClass() { /** {@inheritDoc} */ @Override - public ITestClass getTestClass() { + public @Nullable ITestClass getTestClass() { return m_testClass; } @@ -155,7 +156,7 @@ public String getMethodName() { } @Override - public Object getInstance() { + public @Nullable Object getInstance() { return Optional.ofNullable(m_instance) .map(IObject.IdentifiableObject::getInstance) .map(IParameterInfo::embeddedInstance) @@ -328,7 +329,7 @@ public Optional getFactoryInstance() { * #getFactoryMethodParamsInfo()} this is not part of {@link ITestNGMethod}, so the * lazy-instantiation details stay available to TestNG without being published. */ - public IParameterInfo getFactoryParameterInfo() { + public @Nullable IParameterInfo getFactoryParameterInfo() { Object instance = m_instance == null ? null : m_instance.getInstance(); return instance instanceof IParameterInfo ? (IParameterInfo) instance : null; } @@ -609,7 +610,8 @@ public String toString() { return getSignature(); } - protected String[] getStringArray(String[] methodArray, String[] classArray) { + protected String[] getStringArray( + String @Nullable [] methodArray, String @Nullable [] classArray) { final Set vResult = new HashSet<>(); if (null != methodArray) { Collections.addAll(vResult, methodArray); @@ -646,7 +648,7 @@ public void addMethodDependedUpon(String method) { /** {@inheritDoc} */ @Override - public String getMissingGroup() { + public @Nullable String getMissingGroup() { return m_missingGroup; } @@ -673,7 +675,7 @@ public void setDescription(String description) { /** {@inheritDoc} */ @Override - public String getDescription() { + public @Nullable String getDescription() { return m_description; } @@ -753,7 +755,7 @@ public boolean hasMoreInvocation() { public abstract ITestNGMethod clone(); @Override - public IRetryAnalyzer getRetryAnalyzer(ITestResult result) { + public @Nullable IRetryAnalyzer getRetryAnalyzer(ITestResult result) { return getRetryAnalyzerConsideringMethodParameters(result); } @@ -837,7 +839,7 @@ public void setInterceptedPriority(int priority) { } @Override - public XmlTest getXmlTest() { + public @Nullable XmlTest getXmlTest() { return m_xmlTest; } @@ -883,7 +885,7 @@ public long getInvocationTime() { return invocationTime; } - private IRetryAnalyzer getRetryAnalyzerConsideringMethodParameters(ITestResult tr) { + private @Nullable IRetryAnalyzer getRetryAnalyzerConsideringMethodParameters(ITestResult tr) { if (this.m_retryAnalyzerClass.equals(DisabledRetryAnalyzer.class)) { return null; } 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 9bc821ef5..11419151e 100644 --- a/testng-core/src/main/java/org/testng/internal/ClassImpl.java +++ b/testng-core/src/main/java/org/testng/internal/ClassImpl.java @@ -4,6 +4,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import org.jspecify.annotations.Nullable; import org.testng.IClass; import org.testng.ITest; import org.testng.ITestContext; @@ -24,14 +25,14 @@ public class ClassImpl implements IClass, IObject { private final Class m_class; - private IObject.IdentifiableObject m_defaultInstance = null; + private IObject.@Nullable IdentifiableObject m_defaultInstance = null; private final IAnnotationFinder m_annotationFinder; private final List identifiableObjects = new ArrayList<>(); private final Map, IClass> m_classes; - private long[] m_instanceHashCodes; + private long @Nullable [] m_instanceHashCodes; private final IObject.IdentifiableObject m_instance; private final ITestObjectFactory m_objectFactory; - private String m_testName = null; + private @Nullable String m_testName = null; private final XmlClass m_xmlClass; private final ITestContext m_testContext; diff --git a/testng-core/src/main/java/org/testng/internal/ClassInfoMap.java b/testng-core/src/main/java/org/testng/internal/ClassInfoMap.java index 06a91f4e9..7b9b28a12 100644 --- a/testng-core/src/main/java/org/testng/internal/ClassInfoMap.java +++ b/testng-core/src/main/java/org/testng/internal/ClassInfoMap.java @@ -5,6 +5,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import org.jspecify.annotations.Nullable; import org.testng.xml.XmlClass; public class ClassInfoMap { @@ -56,7 +57,7 @@ public void addClass(Class cls) { m_map.put(cls, null); } - public XmlClass getXmlClass(Class cls) { + public @Nullable XmlClass getXmlClass(Class cls) { return m_map.get(cls); } 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 ddccfdc9d..691e643db 100644 --- a/testng-core/src/main/java/org/testng/internal/ClonedMethod.java +++ b/testng-core/src/main/java/org/testng/internal/ClonedMethod.java @@ -7,6 +7,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.Callable; +import org.jspecify.annotations.Nullable; import org.testng.IClass; import org.testng.IFactoryInstance; import org.testng.IRetryAnalyzer; @@ -19,7 +20,7 @@ public class ClonedMethod implements ITestNGMethod { private final ITestNGMethod m_method; private final Method m_javaMethod; - private String m_id; + private @Nullable String m_id; private int m_currentInvocationCount; private List m_invocationNumbers = new ArrayList<>(); diff --git a/testng-core/src/main/java/org/testng/internal/Configuration.java b/testng-core/src/main/java/org/testng/internal/Configuration.java index 622e6c26b..22879df59 100644 --- a/testng-core/src/main/java/org/testng/internal/Configuration.java +++ b/testng-core/src/main/java/org/testng/internal/Configuration.java @@ -6,6 +6,7 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.ThreadPoolExecutor; +import org.jspecify.annotations.Nullable; import org.testng.IConfigurable; import org.testng.IConfigurationListener; import org.testng.IExecutionListener; @@ -23,11 +24,11 @@ public class Configuration implements IConfiguration { private IAnnotationFinder m_annotationFinder; - private ITestObjectFactory m_objectFactory; - private IHookable m_hookable; - private IConfigurable m_configurable; + private @Nullable ITestObjectFactory m_objectFactory; + private @Nullable IHookable m_hookable; + private @Nullable IConfigurable m_configurable; - private ITestNGListenerFactory m_listenerFactory; + private @Nullable ITestNGListenerFactory m_listenerFactory; private boolean shareThreadPoolForDataProviders = false; private final Map, IExecutionListener> m_executionListeners = @@ -39,7 +40,7 @@ public class Configuration implements IConfiguration { private IInjectorFactory injectorFactory = new GuiceBackedInjectorFactory(); - private ListenerComparator listenerComparator; + private @Nullable ListenerComparator listenerComparator; private boolean overrideIncludedMethods = false; private boolean includeAllDataDrivenTestsWhenSkipping; @@ -51,14 +52,10 @@ public class Configuration implements IConfiguration { private boolean lazyFactoryInstantiation = false; public Configuration() { - init(new JDK15AnnotationFinder(new DefaultAnnotationTransformer())); + this(new JDK15AnnotationFinder(new DefaultAnnotationTransformer())); } public Configuration(IAnnotationFinder finder) { - init(finder); - } - - private void init(IAnnotationFinder finder) { m_annotationFinder = finder; } @@ -73,52 +70,52 @@ public void setAnnotationFinder(IAnnotationFinder finder) { } @Override - public void setListenerFactory(ITestNGListenerFactory testNGListenerFactory) { + public void setListenerFactory(@Nullable ITestNGListenerFactory testNGListenerFactory) { this.m_listenerFactory = testNGListenerFactory; } @Override - public ITestNGListenerFactory getListenerFactory() { + public @Nullable ITestNGListenerFactory getListenerFactory() { return m_listenerFactory; } @Override - public void setListenerComparator(ListenerComparator comparator) { + public void setListenerComparator(@Nullable ListenerComparator comparator) { this.listenerComparator = comparator; } @Override - public ListenerComparator getListenerComparator() { + public @Nullable ListenerComparator getListenerComparator() { return listenerComparator; } @Override - public ITestObjectFactory getObjectFactory() { + public @Nullable ITestObjectFactory getObjectFactory() { return m_objectFactory; } @Override - public void setObjectFactory(ITestObjectFactory factory) { + public void setObjectFactory(@Nullable ITestObjectFactory factory) { m_objectFactory = factory; } @Override - public IHookable getHookable() { + public @Nullable IHookable getHookable() { return m_hookable; } @Override - public void setHookable(IHookable h) { + public void setHookable(@Nullable IHookable h) { m_hookable = h; } @Override - public IConfigurable getConfigurable() { + public @Nullable IConfigurable getConfigurable() { return m_configurable; } @Override - public void setConfigurable(IConfigurable c) { + public void setConfigurable(@Nullable IConfigurable c) { m_configurable = c; } 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 068d7351a..443c30f98 100644 --- a/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java +++ b/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java @@ -7,11 +7,11 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.stream.Collectors; +import org.jspecify.annotations.Nullable; import org.testng.ITestNGMethod; import org.testng.collections.CollectionUtils; import org.testng.log4testng.Logger; @@ -42,7 +42,7 @@ public class ConfigurationGroupMethods { private final ITestNGMethod[] m_allMethods; /** A map that returns the last method belonging to the given group */ - private volatile Map> m_afterGroupsMap = null; + private volatile @Nullable Map> m_afterGroupsMap = null; public ConfigurationGroupMethods( IContainer container, @@ -69,7 +69,6 @@ public List getBeforeGroupMethodsForGroup(String[] groups) { try (AutoCloseableLock ignore = beforeGroups.lock()) { return Arrays.stream(groups) .map(t -> retrieve(beforeGroupsThatHaveAlreadyRun, m_beforeGroupsMethods, t)) - .filter(Objects::nonNull) .flatMap(Collection::stream) .collect(Collectors.toList()); } @@ -89,7 +88,6 @@ public List getAfterGroupMethods(ITestNGMethod testMethod) { return methodGroups.stream() .filter(t -> isLastMethodForGroup(t, testMethod)) .map(t -> retrieve(afterGroupsThatHaveAlreadyRun, m_afterGroupsMethods, t)) - .filter(Objects::nonNull) .flatMap(Collection::stream) .filter(t -> isAfterGroupAllowedToRunAfterTestMethod(t, methodGroups)) .collect(Collectors.toList()); @@ -113,7 +111,10 @@ private boolean isAfterGroupAllowedToRunAfterTestMethod( public void removeBeforeGroups(String[] groups) { for (String group : groups) { m_beforeGroupsMethods.remove(group); - beforeGroupsThatHaveAlreadyRun.get(group).countDown(); + CountDownLatch latch = beforeGroupsThatHaveAlreadyRun.get(group); + if (latch != null) { + latch.countDown(); + } } } @@ -171,7 +172,7 @@ private static List retrieve( return Collections.emptyList(); } tracker.put(group, new CountDownLatch(1)); - return map.get(group); + return map.getOrDefault(group, Collections.emptyList()); } private static List retrieve( @@ -180,6 +181,6 @@ private static List retrieve( return Collections.emptyList(); } tracker.add(group); - return map.get(group); + return map.getOrDefault(group, Collections.emptyList()); } } 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 8d50a65de..5905710e5 100644 --- a/testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java +++ b/testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java @@ -8,6 +8,7 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; +import org.jspecify.annotations.Nullable; import org.testng.ITestNGMethod; import org.testng.ITestObjectFactory; import org.testng.annotations.IAnnotation; @@ -66,7 +67,7 @@ private ConfigurationMethod( String[] beforeGroups, String[] afterGroups, boolean initialize, - IObject.IdentifiableObject instance) { + IObject.@Nullable IdentifiableObject instance) { super(objectFactory, com.getName(), com, annotationFinder, instance); if (initialize) { init(); @@ -105,8 +106,8 @@ public ConfigurationMethod( boolean isIgnoreFailure, String[] beforeGroups, String[] afterGroups, - XmlTest xmlTest, - IObject.IdentifiableObject instance) { + @Nullable XmlTest xmlTest, + IObject.@Nullable IdentifiableObject instance) { this( objectFactory, com, @@ -139,11 +140,11 @@ private static List createMethods( boolean isAfterClass, boolean isBeforeMethod, boolean isAfterMethod, - XmlTest xmlTest, - IObject.IdentifiableObject instance) { + @Nullable XmlTest xmlTest, + IObject.@Nullable IdentifiableObject instance) { List result = new ArrayList<>(); for (ITestNGMethod method : methods) { - if (Modifier.isStatic(method.getConstructorOrMethod().getMethod().getModifiers())) { + if (Modifier.isStatic(method.getConstructorOrMethod().requireMethod().getModifiers())) { String msg = "Detected a static method [" + method.getQualifiedName() @@ -180,7 +181,7 @@ public static List createSuiteConfigurationMethods( ITestNGMethod[] methods, IAnnotationFinder annotationFinder, boolean isBefore, - IObject.IdentifiableObject instance) { + IObject.@Nullable IdentifiableObject instance) { return createMethods( objectFactory, @@ -203,8 +204,8 @@ public static List createTestConfigurationMethods( ITestNGMethod[] methods, IAnnotationFinder annotationFinder, boolean isBefore, - XmlTest xmlTest, - IObject.IdentifiableObject instance) { + @Nullable XmlTest xmlTest, + IObject.@Nullable IdentifiableObject instance) { return createMethods( objectFactory, methods, @@ -226,8 +227,8 @@ public static List createClassConfigurationMethods( ITestNGMethod[] methods, IAnnotationFinder annotationFinder, boolean isBefore, - XmlTest xmlTest, - IObject.IdentifiableObject instance) { + @Nullable XmlTest xmlTest, + IObject.@Nullable IdentifiableObject instance) { return createMethods( objectFactory, methods, @@ -249,7 +250,7 @@ public static ITestNGMethod[] createBeforeConfigurationMethods( ITestNGMethod[] methods, IAnnotationFinder annotationFinder, boolean isBefore, - IObject.IdentifiableObject instance) { + IObject.@Nullable IdentifiableObject instance) { ITestNGMethod[] result = new ITestNGMethod[methods.length]; for (int i = 0; i < methods.length; i++) { result[i] = @@ -280,7 +281,7 @@ public static List createAfterConfigurationMethods( ITestNGMethod[] methods, IAnnotationFinder annotationFinder, boolean isBefore, - IObject.IdentifiableObject instance) { + IObject.@Nullable IdentifiableObject instance) { return Arrays.stream(methods) .parallel() .map( @@ -310,8 +311,8 @@ public static List createTestMethodConfigurationMethods( ITestNGMethod[] methods, IAnnotationFinder annotationFinder, boolean isBefore, - XmlTest xmlTest, - IObject.IdentifiableObject instance) { + @Nullable XmlTest xmlTest, + IObject.@Nullable IdentifiableObject instance) { return createMethods( objectFactory, methods, @@ -402,7 +403,7 @@ private boolean inheritGroupsFromTestClass() { private void init() { IConfigurationAnnotation annotation = - AnnotationHelper.findConfiguration(m_annotationFinder, m_method.getMethod()); + AnnotationHelper.findConfiguration(m_annotationFinder, m_method.requireMethod()); if (annotation != null) { m_inheritGroupsFromTestClass = annotation.getInheritGroups(); setEnabled(annotation.getEnabled()); 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 b4c1a9817..8faad2347 100644 --- a/testng-core/src/main/java/org/testng/internal/DataProviderMethodRemovable.java +++ b/testng-core/src/main/java/org/testng/internal/DataProviderMethodRemovable.java @@ -1,6 +1,7 @@ package org.testng.internal; import java.lang.reflect.Method; +import org.jspecify.annotations.Nullable; import org.testng.annotations.IDataProviderAnnotation; /** Represents an @{@link org.testng.annotations.DataProvider} annotated method. */ @@ -10,11 +11,11 @@ class DataProviderMethodRemovable extends DataProviderMethod { super(instance, method, annotation); } - public void setInstance(Object instance) { + public void setInstance(@Nullable Object instance) { this.instance = instance; } - public void setMethod(Method method) { + public void setMethod(@Nullable Method method) { this.method = method; } } diff --git a/testng-core/src/main/java/org/testng/internal/DefaultListenerFactory.java b/testng-core/src/main/java/org/testng/internal/DefaultListenerFactory.java index 4313ed6ec..b64ea6e35 100644 --- a/testng-core/src/main/java/org/testng/internal/DefaultListenerFactory.java +++ b/testng-core/src/main/java/org/testng/internal/DefaultListenerFactory.java @@ -1,5 +1,6 @@ package org.testng.internal; +import org.jspecify.annotations.Nullable; import org.testng.ITestContext; import org.testng.ITestNGListener; import org.testng.ITestNGListenerFactory; @@ -23,7 +24,7 @@ public DefaultListenerFactory(ITestObjectFactory objectFactory, ITestContext con } @Override - public ITestNGListener createListener(Class listenerClass) { + public @Nullable ITestNGListener createListener(Class listenerClass) { BasicAttributes ba = new BasicAttributes(null, listenerClass); CreationAttributes attributes = new CreationAttributes(context, ba, null); return (ITestNGListener) Dispenser.newInstance(this.m_objectFactory).dispense(attributes); 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 e8b62d177..2bde61e2f 100644 --- a/testng-core/src/main/java/org/testng/internal/DynamicGraph.java +++ b/testng-core/src/main/java/org/testng/internal/DynamicGraph.java @@ -11,6 +11,7 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import org.jspecify.annotations.Nullable; import org.testng.IDynamicGraph; import org.testng.IExecutionVisualiser; @@ -252,6 +253,7 @@ Set fromNodes() { return m_outgoingEdges.keySet(); } + @Nullable Map from(T node) { Map edges = m_outgoingEdges.get(node); return edges == null ? null : Collections.unmodifiableMap(edges); diff --git a/testng-core/src/main/java/org/testng/internal/ExitCode.java b/testng-core/src/main/java/org/testng/internal/ExitCode.java index 7b257eafd..76c8fd4c0 100644 --- a/testng-core/src/main/java/org/testng/internal/ExitCode.java +++ b/testng-core/src/main/java/org/testng/internal/ExitCode.java @@ -1,6 +1,7 @@ package org.testng.internal; import java.util.BitSet; +import org.jspecify.annotations.Nullable; import org.testng.IResultMap; import org.testng.ITestContext; @@ -59,7 +60,8 @@ void computeAndUpdate(ITestContext context) { computeAndUpdate(2, context.getFailedButWithinSuccessPercentageTests(), null); } - private void computeAndUpdate(int index, IResultMap testResults, IResultMap configResults) { + private void computeAndUpdate( + int index, IResultMap testResults, @Nullable IResultMap configResults) { boolean containsResults = testResults.size() != 0; if (configResults != null) { containsResults = containsResults || configResults.size() != 0; diff --git a/testng-core/src/main/java/org/testng/internal/FilteredParameters.java b/testng-core/src/main/java/org/testng/internal/FilteredParameters.java index 5242d32bf..410c267d1 100644 --- a/testng-core/src/main/java/org/testng/internal/FilteredParameters.java +++ b/testng-core/src/main/java/org/testng/internal/FilteredParameters.java @@ -2,6 +2,7 @@ import java.util.Iterator; import java.util.List; +import org.jspecify.annotations.Nullable; import org.testng.ITestNGMethod; import org.testng.TestNGException; @@ -41,7 +42,7 @@ public boolean hasNext() { } @Override - public Object[] next() { + public Object @Nullable [] next() { testMethod.setParameterInvocationCount(index); Object[] next = parameters.next(); if (next == null) { 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 fcc28d75a..65a9dd787 100644 --- a/testng-core/src/main/java/org/testng/internal/Graph.java +++ b/testng-core/src/main/java/org/testng/internal/Graph.java @@ -14,6 +14,7 @@ import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; import org.testng.collections.Maps; import org.testng.log4testng.Logger; @@ -26,7 +27,7 @@ */ public class Graph { private final Map> m_nodes = new LinkedHashMap<>(); - private List m_strictlySortedNodes = null; + private @Nullable List m_strictlySortedNodes = null; private final Comparator> comparator; // A map of nodes that are not the predecessors of any node @@ -52,7 +53,7 @@ public boolean isIndependent(T object) { return m_independentNodes.containsKey(object); } - private Node findNode(T object) { + private @Nullable Node findNode(T object) { return m_nodes.get(object); } @@ -175,7 +176,7 @@ private static void log(Supplier s) { Logger.getLogger(Graph.class).trace("[Graph] " + s.get()); } - private Node findNodeWithNoPredecessors(List> nodes) { + private @Nullable Node findNodeWithNoPredecessors(List> nodes) { return nodes.parallelStream().filter(it -> !it.hasPredecessors()).findFirst().orElse(null); } diff --git a/testng-core/src/main/java/org/testng/internal/IConfiguration.java b/testng-core/src/main/java/org/testng/internal/IConfiguration.java index 407a8d055..103288f09 100644 --- a/testng-core/src/main/java/org/testng/internal/IConfiguration.java +++ b/testng-core/src/main/java/org/testng/internal/IConfiguration.java @@ -1,6 +1,7 @@ package org.testng.internal; import java.util.List; +import org.jspecify.annotations.Nullable; import org.testng.IConfigurable; import org.testng.IConfigurationListener; import org.testng.IExecutionListener; @@ -17,25 +18,30 @@ public interface IConfiguration { void setAnnotationFinder(IAnnotationFinder finder); - void setListenerFactory(ITestNGListenerFactory testNGListenerFactory); + void setListenerFactory(@Nullable ITestNGListenerFactory testNGListenerFactory); + @Nullable ITestNGListenerFactory getListenerFactory(); - void setListenerComparator(ListenerComparator comparator); + void setListenerComparator(@Nullable ListenerComparator comparator); + @Nullable ListenerComparator getListenerComparator(); + @Nullable ITestObjectFactory getObjectFactory(); - void setObjectFactory(ITestObjectFactory m_objectFactory); + void setObjectFactory(@Nullable ITestObjectFactory m_objectFactory); + @Nullable IHookable getHookable(); - void setHookable(IHookable h); + void setHookable(@Nullable IHookable h); + @Nullable IConfigurable getConfigurable(); - void setConfigurable(IConfigurable c); + void setConfigurable(@Nullable IConfigurable c); List getExecutionListeners(); 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 ac1bab6ed..15cab568a 100644 --- a/testng-core/src/main/java/org/testng/internal/IObject.java +++ b/testng-core/src/main/java/org/testng/internal/IObject.java @@ -3,6 +3,7 @@ import java.util.Objects; import java.util.Optional; import java.util.UUID; +import org.jspecify.annotations.Nullable; /** * Represents the associations of a class with one or more instances. Relevant with @Factory @@ -85,7 +86,7 @@ public IdentifiableObject(Object instance, UUID instanceId) { this.instanceId = instanceId; } - public static Object unwrap(IdentifiableObject object) { + public static @Nullable Object unwrap(@Nullable IdentifiableObject object) { if (object == null) { return null; } diff --git a/testng-core/src/main/java/org/testng/internal/LazyParameterInfo.java b/testng-core/src/main/java/org/testng/internal/LazyParameterInfo.java index 5c8ba9739..d7e18d321 100644 --- a/testng-core/src/main/java/org/testng/internal/LazyParameterInfo.java +++ b/testng-core/src/main/java/org/testng/internal/LazyParameterInfo.java @@ -1,6 +1,7 @@ package org.testng.internal; import java.util.function.Supplier; +import org.jspecify.annotations.Nullable; import org.testng.IFactoryInstance; /** @@ -22,8 +23,8 @@ public class LazyParameterInfo implements IParameterInfo { private final Object lock = new Object(); private volatile boolean instantiated = false; - private volatile Object instance; - private volatile Throwable failure; + private volatile @Nullable Object instance; + private volatile @Nullable Throwable failure; public LazyParameterInfo( FactoryInstance factoryInstance, Class targetClass, Supplier creator) { diff --git a/testng-core/src/main/java/org/testng/internal/ListenerOrderDeterminer.java b/testng-core/src/main/java/org/testng/internal/ListenerOrderDeterminer.java index 5fbd11d28..a4869f855 100644 --- a/testng-core/src/main/java/org/testng/internal/ListenerOrderDeterminer.java +++ b/testng-core/src/main/java/org/testng/internal/ListenerOrderDeterminer.java @@ -9,6 +9,7 @@ import java.util.Objects; import java.util.function.Predicate; import java.util.stream.Collectors; +import org.jspecify.annotations.Nullable; import org.testng.ITestNGListener; import org.testng.ListenerComparator; import org.testng.collections.Lists; @@ -48,7 +49,7 @@ private ListenerOrderDeterminer() { * @return - A re-ordered collection wherein preferential listeners are added at the end */ public static List order( - Collection original, ListenerComparator comparator) { + Collection original, @Nullable ListenerComparator comparator) { original = sort(original, comparator); Pair, List> ordered = arrange(original); List ideListeners = ordered.first(); @@ -62,7 +63,7 @@ public static List order( * followed by preferential listeners also in reverse order. */ public static List reversedOrder( - Collection original, ListenerComparator comparator) { + Collection original, @Nullable ListenerComparator comparator) { original = sort(original, comparator); Pair, List> ordered = arrange(original); List preferentialListeners = ordered.first(); 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 3897ff0e0..db5dd38d3 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodHelper.java +++ b/testng-core/src/main/java/org/testng/internal/MethodHelper.java @@ -17,6 +17,7 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.jspecify.annotations.Nullable; import org.testng.IMethodInstance; import org.testng.ITestClass; import org.testng.ITestNGMethod; @@ -270,7 +271,7 @@ public static boolean isEnabled(Method m, IAnnotationFinder finder) { return isEnabled(annotation); } - public static boolean isEnabled(ITestOrConfiguration test) { + public static boolean isEnabled(@Nullable ITestOrConfiguration test) { return null == test || test.getEnabled(); } @@ -425,7 +426,7 @@ private static Map> sortMethodsByInstance(ITestNGMet } protected static String calculateMethodCanonicalName(ITestNGMethod m) { - return calculateMethodCanonicalName(m.getConstructorOrMethod().getMethod()); + return calculateMethodCanonicalName(m.getConstructorOrMethod().requireMethod()); } private static String calculateMethodCanonicalName(Method m) { 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 bc031b965..be25b7847 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodInheritance.java +++ b/testng-core/src/main/java/org/testng/internal/MethodInheritance.java @@ -9,6 +9,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Objects; +import org.jspecify.annotations.Nullable; import org.testng.ITestNGMethod; public class MethodInheritance { @@ -55,7 +56,7 @@ public class MethodInheritance { }; /** Look in map for a class that is a superclass of methodClass */ - private static List findMethodListSuperClass( + private static @Nullable List findMethodListSuperClass( Map, List> map, Class methodClass) { return map.entrySet().stream() .parallel() diff --git a/testng-core/src/main/java/org/testng/internal/NoOpTestClass.java b/testng-core/src/main/java/org/testng/internal/NoOpTestClass.java index bb3feb3bb..a18805988 100644 --- a/testng-core/src/main/java/org/testng/internal/NoOpTestClass.java +++ b/testng-core/src/main/java/org/testng/internal/NoOpTestClass.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.List; +import org.jspecify.annotations.Nullable; import org.testng.ITestClass; import org.testng.ITestNGMethod; import org.testng.xml.XmlClass; @@ -9,7 +10,7 @@ public class NoOpTestClass implements ITestClass, IObject { - protected Class m_testClass = null; + protected @Nullable Class m_testClass = null; // Test methods protected List m_beforeClassMethods = new ArrayList<>(); @@ -24,11 +25,11 @@ public class NoOpTestClass implements ITestClass, IObject { protected ITestNGMethod[] m_beforeGroupsMethods = new ITestNGMethod[0]; protected List m_afterGroupsMethods = new ArrayList<>(); - private final IdentifiableObject[] m_instances; - private final long[] m_instanceHashes; + private final IdentifiableObject @Nullable [] m_instances; + private final long @Nullable [] m_instanceHashes; - private final XmlTest m_xmlTest; - private final XmlClass m_xmlClass; + private final @Nullable XmlTest m_xmlTest; + private final @Nullable XmlClass m_xmlClass; protected NoOpTestClass() { m_instances = null; @@ -162,13 +163,12 @@ public void setTestClass(Class declaringClass) { } @Override - public String getTestName() { - // TODO Auto-generated method stub + public @Nullable String getTestName() { return null; } @Override - public XmlTest getXmlTest() { + public @Nullable XmlTest getXmlTest() { return m_xmlTest; } 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 c2228ba2f..fb119cbfb 100644 --- a/testng-core/src/main/java/org/testng/internal/Parameters.java +++ b/testng-core/src/main/java/org/testng/internal/Parameters.java @@ -12,6 +12,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import org.jspecify.annotations.Nullable; import org.testng.DataProviderHolder; import org.testng.IDataProviderInterceptor; import org.testng.IDataProviderListener; @@ -959,10 +960,10 @@ public static Object[] injectParameters( /** A parameter passing helper class. */ public static class MethodParameters { private final Map xmlParameters; - private final Method currentTestMethod; - private final ITestContext context; - private final Object[] parameterValues; - private final ITestResult testResult; + private final @Nullable Method currentTestMethod; + private final @Nullable ITestContext context; + private final Object @Nullable [] parameterValues; + private final @Nullable ITestResult testResult; public MethodParameters(Map params, Map methodParams) { this(params, methodParams, null, null, null, null); @@ -987,10 +988,10 @@ public static MethodParameters newInstance( public MethodParameters( Map params, Map methodParams, - Object[] pv, - Method m, - ITestContext ctx, - ITestResult tr) { + Object @Nullable [] pv, + @Nullable Method m, + @Nullable ITestContext ctx, + @Nullable ITestResult tr) { Map allParams = new HashMap<>(); allParams.putAll(params); allParams.putAll(methodParams); diff --git a/testng-core/src/main/java/org/testng/internal/Tarjan.java b/testng-core/src/main/java/org/testng/internal/Tarjan.java index 8a383c7d5..98a1f9ad7 100644 --- a/testng-core/src/main/java/org/testng/internal/Tarjan.java +++ b/testng-core/src/main/java/org/testng/internal/Tarjan.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import org.jspecify.annotations.Nullable; /** * Implementation of the Tarjan algorithm to find and display a cycle in a graph. @@ -17,7 +18,7 @@ public class Tarjan { private final ArrayDeque stack; Map visitedNodes = new HashMap<>(); Map m_lowlinks = new HashMap<>(); - private List m_cycle; + private @Nullable List m_cycle; public Tarjan(Graph graph, T start) { stack = new ArrayDeque<>(); diff --git a/testng-core/src/main/java/org/testng/internal/TestListenerHelper.java b/testng-core/src/main/java/org/testng/internal/TestListenerHelper.java index c585193ab..729eb149c 100644 --- a/testng-core/src/main/java/org/testng/internal/TestListenerHelper.java +++ b/testng-core/src/main/java/org/testng/internal/TestListenerHelper.java @@ -4,6 +4,7 @@ import java.util.Arrays; import java.util.List; import java.util.Optional; +import org.jspecify.annotations.Nullable; import org.testng.IClass; import org.testng.IConfigurationListener; import org.testng.ITestContext; @@ -26,10 +27,10 @@ private TestListenerHelper() { public static void runPreConfigurationListeners( ITestResult tr, - ITestNGMethod tm, + @Nullable ITestNGMethod tm, List listeners, IConfigurationListener internal, - ListenerComparator comparator) { + @Nullable ListenerComparator comparator) { internal.beforeConfiguration(tr); List original = ListenerOrderDeterminer.order(listeners, comparator); @@ -45,10 +46,10 @@ public static void runPreConfigurationListeners( public static void runPostConfigurationListeners( ITestResult tr, - ITestNGMethod tm, + @Nullable ITestNGMethod tm, List listeners, IConfigurationListener internal, - ListenerComparator comparator) { + @Nullable ListenerComparator comparator) { List listenersreversed = ListenerOrderDeterminer.reversedOrder(listeners, comparator); listenersreversed.add(internal); diff --git a/testng-core/src/main/java/org/testng/internal/TestMethodContainer.java b/testng-core/src/main/java/org/testng/internal/TestMethodContainer.java index 3c2cbccf1..ab41ebce3 100644 --- a/testng-core/src/main/java/org/testng/internal/TestMethodContainer.java +++ b/testng-core/src/main/java/org/testng/internal/TestMethodContainer.java @@ -2,6 +2,7 @@ import java.util.Arrays; import java.util.function.Supplier; +import org.jspecify.annotations.Nullable; import org.testng.ITestNGMethod; /** @@ -13,7 +14,7 @@ */ public final class TestMethodContainer implements IContainer { - private ITestNGMethod[] methods; + private ITestNGMethod @Nullable [] methods; private final Supplier supplier; private boolean isCleared = false; diff --git a/testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java b/testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java index a74ef1229..e2312082f 100644 --- a/testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java +++ b/testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java @@ -13,6 +13,7 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import org.jspecify.annotations.Nullable; import org.testng.DataProviderHolder; import org.testng.IClass; import org.testng.IInstanceInfo; @@ -39,7 +40,7 @@ public class TestNGClassFinder extends BaseClassFinder { private final ITestObjectFactory objectFactory; private final IAnnotationFinder annotationFinder; - private String m_factoryCreationFailedMessage = null; + private @Nullable String m_factoryCreationFailedMessage = null; public String getFactoryCreationFailedMessage() { return m_factoryCreationFailedMessage; @@ -198,7 +199,7 @@ private ClassInfoMap processFactory( oneMoreClass = o.getTargetClass(); } else { Object objToInspect = o.getInstance(); - if (IInstanceInfo.class.isAssignableFrom(objToInspect.getClass())) { + if (objToInspect instanceof IInstanceInfo) { IInstanceInfo ii = (IInstanceInfo) objToInspect; addInstance(ii); oneMoreClass = ii.getInstanceClass(); @@ -347,7 +348,12 @@ private void addInstance(IObject.IdentifiableObject o) { if (wrapped instanceof IParameterInfo) { // Use the target class, which a lazy IParameterInfo can answer without instantiating its // (not-yet-created) instance. - key = ((IParameterInfo) wrapped).getTargetClass(); + // A lazy IParameterInfo whose construction failed has no target class; fall back to the + // wrapper's own class rather than dropping the instance. + Class targetClass = ((IParameterInfo) wrapped).getTargetClass(); + if (targetClass != null) { + key = targetClass; + } } addInstance(key, o); } 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 a3e9b6cde..2a1d0e970 100644 --- a/testng-core/src/main/java/org/testng/internal/TestNGMethod.java +++ b/testng-core/src/main/java/org/testng/internal/TestNGMethod.java @@ -5,6 +5,7 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import org.jspecify.annotations.Nullable; import org.testng.IDataProviderMethod; import org.testng.ITestClass; import org.testng.ITestNGMethod; @@ -33,7 +34,7 @@ public TestNGMethod( Method method, IAnnotationFinder finder, XmlTest xmlTest, - IObject.IdentifiableObject instance) { + IObject.@Nullable IdentifiableObject instance) { this(objectFactory, method, finder, true, xmlTest, instance); } @@ -43,7 +44,7 @@ private TestNGMethod( IAnnotationFinder finder, boolean initialize, XmlTest xmlTest, - IObject.IdentifiableObject instance) { + IObject.@Nullable IdentifiableObject instance) { super(objectFactory, method.getName(), new ConstructorOrMethod(method), finder, instance); setXmlTest(xmlTest); @@ -86,7 +87,7 @@ private void init(XmlTest xmlTest) { setInvocationNumbers(xmlTest.getInvocationNumbers(className + "." + m_method.getName())); ITestAnnotation testAnnotation = - AnnotationHelper.findTest(getAnnotationFinder(), m_method.getMethod()); + AnnotationHelper.findTest(getAnnotationFinder(), m_method.requireMethod()); if (testAnnotation == null) { // Try on the class @@ -138,7 +139,7 @@ private String findDescription(ITestAnnotation testAnnotation, XmlTest xmlTest) } private boolean classNameMatcher(XmlClass xmlClass) { - return xmlClass.getName().equals(m_method.getMethod().getDeclaringClass().getName()); + return xmlClass.getName().equals(m_method.getDeclaringClass().getName()); } private boolean methodNameMatcher(XmlInclude xmlInclude) { @@ -173,7 +174,7 @@ public BaseTestMethod clone() { TestNGMethod clone = new TestNGMethod( m_objectFactory, - getConstructorOrMethod().getMethod(), + getConstructorOrMethod().requireMethod(), getAnnotationFinder(), false, getXmlTest(), @@ -226,7 +227,7 @@ public IDataProviderMethod getDataProviderMethod() { return dataProviderMethod; } - public void setDataProviderMethod(IDataProviderMethod dataProviderMethod) { + public void setDataProviderMethod(@Nullable IDataProviderMethod dataProviderMethod) { this.dataProviderMethod = dataProviderMethod; } } diff --git a/testng-core/src/main/java/org/testng/internal/XmlMethodSelector.java b/testng-core/src/main/java/org/testng/internal/XmlMethodSelector.java index a8e1de339..18a5b2b0e 100644 --- a/testng-core/src/main/java/org/testng/internal/XmlMethodSelector.java +++ b/testng-core/src/main/java/org/testng/internal/XmlMethodSelector.java @@ -14,6 +14,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Stream; +import org.jspecify.annotations.Nullable; import org.testng.IFactoryInstance; import org.testng.IMethodSelector; import org.testng.IMethodSelectorContext; @@ -44,7 +45,7 @@ public class XmlMethodSelector implements IMethodSelector { private Map m_includedGroups = new HashMap<>(); private Map m_excludedGroups = new HashMap<>(); private List m_classes = Collections.emptyList(); - private ScriptMethodSelector scriptSelector; + private @Nullable ScriptMethodSelector scriptSelector; private boolean m_isInitialized = false; private List m_testMethods = Collections.emptyList(); 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 48a89499b..d654f6af4 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 @@ -340,7 +340,7 @@ public void invokeConfigurations(ConfigMethodArguments arguments) { parameters = Parameters.createConfigurationParameters( - tm.getConstructorOrMethod().getMethod(), + tm.getConstructorOrMethod().requireMethod(), arguments.getParameters(), arguments.getParameterValues(), arguments.getTestMethod(), @@ -415,11 +415,10 @@ private void invokeConfigurationMethod( return; } boolean willfullyIgnored = false; - boolean usesConfigurableInstance = configurableInstance != null; - if (usesConfigurableInstance) { + if (configurableInstance != null) { willfullyIgnored = !MethodInvocationHelper.invokeConfigurable( - targetInstance, params, configurableInstance, method.getMethod(), testResult); + targetInstance, params, configurableInstance, method.requireMethod(), testResult); } else { MethodInvocationHelper.invokeMethodConsideringTimeout( tm, method, targetInstance, params, testResult, m_configuration); @@ -427,7 +426,7 @@ private void invokeConfigurationMethod( boolean testStatusRemainedUnchanged = testResult.isNotRunning(); boolean throwException = !RuntimeBehavior.ignoreCallbackInvocationSkips(); if (throwException - && usesConfigurableInstance + && configurableInstance != null && willfullyIgnored && testStatusRemainedUnchanged) { throw new ConfigurationNotInvokedException(tm); @@ -456,7 +455,7 @@ private void throwConfigurationFailure(ITestResult testResult, Throwable ex) { testResult.setThrowable(ex.getCause() == null ? ex : ex.getCause()); } - private IConfigurable computeConfigurableInstance( + private @Nullable IConfigurable computeConfigurableInstance( ConstructorOrMethod method, Object targetInstance) { return IConfigurable.class.isAssignableFrom(method.getDeclaringClass()) ? (IConfigurable) targetInstance 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 5005c8cac..6d23e4ce9 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 @@ -51,11 +51,11 @@ private boolean runOne() { ConstructorOrMethod m = m_method.getConstructorOrMethod(); if (m_hookable == null) { invoked = true; - MethodInvocationHelper.invokeMethod(m.getMethod(), m_instance, m_parameters); + MethodInvocationHelper.invokeMethod(m.requireMethod(), m_instance, m_parameters); } else { invoked = MethodInvocationHelper.invokeHookable( - m_instance, m_parameters, m_hookable, m.getMethod(), m_testResult); + m_instance, m_parameters, m_hookable, m.requireMethod(), m_testResult); } } catch (Throwable e) { invoked = true; 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 3d59739f4..dc29ff891 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 @@ -85,7 +85,7 @@ protected static void invokeMethodConsideringTimeout( IConfiguration config) throws Throwable { if (MethodHelper.calculateTimeOut(tm) <= 0) { - MethodInvocationHelper.invokeMethod(method.getMethod(), targetInstance, params); + MethodInvocationHelper.invokeMethod(method.requireMethod(), targetInstance, params); } else { MethodInvocationHelper.invokeWithTimeout(config, tm, targetInstance, params, testResult); if (!testResult.isSuccess()) { diff --git a/testng-core/src/main/java/org/testng/internal/invokers/MethodRunner.java b/testng-core/src/main/java/org/testng/internal/invokers/MethodRunner.java index 48af3d33b..024bddb21 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/MethodRunner.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/MethodRunner.java @@ -51,7 +51,7 @@ public List runInSequence( } Object[] parameterValues = Parameters.injectParameters( - next, arguments.getTestMethod().getConstructorOrMethod().getMethod(), context); + next, arguments.getTestMethod().getConstructorOrMethod().requireMethod(), context); List tmpResults = new ArrayList<>(); int tmpResultsIndex = -1; @@ -125,7 +125,7 @@ public List runInParallel( } Object[] parameterValues = Parameters.injectParameters( - next, arguments.getTestMethod().getConstructorOrMethod().getMethod(), context); + next, arguments.getTestMethod().getConstructorOrMethod().requireMethod(), context); workers.add( new TestMethodWithDataProviderMethodWorker( 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 eb7961db1..8fa6c3891 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 @@ -19,13 +19,13 @@ import org.testng.xml.XmlSuite; class ParameterHandler { - private final ITestObjectFactory objectFactory; + private final @Nullable ITestObjectFactory objectFactory; private final IAnnotationFinder finder; private final DataProviderHolder holder; private int verbose; ParameterHandler( - ITestObjectFactory objectFactory, + @Nullable ITestObjectFactory objectFactory, IAnnotationFinder finder, DataProviderHolder holder, int verbose) { @@ -125,9 +125,12 @@ boolean hasErrors() { } boolean runInParallel() { - return (parameterHolder != null) - && (parameterHolder.origin == ParameterHolder.ParameterOrigin.ORIGIN_DATA_PROVIDER - && parameterHolder.dataProviderHolder.isParallel()); + if (parameterHolder == null + || parameterHolder.origin != ParameterHolder.ParameterOrigin.ORIGIN_DATA_PROVIDER) { + return false; + } + IDataProviderMethod dataProvider = parameterHolder.dataProviderHolder; + return dataProvider != null && dataProvider.isParallel(); } boolean isBubbleUpFailures() { 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 dadab3636..9a6ed980e 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 @@ -17,7 +17,7 @@ public enum ParameterOrigin { NATIVE // Native injection is involved. } - final IDataProviderMethod dataProviderHolder; + final @Nullable IDataProviderMethod dataProviderHolder; public final Iterator parameters; final ParameterOrigin origin; @@ -31,14 +31,14 @@ public enum ParameterOrigin { private final @Nullable CloseableIterator closeableSource; public ParameterHolder( - Iterator parameters, ParameterOrigin origin, IDataProviderMethod dph) { + Iterator parameters, ParameterOrigin origin, @Nullable IDataProviderMethod dph) { this(parameters, origin, dph, null); } public ParameterHolder( Iterator parameters, ParameterOrigin origin, - IDataProviderMethod dph, + @Nullable IDataProviderMethod dph, @Nullable CloseableIterator closeableSource) { super(); this.parameters = parameters; 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 775c1a04b..dd1646681 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 @@ -116,7 +116,7 @@ public List invokeTestMethods( } if (!MethodHelper.isEnabled( - testMethod.getConstructorOrMethod().getMethod(), annotationFinder())) { + testMethod.getConstructorOrMethod().requireMethod(), annotationFinder())) { // return if the method is not enabled. No need to do any more calculations return Collections.emptyList(); } @@ -781,9 +781,8 @@ private ITestResult invokeMethod( : m_configuration.getHookable(); boolean willfullyIgnored = false; - boolean usesHookableInstance = hookableInstance != null; if (MethodHelper.calculateTimeOut(arguments.getTestMethod()) <= 0) { - if (usesHookableInstance) { + if (hookableInstance != null) { willfullyIgnored = !MethodInvocationHelper.invokeHookable( arguments.getInstance(), @@ -812,7 +811,7 @@ private ITestResult invokeMethod( boolean testStatusRemainedUnchanged = testResult.isNotRunning(); boolean throwException = !RuntimeBehavior.ignoreCallbackInvocationSkips(); if (throwException - && usesHookableInstance + && hookableInstance != null && willfullyIgnored && testStatusRemainedUnchanged) { TestNotInvokedException tn = new TestNotInvokedException(arguments.tm); diff --git a/testng-core/src/main/java/org/testng/reporters/FailedReporter.java b/testng-core/src/main/java/org/testng/reporters/FailedReporter.java index 16fa63442..3dd947154 100644 --- a/testng-core/src/main/java/org/testng/reporters/FailedReporter.java +++ b/testng-core/src/main/java/org/testng/reporters/FailedReporter.java @@ -226,10 +226,10 @@ private static void getAllGroupApplicableConfigs( method -> relevantConfigs.stream() .map(ITestNGMethod::getConstructorOrMethod) - .map(ConstructorOrMethod::getMethod) + .map(ConstructorOrMethod::requireMethod) .noneMatch( configMethod -> - method.getConstructorOrMethod().getMethod().equals(configMethod))) + method.getConstructorOrMethod().requireMethod().equals(configMethod))) .filter(method -> method.getGroups().length > 0) .filter( method -> diff --git a/testng-runner-api/src/main/java/org/testng/internal/Attributes.java b/testng-runner-api/src/main/java/org/testng/internal/Attributes.java index 60fe87e54..d0599da60 100644 --- a/testng-runner-api/src/main/java/org/testng/internal/Attributes.java +++ b/testng-runner-api/src/main/java/org/testng/internal/Attributes.java @@ -3,6 +3,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import org.jspecify.annotations.Nullable; import org.testng.IAttributes; /** Simple implementation of IAttributes. */ @@ -11,7 +12,7 @@ public class Attributes implements IAttributes { private final Map m_attributes = new ConcurrentHashMap<>(); @Override - public Object getAttribute(String name) { + public @Nullable Object getAttribute(String name) { return m_attributes.get(name); } @@ -26,7 +27,7 @@ public void setAttribute(String name, Object value) { } @Override - public Object removeAttribute(String name) { + public @Nullable Object removeAttribute(String name) { return m_attributes.remove(name); } } diff --git a/testng-runner-api/src/main/java/org/testng/internal/LiteWeightTestNGMethod.java b/testng-runner-api/src/main/java/org/testng/internal/LiteWeightTestNGMethod.java index d038e98f7..b4090150b 100644 --- a/testng-runner-api/src/main/java/org/testng/internal/LiteWeightTestNGMethod.java +++ b/testng-runner-api/src/main/java/org/testng/internal/LiteWeightTestNGMethod.java @@ -7,6 +7,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.Callable; +import org.jspecify.annotations.Nullable; import org.testng.IClass; import org.testng.IDataProviderMethod; import org.testng.IFactoryInstance; @@ -44,7 +45,7 @@ public class LiteWeightTestNGMethod implements ITestNGMethod { private final boolean isAfterClassConfiguration; private final boolean isBeforeSuiteConfiguration; private final boolean isAfterSuiteConfiguration; - private final IFactoryInstance factoryInstance; + private final @Nullable IFactoryInstance factoryInstance; private List invocationNumbers; private final List failedInvocationNumbers; private boolean ignoreMissingDependencies; 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 f573c534e..ab599161b 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 @@ -10,6 +10,7 @@ import java.util.UUID; import java.util.regex.Pattern; import java.util.stream.Collectors; +import org.jspecify.annotations.Nullable; import org.testng.IAttributes; import org.testng.IClass; import org.testng.IFactoryInstance; @@ -27,18 +28,18 @@ public class TestResult implements ITestResult { private static final Object[] NO_FACTORY_PARAMETERS = {}; - private ITestNGMethod m_method = null; + private @Nullable ITestNGMethod m_method = null; private List skippedDueTo = new ArrayList<>(); private boolean skipAnalysed = false; private int m_status = CREATED; - private Throwable m_throwable = null; + private @Nullable Throwable m_throwable = null; private long m_startMillis = 0; private long m_endMillis = 0; - private String m_name = null; - private String m_host; + private @Nullable String m_name = null; + private @Nullable String m_host; private Object[] m_parameters = {}; - private String m_instanceName; - private ITestContext m_context; + private @Nullable String m_instanceName; + private @Nullable ITestContext m_context; private int m_parameterIndex = -1; private boolean m_wasRetried; private final IAttributes m_attributes = new Attributes(); @@ -63,7 +64,8 @@ public static TestResult newTestResultFor(ITestNGMethod method) { return newContextAwareTestResult(method, null); } - public static TestResult newContextAwareTestResult(ITestNGMethod method, ITestContext ctx) { + public static TestResult newContextAwareTestResult( + ITestNGMethod method, @Nullable ITestContext ctx) { TestResult result = newEmptyTestResult(); long time = System.currentTimeMillis(); result.init(method, ctx, null, time, 0L); @@ -71,7 +73,7 @@ public static TestResult newContextAwareTestResult(ITestNGMethod method, ITestCo } public static TestResult newTestResultWithCauseAs( - ITestNGMethod method, ITestContext ctx, Throwable t) { + ITestNGMethod method, @Nullable ITestContext ctx, Throwable t) { TestResult result = newEmptyTestResult(); long time = System.currentTimeMillis(); result.init(method, ctx, t, time, time); @@ -79,7 +81,7 @@ public static TestResult newTestResultWithCauseAs( } public static TestResult newEndTimeAwareTestResult( - ITestNGMethod method, ITestContext ctx, Throwable t, long start) { + ITestNGMethod method, @Nullable ITestContext ctx, @Nullable Throwable t, long start) { TestResult result = newEmptyTestResult(); long time = System.currentTimeMillis(); result.init(method, ctx, t, start, time); @@ -87,7 +89,7 @@ public static TestResult newEndTimeAwareTestResult( } public static TestResult newTestResultFrom( - TestResult result, ITestNGMethod method, ITestContext ctx, long start) { + TestResult result, ITestNGMethod method, @Nullable ITestContext ctx, long start) { TestResult testResult = TestResult.newTestResult(result.getParameters(), result.getParameterIndex()); testResult.setHost(result.getHost()); @@ -96,7 +98,12 @@ public static TestResult newTestResultFrom( return testResult; } - private void init(ITestNGMethod method, ITestContext ctx, Throwable t, long start, long end) { + private void init( + ITestNGMethod method, + @Nullable ITestContext ctx, + @Nullable Throwable t, + long start, + long end) { m_throwable = t; m_instanceName = method.getTestClass().getName(); if (null == m_throwable) { @@ -104,11 +111,7 @@ private void init(ITestNGMethod method, ITestContext ctx, Throwable t, long star } m_startMillis = start; m_endMillis = end; - if (RuntimeBehavior.isMemoryFriendlyMode()) { - m_method = new LiteWeightTestNGMethod(method); - } else { - m_method = method; - } + m_method = RuntimeBehavior.isMemoryFriendlyMode() ? new LiteWeightTestNGMethod(method) : method; m_context = ctx; Object instance = method.getInstance(); @@ -116,7 +119,7 @@ private void init(ITestNGMethod method, ITestContext ctx, Throwable t, long star // Calculate the name: either the method name, ITest#getTestName or // toString() if it's been overridden. if (instance == null) { - m_name = m_method.getMethodName(); + m_name = method.getMethodName(); return; } if (instance instanceof ITest) { @@ -124,7 +127,7 @@ private void init(ITestNGMethod method, ITestContext ctx, Throwable t, long star if (m_name != null) { return; } - m_name = m_method.getMethodName(); + m_name = method.getMethodName(); if (Utils.getVerbose() > 1) { String msg = String.format( @@ -140,7 +143,7 @@ private void init(ITestNGMethod method, ITestContext ctx, Throwable t, long star } String string = instance.toString(); // Only display toString() if it's been overridden by the user - m_name = getMethod().getMethodName(); + m_name = method.getMethodName(); try { if (!Object.class.getMethod("toString").equals(instance.getClass().getMethod("toString"))) { m_instanceName = string.startsWith("class ") ? string.substring("class ".length()) : string; @@ -161,7 +164,7 @@ public void setEndMillis(long millis) { * name, otherwise returns null. */ @Override - public String getTestName() { + public @Nullable String getTestName() { if (this.m_method == null) { return null; } @@ -176,18 +179,28 @@ public String getTestName() { } @Override - public String getName() { + public @Nullable String getName() { return m_name; } /** @return Returns the method. */ @Override - public ITestNGMethod getMethod() { + public @Nullable ITestNGMethod getMethod() { return m_method; } + /** + * The method this result belongs to, for the members that only make sense on a result built + * through one of the method-aware factories. {@link #newTestResult(Object[], int)} deliberately + * builds a carrier that has no method, and those members are not reachable on it. + */ + private ITestNGMethod requireMethod() { + return java.util.Objects.requireNonNull( + m_method, "This TestResult carries parameters only; it has no test method"); + } + /** @param method The method to set. */ - public void setMethod(ITestNGMethod method) { + public void setMethod(@Nullable ITestNGMethod method) { m_method = method; } @@ -211,18 +224,18 @@ public boolean isSuccess() { /** @return Returns the testClass. */ @Override public IClass getTestClass() { - return m_method.getTestClass(); + return requireMethod().getTestClass(); } /** @return Returns the throwable. */ @Override - public Throwable getThrowable() { + public @Nullable Throwable getThrowable() { return m_throwable; } /** @param throwable The throwable to set. */ @Override - public void setThrowable(Throwable throwable) { + public void setThrowable(@Nullable Throwable throwable) { m_throwable = throwable; } @@ -271,11 +284,11 @@ private static String toString(int status) { } @Override - public String getHost() { + public @Nullable String getHost() { return m_host; } - public void setHost(String host) { + public void setHost(@Nullable String host) { m_host = host; } @@ -306,13 +319,13 @@ public void setParameters(Object[] parameters) { } @Override - public Object getInstance() { - return IParameterInfo.embeddedInstance(this.m_method.getInstance()); + public @Nullable Object getInstance() { + return IParameterInfo.embeddedInstance(requireMethod().getInstance()); } @Override public Object[] getFactoryParameters() { - return this.m_method + return requireMethod() .getFactoryInstance() .map(IFactoryInstance::getParameters) .orElse(NO_FACTORY_PARAMETERS); @@ -339,11 +352,11 @@ public Object removeAttribute(String name) { } @Override - public ITestContext getTestContext() { + public @Nullable ITestContext getTestContext() { return m_context; } - public void setContext(ITestContext context) { + public void setContext(@Nullable ITestContext context) { m_context = context; } @@ -353,7 +366,7 @@ public int compareTo(ITestResult comparison) { } @Override - public String getInstanceName() { + public @Nullable String getInstanceName() { return m_instanceName; } @@ -391,8 +404,12 @@ public List getSkipCausedBy() { return Collections.unmodifiableList(skippedDueTo); } skipAnalysed = true; + ITestContext context = m_context; + if (context == null) { + return Collections.unmodifiableList(skippedDueTo); + } // check if there were any config failures - Set skippedConfigs = m_context.getFailedConfigurations().getAllResults(); + Set skippedConfigs = context.getFailedConfigurations().getAllResults(); for (ITestResult skippedConfig : skippedConfigs) { if (isGlobalFailure(skippedConfig) || isRelated(skippedConfig)) { // If there's a failure in @BeforeTest/@BeforeSuite/@BeforeClass @@ -410,7 +427,7 @@ public List getSkipCausedBy() { return Collections.unmodifiableList(skippedDueTo); } // Looks like we didn't have any configuration failures. So some upstream method perhaps failed. - if (m_method.getMethodsDependedUpon().length == 0) { + if (requireMethod().getMethodsDependedUpon().length == 0) { // Maybe group dependencies exist ? if (m_method.getGroupsDependedUpon().length == 0) { return Collections.emptyList(); @@ -434,7 +451,7 @@ public List getSkipCausedBy() { return Collections.unmodifiableList(skippedDueTo); } - List upstreamMethods = Arrays.asList(m_method.getMethodsDependedUpon()); + List upstreamMethods = Arrays.asList(requireMethod().getMethodsDependedUpon()); // So we have dependsOnMethod failures List allFailures = @@ -479,6 +496,9 @@ private boolean isRelated(ITestResult result) { } Object current = this.getInstance(); Object thatObject = result.getInstance(); + if (current == null || thatObject == null) { + return false; + } return current.getClass().isAssignableFrom(thatObject.getClass()) || thatObject.getClass().isAssignableFrom(current.getClass()); } @@ -488,7 +508,7 @@ private boolean belongToSameGroup(ITestResult result) { if (!m.isBeforeGroupsConfiguration()) { return false; } - String[] myGroups = this.m_method.getGroups(); + String[] myGroups = requireMethod().getGroups(); if (myGroups.length == 0 || m.getGroups().length == 0) { return false; } diff --git a/testng-yaml/src/main/java/org/testng/internal/Yaml.java b/testng-yaml/src/main/java/org/testng/internal/Yaml.java index 1c5cf08a7..3920e06a2 100644 --- a/testng-yaml/src/main/java/org/testng/internal/Yaml.java +++ b/testng-yaml/src/main/java/org/testng/internal/Yaml.java @@ -9,6 +9,7 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; import org.testng.xml.XmlClass; import org.testng.xml.XmlDefine; @@ -40,7 +41,7 @@ private Yaml() {} * @throws FileNotFoundException if {@code is} is null and {@code filePath} does not exist * @throws TestNGException if the document is malformed or uses a key outside the schema */ - public static XmlSuite parse(String filePath, InputStream is, boolean loadClasses) + public static XmlSuite parse(String filePath, @Nullable InputStream is, boolean loadClasses) throws FileNotFoundException { org.yaml.snakeyaml.Yaml y = new org.yaml.snakeyaml.Yaml(YamlSchema.constructor(loadClasses)); if (is == null) { @@ -213,7 +214,7 @@ private static Map testToMap(XmlTest test) { * getIncludedGroups()}: on a test that getter returns the union with the suite's groups, and on a * suite it delegates to the parent suite. Either one would duplicate groups on the way out. */ - private static void putRunGroups(Map result, XmlGroups groups) { + private static void putRunGroups(Map result, @Nullable XmlGroups groups) { if (groups == null || groups.getRun() == null) { return; } @@ -226,7 +227,7 @@ private static void putRunGroups(Map result, XmlGroups groups) { * so a suite level {@code } has no key to be read back through and writing one would make * the file unloadable. */ - private static void putMetaGroups(Map result, XmlGroups groups) { + private static void putMetaGroups(Map result, @Nullable XmlGroups groups) { if (groups == null) { return; } @@ -252,7 +253,7 @@ private static List packagesToNodes(List packages) { *

{@code getXmlClasses()} is deliberately not called: it scans the classpath, which has * nothing to do with what the suite file says. */ - private static Object packageToNode(XmlPackage xmlPackage) { + private static @Nullable Object packageToNode(XmlPackage xmlPackage) { List include = xmlPackage.getInclude(); List exclude = xmlPackage.getExclude(); if (include.isEmpty() && exclude.isEmpty()) { @@ -376,13 +377,16 @@ private static int defaultDataProviderThreadCount() { } private static void putIfDifferent( - Map result, String key, Object value, Object defaultValue) { + Map result, + String key, + @Nullable Object value, + @Nullable Object defaultValue) { if (value != null && !value.equals(defaultValue)) { result.put(key, value instanceof Enum ? value.toString() : value); } } - private static void putIfPresent(Map result, String key, Object value) { + private static void putIfPresent(Map result, String key, @Nullable Object value) { if (value == null) { return; } diff --git a/testng-yaml/src/main/java/org/testng/internal/YamlParser.java b/testng-yaml/src/main/java/org/testng/internal/YamlParser.java index cb6629486..aff045453 100644 --- a/testng-yaml/src/main/java/org/testng/internal/YamlParser.java +++ b/testng-yaml/src/main/java/org/testng/internal/YamlParser.java @@ -2,6 +2,7 @@ import java.io.FileNotFoundException; import java.io.InputStream; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; import org.testng.xml.ISuiteParser; import org.testng.xml.XmlSuite; @@ -10,7 +11,7 @@ public class YamlParser implements ISuiteParser { @Override - public XmlSuite parse(String filePath, InputStream is, boolean loadClasses) + public XmlSuite parse(String filePath, @Nullable InputStream is, boolean loadClasses) throws TestNGException { try { return Yaml.parse(filePath, is, loadClasses); diff --git a/testng-yaml/src/main/java/org/testng/internal/YamlSchema.java b/testng-yaml/src/main/java/org/testng/internal/YamlSchema.java index 8bb1337d2..e770373d7 100644 --- a/testng-yaml/src/main/java/org/testng/internal/YamlSchema.java +++ b/testng-yaml/src/main/java/org/testng/internal/YamlSchema.java @@ -8,6 +8,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.BiConsumer; import java.util.function.Function; @@ -91,8 +92,14 @@ static SchemaType suite() { .key( "configFailurePolicy", String.class, - (XmlSuite suite, String value) -> - suite.setConfigFailurePolicy(XmlSuite.FailurePolicy.getValidPolicy(value))) + (XmlSuite suite, String value) -> { + // An unrecognised value keeps the default, as TestNGContentHandler does for XML + // and as the sibling parallel key does for its own enum. + XmlSuite.FailurePolicy policy = XmlSuite.FailurePolicy.getValidPolicy(value); + if (policy != null) { + suite.setConfigFailurePolicy(policy); + } + }) .key("skipFailedInvocationCounts", Boolean.class, XmlSuite::setSkipFailedInvocationCounts) .key("preserveOrder", Boolean.class, XmlSuite::setPreserveOrder) .key("groupByInstances", Boolean.class, XmlSuite::setGroupByInstances) @@ -368,8 +375,11 @@ public Property getProperty(String name) { } String canonical = deprecatedAliases.get(name); if (canonical != null) { + SchemaProperty aliased = + Objects.requireNonNull( + keys.get(canonical), "alias " + name + " points at unknown key " + canonical); LOGGER.warn(deprecationMessage(element, name, canonical)); - return keys.get(canonical); + return aliased; } throw new YAMLException(unknownKeyMessage(element, name, keys.keySet(), deprecatedAliases)); } From b78b7865bf3cddc945b01e4ce51774b816ec4628 Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Tue, 18 Aug 2026 23:27:27 +0200 Subject: [PATCH 4/7] refactor(internal)!: let Utils.escapeHtml and escapeUnicode reject null Under a null-marked package the old shape was self-contradictory: the signature declared a non-null parameter while the body's first act was to answer null with null, so the nullable return was reachable only through an argument the type said could not occur. A probe confirmed it: with both parameters declared non-null, no call site in the tree reports passing a nullable value. The fourteen in-tree callers pass suite names, test names, class names, stack traces and literals. This follows the same reading as XMLUtils.escape, which lost its null branch for the same reason, and leaves the two methods with the contract their bodies already implement. BREAKING CHANGE: Utils.escapeHtml(String) and Utils.escapeUnicode(String) throw NullPointerException for a null input where they used to return null, and their return types are no longer nullable. A Kotlin caller passing a String? no longer compiles. Recorded in CHANGES.txt. --- CHANGES.txt | 2 ++ .../src/main/java/org/testng/internal/Utils.java | 12 ++---------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 39213f7a1..4ddf707f2 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -8,12 +8,14 @@ New: GITHUB-3322: A suite file may declare the schema instead of a doctype, with Changed: GITHUB-3322: The hint printed for a suite file that declares no grammar now offers the schema first and the doctype second, and is no longer printed for a file that declares a schema (Julien Herr) Changed: GITHUB-3322: toXml() now declares the schema on instead of emitting a doctype, so what TestNG writes -- testng-failed.xml above all -- is what TestNG recommends. The two cannot both be declared: the DTD declares neither xmlns:xsi nor xsi:noNamespaceSchemaLocation, so a document carrying both is not DTD-valid. A suite file that already declares a doctype is unaffected; only regenerated output changes (Julien Herr) Changed: GITHUB-3322: Validating a suite against the schema requires a namespace-aware parser, which widens what counts as malformed: in a suite file that declares no doctype, an undeclared namespace prefix is now an error where it used to be read as part of the name. A suite declaring a doctype is unaffected, and testng.xml.validation=off restores the previous behaviour (Julien Herr) +Changed: org.testng.internal.Utils.escapeHtml(String) and escapeUnicode(String) no longer accept null. Both used to answer null with null; they now throw a NullPointerException, and their return types are no longer nullable. No caller in TestNG passes null to either, and the null branch made the signature contradict itself once the package declares its nullness (Julien Herr) Changed: org.testng.reporters.XMLUtils.escape(String) no longer accepts null. It used to answer null with null; it now throws a NullPointerException, and its return type is no longer nullable. Nothing in TestNG ever called it with null, and nothing in TestNG calls it at all outside XMLUtils itself (Julien Herr) Possible backward incompatible changes: - testng-failed.xml no longer records the index of a @Factory produced instance as an invocation-number. That attribute selects rows of a method's own data provider, which is the only thing TestNG ever reads it back as, so a factory powered failure produced a file that looked filtered and re-ran everything -- and, for a method that had a data provider of its own, re-ran the wrong rows because the factory index had overwritten the row index. The instance index now goes to the new factory-instances attribute of , which is honoured on re-run. Tooling that parses testng-failed.xml to learn which factory instance failed must read factory-instances rather than invocation-numbers; a method with its own data provider now re-runs the rows that actually failed. A file generated by 7.13 and re-run by an older TestNG ignores the unknown attribute and re-runs every instance, which is what those versions already did. (GITHUB-3111, GITHUB-2517, GITHUB-2521) - The constructors of org.testng.internal.ParameterInfo and org.testng.internal.LazyParameterInfo now take an org.testng.internal.FactoryInstance instead of a loose index and parameter array. Both are implementation classes of an internal package; only code constructing them directly is affected. (GITHUB-3111) +- org.testng.internal.Utils.escapeHtml(String) and escapeUnicode(String) reject null instead of answering null with null. Both are public members of an internal, OSGi exported package: a Kotlin caller passing a String? stops compiling, and a Java caller passing null gets a NullPointerException from the first character read rather than a null result. Callers that relied on null-in/null-out must test for null themselves. - org.testng.reporters.XMLUtils.escape(String) rejects null instead of answering null with null. The package is now @NullMarked, so the parameter is declared non-null: a Kotlin caller passing a String? stops compiling, and a Java caller passing null gets a NullPointerException from the first character read rather than a null result. Callers that relied on null-in/null-out must test for null themselves. Fixed: GITHUB-3238: A worker that completed exceptionally -- typically because a listener threw -- reached the graph orchestrator as a null worker, so marking its nodes finished threw a NullPointerException from inside FutureTask.done(). The graph never reached its final state and the parallel run hung until the test time-out. The worker now reaches the orchestrator in that case too, and the failure is recorded before listeners are notified so that a listener throwing a second time can no longer turn a failed run green (Krishnan Mahadevan) 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 9f7699aec..89883e427 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 @@ -358,11 +358,7 @@ private enum StackTraceType { } /** Escapes the five characters that must not appear literally in HTML or XML text. */ - public static @Nullable String escapeHtml(@Nullable String s) { - if (s == null) { - return null; - } - + public static String escapeHtml(String s) { StringBuilder result = new StringBuilder(); for (int i = 0; i < s.length(); i++) { @@ -379,11 +375,7 @@ private enum StackTraceType { } /** Replaces every character the JVM does not define with the Unicode replacement character. */ - public static @Nullable String escapeUnicode(@Nullable String s) { - if (s == null) { - return null; - } - + public static String escapeUnicode(String s) { StringBuilder result = new StringBuilder(); for (int i = 0; i < s.length(); i++) { From 813ea0480f8c3a7e7d0f85e261cc12efa31d82c5 Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Wed, 19 Aug 2026 09:21:59 +0200 Subject: [PATCH 5/7] refactor(internal): align the accessors with their nullable fields Review follow-up. The previous commit annotated the fields but left the accessors that return them declared non-null, so the two halves contradicted each other: - ClassImpl.getTestName and getInstanceHashCodes, ClonedMethod.getId, LazyParameterInfo.getInstance and getInstantiationFailure, NoOpTestClass.getInstanceHashCodes, getInstances and getXmlClass, TestNGClassFinder.getFactoryCreationFailedMessage, Graph.getStrictlySortedNodes, MethodInheritance.findSubClass, DynamicGraph.Edges.to and findReversedEdge, TestNGMethod.getDataProviderMethod and the two DataProviderMethod members its subclass can clear. Two null dereferences the annotations made visible: - Graph.getPredecessors dereferenced findNode, which answers null for a node that was never registered. It now raises the same explicit TestNGException addPredecessor already raises for that case. - TestMethodContainer.clearItems called Arrays.fill on the cached array before getItems had populated it, so clearing an untouched container threw. Tarjan.m_cycle is initialised to an empty list rather than annotated: a run that finds no cycle has no cycle to report, and Graph iterates the result without testing it. ITestNGMethod.getDataProviderMethod already documents the null but is not annotated -- it belongs to org.testng, which is not marked yet. --- .../main/java/org/testng/internal/BaseTestMethod.java | 4 ++++ .../src/main/java/org/testng/internal/ClassImpl.java | 4 ++-- .../src/main/java/org/testng/internal/ClonedMethod.java | 2 +- .../java/org/testng/internal/DataProviderMethod.java | 9 +++++---- .../src/main/java/org/testng/internal/DynamicGraph.java | 3 ++- testng-core/src/main/java/org/testng/internal/Graph.java | 8 ++++++-- .../main/java/org/testng/internal/LazyParameterInfo.java | 4 ++-- .../main/java/org/testng/internal/MethodInheritance.java | 2 +- .../src/main/java/org/testng/internal/NoOpTestClass.java | 6 +++--- .../src/main/java/org/testng/internal/Tarjan.java | 3 +-- .../java/org/testng/internal/TestMethodContainer.java | 6 ++++-- .../main/java/org/testng/internal/TestNGClassFinder.java | 2 +- .../src/main/java/org/testng/internal/TestNGMethod.java | 4 ++-- 13 files changed, 34 insertions(+), 23 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 1ae39ba04..654c1354a 100644 --- a/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java +++ b/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java @@ -765,6 +765,10 @@ public void setRetryAnalyzerClass(Class clazz) { } @Override + /** + * @return the retry analyzer class, never null: it is {@link DisabledRetryAnalyzer} until a retry + * analyzer is set, and the setter normalises null back to it. + */ public Class getRetryAnalyzerClass() { return m_retryAnalyzerClass; } 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 11419151e..c831e88ff 100644 --- a/testng-core/src/main/java/org/testng/internal/ClassImpl.java +++ b/testng-core/src/main/java/org/testng/internal/ClassImpl.java @@ -63,7 +63,7 @@ public ClassImpl( } @Override - public String getTestName() { + public @Nullable String getTestName() { return m_testName; } @@ -78,7 +78,7 @@ public Class getRealClass() { } @Override - public long[] getInstanceHashCodes() { + public long @Nullable [] getInstanceHashCodes() { return m_instanceHashCodes; } 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 691e643db..b92a13cda 100644 --- a/testng-core/src/main/java/org/testng/internal/ClonedMethod.java +++ b/testng-core/src/main/java/org/testng/internal/ClonedMethod.java @@ -87,7 +87,7 @@ public String[] getGroupsDependedUpon() { } @Override - public String getId() { + public @Nullable String getId() { return m_id; } 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 2fca52ad4..da5aff480 100644 --- a/testng-core/src/main/java/org/testng/internal/DataProviderMethod.java +++ b/testng-core/src/main/java/org/testng/internal/DataProviderMethod.java @@ -2,6 +2,7 @@ import java.lang.reflect.Method; import java.util.List; +import org.jspecify.annotations.Nullable; import org.testng.IDataProviderMethod; import org.testng.IRetryDataProvider; import org.testng.annotations.IDataProviderAnnotation; @@ -9,8 +10,8 @@ /** Represents an @{@link org.testng.annotations.DataProvider} annotated method. */ class DataProviderMethod implements IDataProviderMethod { - protected Object instance; - protected Method method; + protected @Nullable Object instance; + protected @Nullable Method method; private final IDataProviderAnnotation annotation; DataProviderMethod(Object instance, Method method, IDataProviderAnnotation annotation) { @@ -20,12 +21,12 @@ class DataProviderMethod implements IDataProviderMethod { } @Override - public Object getInstance() { + public @Nullable Object getInstance() { return instance; } @Override - public Method getMethod() { + public @Nullable Method getMethod() { return method; } 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 2bde61e2f..075332f6f 100644 --- a/testng-core/src/main/java/org/testng/internal/DynamicGraph.java +++ b/testng-core/src/main/java/org/testng/internal/DynamicGraph.java @@ -259,6 +259,7 @@ Map from(T node) { return edges == null ? null : Collections.unmodifiableMap(edges); } + @Nullable Map to(T node) { Map edges = m_incomingEdges.get(node); return edges == null ? null : Collections.unmodifiableMap(edges); @@ -272,7 +273,7 @@ Map to(T node) { * @param to - the to edge * @return the weight of the reversed edge or null if edge does not exist */ - private Integer findReversedEdge(T from, T to) { + private @Nullable Integer findReversedEdge(T from, T to) { Map edges = m_outgoingEdges.get(to); return edges == null ? null : edges.get(from); } 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 65a9dd787..7f036327e 100644 --- a/testng-core/src/main/java/org/testng/internal/Graph.java +++ b/testng-core/src/main/java/org/testng/internal/Graph.java @@ -46,7 +46,11 @@ public void addNode(T tm) { } public Set getPredecessors(T node) { - return findNode(node).getPredecessors().keySet(); + Node n = findNode(node); + if (null == n) { + throw new TestNGException("Non-existing node: " + node); + } + return n.getPredecessors().keySet(); } public boolean isIndependent(T object) { @@ -81,7 +85,7 @@ public Set getIndependentNodes() { } /** @return All the nodes that have an order with each other, sorted in one of the valid sorts. */ - public List getStrictlySortedNodes() { + public @Nullable List getStrictlySortedNodes() { return m_strictlySortedNodes; } diff --git a/testng-core/src/main/java/org/testng/internal/LazyParameterInfo.java b/testng-core/src/main/java/org/testng/internal/LazyParameterInfo.java index d7e18d321..ac154ac2e 100644 --- a/testng-core/src/main/java/org/testng/internal/LazyParameterInfo.java +++ b/testng-core/src/main/java/org/testng/internal/LazyParameterInfo.java @@ -34,7 +34,7 @@ public LazyParameterInfo( } @Override - public Object getInstance() { + public @Nullable Object getInstance() { if (!instantiated) { synchronized (lock) { if (!instantiated) { @@ -60,7 +60,7 @@ public Object getInstance() { } @Override - public Throwable getInstantiationFailure() { + public @Nullable Throwable getInstantiationFailure() { return failure; } 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 be25b7847..4b77602d7 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodInheritance.java +++ b/testng-core/src/main/java/org/testng/internal/MethodInheritance.java @@ -67,7 +67,7 @@ public class MethodInheritance { } /** Look in map for a class that is a subclass of methodClass */ - private static Class findSubClass( + private static @Nullable Class findSubClass( Map, List> map, Class methodClass) { return map.keySet().stream() .parallel() diff --git a/testng-core/src/main/java/org/testng/internal/NoOpTestClass.java b/testng-core/src/main/java/org/testng/internal/NoOpTestClass.java index a18805988..6c30da7db 100644 --- a/testng-core/src/main/java/org/testng/internal/NoOpTestClass.java +++ b/testng-core/src/main/java/org/testng/internal/NoOpTestClass.java @@ -128,12 +128,12 @@ public ITestNGMethod[] getAfterGroupsMethods() { /** @see org.testng.internal.IObject#getInstanceHashCodes() */ @Override - public long[] getInstanceHashCodes() { + public long @Nullable [] getInstanceHashCodes() { return m_instanceHashes; } @Override - public Object[] getInstances(boolean reuse) { + public Object @Nullable [] getInstances(boolean reuse) { return m_instances; } @@ -173,7 +173,7 @@ public void setTestClass(Class declaringClass) { } @Override - public XmlClass getXmlClass() { + public @Nullable XmlClass getXmlClass() { return m_xmlClass; } } diff --git a/testng-core/src/main/java/org/testng/internal/Tarjan.java b/testng-core/src/main/java/org/testng/internal/Tarjan.java index 98a1f9ad7..3e34b8ca0 100644 --- a/testng-core/src/main/java/org/testng/internal/Tarjan.java +++ b/testng-core/src/main/java/org/testng/internal/Tarjan.java @@ -6,7 +6,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import org.jspecify.annotations.Nullable; /** * Implementation of the Tarjan algorithm to find and display a cycle in a graph. @@ -18,7 +17,7 @@ public class Tarjan { private final ArrayDeque stack; Map visitedNodes = new HashMap<>(); Map m_lowlinks = new HashMap<>(); - private @Nullable List m_cycle; + private List m_cycle = new ArrayList<>(); public Tarjan(Graph graph, T start) { stack = new ArrayDeque<>(); diff --git a/testng-core/src/main/java/org/testng/internal/TestMethodContainer.java b/testng-core/src/main/java/org/testng/internal/TestMethodContainer.java index ab41ebce3..7f140e086 100644 --- a/testng-core/src/main/java/org/testng/internal/TestMethodContainer.java +++ b/testng-core/src/main/java/org/testng/internal/TestMethodContainer.java @@ -44,8 +44,10 @@ public void clearItems() { if (isCleared) { return; } - Arrays.fill(methods, null); - methods = null; + if (methods != null) { + Arrays.fill(methods, null); + methods = null; + } isCleared = true; } } diff --git a/testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java b/testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java index e2312082f..5ad5e4dda 100644 --- a/testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java +++ b/testng-core/src/main/java/org/testng/internal/TestNGClassFinder.java @@ -42,7 +42,7 @@ public class TestNGClassFinder extends BaseClassFinder { private @Nullable String m_factoryCreationFailedMessage = null; - public String getFactoryCreationFailedMessage() { + public @Nullable String getFactoryCreationFailedMessage() { return m_factoryCreationFailedMessage; } 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 2a1d0e970..7a6d2690d 100644 --- a/testng-core/src/main/java/org/testng/internal/TestNGMethod.java +++ b/testng-core/src/main/java/org/testng/internal/TestNGMethod.java @@ -26,7 +26,7 @@ public class TestNGMethod extends BaseTestMethod { private int m_successPercentage = 100; private boolean isDataDriven = false; private CustomAttribute[] m_attributes = {}; - private IDataProviderMethod dataProviderMethod = null; + private @Nullable IDataProviderMethod dataProviderMethod = null; /** Constructs a TestNGMethod */ public TestNGMethod( @@ -223,7 +223,7 @@ public CustomAttribute[] getAttributes() { } @Override - public IDataProviderMethod getDataProviderMethod() { + public @Nullable IDataProviderMethod getDataProviderMethod() { return dataProviderMethod; } From 0c32b909c386827ec83bc3ba7313d751c310f19e Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Wed, 19 Aug 2026 10:05:46 +0200 Subject: [PATCH 6/7] refactor(internal): resolve the nullable dereferences the annotations exposed Review follow-up, second round. Four contradictions between a nullable value and the code that consumed it: - DynamicGraph.dependencies answers null with an empty list through Optional.ofNullable, and both of its callers hand it the nullable result of Edges.from or Edges.to, so its parameter is nullable too. - Graph.topologicalSort assigned m_strictlySortedNodes and then read the field again to fill it and to dump it, across calls that invalidate what is known about a field. The list is now held in a local and handed to dumpSortedNodes. - NoOpTestClass.getName dereferenced m_testClass, which the protected constructor leaves unset, while getRealClass returned it as non-null. Both go through getRealClass, which rejects the unset case explicitly. The whole suite passes, so no path reaches it: the only subclass assigns the class from its own constructor, and the other constructor takes it from the ITestClass. - NoOpTestClass.getObjects, getInstances and getInstanceHashCodes returned the fields the protected constructor set to null. They are empty arrays now, which is what "no instances" means; TestClass, the only subclass, overrides every reader of them, so nothing observable changes and four annotations go away. --- .../org/testng/internal/DynamicGraph.java | 2 +- .../main/java/org/testng/internal/Graph.java | 11 +++++----- .../org/testng/internal/NoOpTestClass.java | 21 ++++++++++++------- 3 files changed, 20 insertions(+), 14 deletions(-) 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 075332f6f..b66f75d97 100644 --- a/testng-core/src/main/java/org/testng/internal/DynamicGraph.java +++ b/testng-core/src/main/java/org/testng/internal/DynamicGraph.java @@ -88,7 +88,7 @@ public List getDependenciesFor(T node) { return dependencies(m_edges.to(node)); } - private List dependencies(Map dependencies) { + private List dependencies(@Nullable Map dependencies) { return Optional.ofNullable(dependencies) .map(found -> new ArrayList<>(found.keySet())) .orElse(new ArrayList<>()); 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 7f036327e..9936fbb0f 100644 --- a/testng-core/src/main/java/org/testng/internal/Graph.java +++ b/testng-core/src/main/java/org/testng/internal/Graph.java @@ -91,7 +91,8 @@ public Set getIndependentNodes() { public void topologicalSort() { log("================ SORTING"); - m_strictlySortedNodes = new ArrayList<>(); + List sorted = new ArrayList<>(); + m_strictlySortedNodes = sorted; initializeIndependentNodes(); // @@ -131,13 +132,13 @@ public void topologicalSort() { } throw new TestNGException(sb.toString()); } else { - m_strictlySortedNodes.add(node.getObject()); + sorted.add(node.getObject()); removeFromNodes(nodes2, node); } } log("=============== DONE SORTING"); - dumpSortedNodes(); + dumpSortedNodes(sorted); } private void initializeIndependentNodes() { @@ -155,9 +156,9 @@ private void initializeIndependentNodes() { } } - private void dumpSortedNodes() { + private void dumpSortedNodes(List sorted) { log("====== SORTED NODES"); - for (T n : m_strictlySortedNodes) { + for (T n : sorted) { log(" " + n); } log("====== END SORTED NODES"); diff --git a/testng-core/src/main/java/org/testng/internal/NoOpTestClass.java b/testng-core/src/main/java/org/testng/internal/NoOpTestClass.java index 6c30da7db..b64dadcce 100644 --- a/testng-core/src/main/java/org/testng/internal/NoOpTestClass.java +++ b/testng-core/src/main/java/org/testng/internal/NoOpTestClass.java @@ -25,15 +25,15 @@ public class NoOpTestClass implements ITestClass, IObject { protected ITestNGMethod[] m_beforeGroupsMethods = new ITestNGMethod[0]; protected List m_afterGroupsMethods = new ArrayList<>(); - private final IdentifiableObject @Nullable [] m_instances; - private final long @Nullable [] m_instanceHashes; + private final IdentifiableObject[] m_instances; + private final long[] m_instanceHashes; private final @Nullable XmlTest m_xmlTest; private final @Nullable XmlClass m_xmlClass; protected NoOpTestClass() { - m_instances = null; - m_instanceHashes = null; + m_instances = new IdentifiableObject[0]; + m_instanceHashes = new long[0]; m_xmlTest = null; m_xmlClass = null; } @@ -128,23 +128,28 @@ public ITestNGMethod[] getAfterGroupsMethods() { /** @see org.testng.internal.IObject#getInstanceHashCodes() */ @Override - public long @Nullable [] getInstanceHashCodes() { + public long[] getInstanceHashCodes() { return m_instanceHashes; } @Override - public Object @Nullable [] getInstances(boolean reuse) { + public Object[] getInstances(boolean reuse) { return m_instances; } @Override public String getName() { - return m_testClass.getName(); + return getRealClass().getName(); } @Override public Class getRealClass() { - return m_testClass; + Class testClass = m_testClass; + if (testClass == null) { + throw new IllegalStateException( + "setTestClass has not been called on " + getClass().getName()); + } + return testClass; } @Override From 109c8970c8d63a7e811d328b2fd6fed2a266bb8b Mon Sep 17 00:00:00 2001 From: Julien Herr Date: Wed, 19 Aug 2026 10:38:52 +0200 Subject: [PATCH 7/7] docs(internal): say why testClassPaths is volatile The keyword arrived with the publish-once rewrite and reads as bookkeeping. It is the part that makes the rewrite work: the field is written without a lock and read from every package scan, so an unsafe publication lets a reader observe the reference before the element writes that filled it. The result is not a crash but a scan that silently returns fewer classes. --- .../main/java/org/testng/internal/PackageUtils.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/testng-core-api/src/main/java/org/testng/internal/PackageUtils.java b/testng-core-api/src/main/java/org/testng/internal/PackageUtils.java index 84ca869f5..afbcb0f61 100644 --- a/testng-core-api/src/main/java/org/testng/internal/PackageUtils.java +++ b/testng-core-api/src/main/java/org/testng/internal/PackageUtils.java @@ -30,6 +30,18 @@ * @author Cedric Beust */ public class PackageUtils { + /** + * The classpath fragments {@code testng.test.classpath} names, normalised once and cached. + * + *

Written by {@link #getTestClasspath()} without a lock, and read from every package scan -- + * which parallel suites run concurrently. {@code volatile} is what makes the array safe to + * publish: without it a reader may see the reference while the element writes that filled it are + * still invisible, and observe an array of nulls. That is not a crash but a silent wrong answer, + * because {@code matchTestClasspath} would concatenate {@code "null"} into every comparison, + * match nothing, and drop classes from the scan with no error anywhere. Two threads racing to + * build it is harmless: the fragments derive from a system property, so both compute the same + * value and either one may win. + */ private static volatile String @Nullable [] testClassPaths; /** The additional class loaders to find classes in. */