Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 <suite> 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 <include>, 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 <code>
Expand All @@ -27,7 +28,7 @@ public void close() {
}

@Override
public boolean equals(Object object) {
public boolean equals(@Nullable Object object) {
if (this == object) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -67,7 +68,7 @@ static List<ClassLoader> appendContextualClassLoaders(List<ClassLoader> 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<ClassLoader> allClassLoaders = appendContextualClassLoaders(classLoaders);

for (ClassLoader classLoader : allClassLoaders) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -57,7 +77,7 @@ public void makeAccessible() {
}

@Override
public boolean equals(Object o) {
public boolean equals(@Nullable Object o) {
if (this == o) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <code>null</code> if a lazy
* implementation's construction failed -- the failure is memoized rather than rethrown, and
* {@link #getInstantiationFailure()} reports it.
*/
@Nullable
Object getInstance();

/**
Expand All @@ -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;
}

Expand All @@ -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();
}
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -36,7 +37,7 @@ public void close() {
}

@Override
public boolean equals(Object object) {
public boolean equals(@Nullable Object object) {
if (this == object) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,7 +30,19 @@
* @author <a href="mailto:cedric@beust.com">Cedric Beust</a>
*/
public class PackageUtils {
private static String[] testClassPaths;
/**
* The classpath fragments {@code testng.test.classpath} names, normalised once and cached.
*
* <p>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. */
private static final Collection<ClassLoader> classLoaders = new ConcurrentLinkedDeque<>();
Expand Down Expand Up @@ -79,9 +92,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();
Expand All @@ -90,7 +104,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;
Expand All @@ -105,10 +119,11 @@ private static String[] getTestClasspath() {
}
}

testClassPaths[i] = path.replace('\\', '/');
paths[i] = path.replace('\\', '/');
}

return testClassPaths;
testClassPaths = paths;
return paths;
}

private static Function<ClassLoader, Stream<URL>> asURLs(String packageDir) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -21,7 +22,8 @@ public class PropertyUtils {
private static final Logger LOGGER = Logger.getLogger(PropertyUtils.class);

@SuppressWarnings("unchecked")
public static <T> T convertType(Class<T> type, String value, String paramName) {
public static <T> @Nullable T convertType(
Class<T> type, @Nullable String value, String paramName) {
try {
if (value == null || NULL_VALUE.equalsIgnoreCase(value)) {
if (type.isPrimitive()) {
Expand Down Expand Up @@ -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");
Expand All @@ -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) {
Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}

Expand Down
Loading
Loading