diff --git a/CHANGES.txt b/CHANGES.txt index 2a31d3e05..9d33ad78b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -9,11 +9,17 @@ Changed: GITHUB-3322: toXml() now declares the schema on instead of emit 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) +Changed: org.testng.internal.ClonedMethod.getConstructorOrMethod() returns the wrapped method instead of null. It answered null while the class held the java.lang.reflect.Method all along, so its own toString() threw a NullPointerException on every call, and none of the 59 call sites of ITestNGMethod.getConstructorOrMethod() in TestNG tested the result. Keeping it non-null is what lets ITestNGMethod.getConstructorOrMethod() stay non-null in the published interface (Julien Herr) +Fixed: org.testng.internal.TestNGMethod.clone() no longer throws a NullPointerException when the method has not been bound to a test class yet. It wrapped getTestClass() in a NoOpTestClass, which dereferences it on the spot; the absence is now propagated, which is what ConfigurationMethod.clone() already did (Julien Herr) +Fixed: A configuration method that is not a @BeforeGroups or @AfterGroups method now reports an empty array from getBeforeGroups() and getAfterGroups() instead of null. TestNGMethodFinder wrote null into fields whose declaration says {}, and MethodGroupsHelper iterates them without testing (Julien Herr) +Fixed: Sorting test methods by index no longer throws a NullPointerException when a tag carries no name (Julien Herr) +Fixed: A tag that carries no name attribute is now reported the way an unreadable package already was, instead of raising a NullPointerException from inside PackageUtils.findClassesInPackage (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.ClonedMethod.getConstructorOrMethod() answers the wrapped method rather than null. Code that tested the result for null now takes the other branch; no caller in TestNG did, and the method's own toString() could never run before. ITestNGMethod.getConstructorOrMethod() therefore stays non-null when org.testng is marked in turn. - 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. diff --git a/testng-collections/src/main/java/org/testng/collections/MultiMap.java b/testng-collections/src/main/java/org/testng/collections/MultiMap.java index 7216196c9..6bd338493 100644 --- a/testng-collections/src/main/java/org/testng/collections/MultiMap.java +++ b/testng-collections/src/main/java/org/testng/collections/MultiMap.java @@ -22,7 +22,7 @@ protected MultiMap(boolean isSorted) { protected abstract C createValue(); - public boolean put(K key, V method) { + public boolean put(@Nullable K key, V method) { AtomicBoolean exists = new AtomicBoolean(true); return m_objects .computeIfAbsent( @@ -35,7 +35,7 @@ public boolean put(K key, V method) { && exists.get(); } - public C get(K key) { + public C get(@Nullable K key) { return m_objects.computeIfAbsent(key, k -> createValue()); } @@ -43,7 +43,7 @@ public Set keySet() { return new HashSet<>(m_objects.keySet()); } - public boolean containsKey(K k) { + public boolean containsKey(@Nullable K k) { return m_objects.containsKey(k); } @@ -68,7 +68,7 @@ public int size() { return m_objects.size(); } - public boolean remove(K key, V value) { + public boolean remove(@Nullable K key, V value) { return get(key).remove(value); } @@ -78,7 +78,7 @@ public boolean remove(K key, V value) { * @param key the key to drop. * @return the values that were held, or {@code null} when the key was not present. */ - public @Nullable C removeAll(K key) { + public @Nullable C removeAll(@Nullable K key) { return m_objects.remove(key); } @@ -90,7 +90,7 @@ public Collection values() { return m_objects.values(); } - public boolean putAll(K k, Collection values) { + public boolean putAll(@Nullable K k, Collection values) { boolean result = false; for (V v : values) { result = put(k, v) || result; diff --git a/testng-core-api/src/main/java/org/testng/internal/ConstructorOrMethod.java b/testng-core-api/src/main/java/org/testng/internal/ConstructorOrMethod.java index b599905df..9c9c819f5 100644 --- a/testng-core-api/src/main/java/org/testng/internal/ConstructorOrMethod.java +++ b/testng-core-api/src/main/java/org/testng/internal/ConstructorOrMethod.java @@ -49,6 +49,10 @@ public Class[] getParameterTypes() { return member instanceof Method ? (Method) member : null; } + /** + * @return the wrapped member if it is a constructor, or {@code null} if it is a method. Prefer + * {@link #requireConstructor()} unless the null is what you are testing for. + */ public @Nullable Constructor getConstructor() { return member instanceof Constructor ? (Constructor) member : null; } @@ -68,6 +72,21 @@ public Method requireMethod() { throw new NullPointerException("Expected a method, but " + member + " is a constructor"); } + /** + * The wrapped member as a {@link Constructor}, for the callers that have already established it + * is not a method. + * + * @return the wrapped constructor + * @throws NullPointerException if this wrapper holds a method -- the same failure the call sites + * saw before, with a message instead of a bare dereference + */ + public Constructor requireConstructor() { + if (member instanceof Constructor) { + return (Constructor) member; + } + throw new NullPointerException("Expected a constructor, but " + member + " is a method"); + } + /** * Makes the wrapped member accessible. When interning is on the handle is shared, so this is * observable through every wrapper of the same member. diff --git a/testng-core-api/src/main/java/org/testng/internal/Utils.java b/testng-core-api/src/main/java/org/testng/internal/Utils.java index 89883e427..f3d2d3e04 100644 --- a/testng-core-api/src/main/java/org/testng/internal/Utils.java +++ b/testng-core-api/src/main/java/org/testng/internal/Utils.java @@ -286,7 +286,9 @@ public static void writeResourceToFile(File file, String resourceName, Class } public static String defaultIfStringEmpty(@Nullable String s, String defaultValue) { - return isStringEmpty(s) ? defaultValue : s; + // Inlined rather than delegated to isStringEmpty: a nullness check hidden behind a call does + // not refine s on the other branch. + return s == null || s.isEmpty() ? defaultValue : s; } public static boolean isStringBlank(@Nullable String s) { diff --git a/testng-core-api/src/main/java/org/testng/internal/package-info.java b/testng-core-api/src/main/java/org/testng/internal/package-info.java new file mode 100644 index 000000000..528b737f7 --- /dev/null +++ b/testng-core-api/src/main/java/org/testng/internal/package-info.java @@ -0,0 +1,5 @@ +/** The engine behind the public API: test methods, parameters, configuration and scheduling. */ +@NullMarked +package org.testng.internal; + +import org.jspecify.annotations.NullMarked; diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlPackage.java b/testng-core-api/src/main/java/org/testng/xml/XmlPackage.java index 6bc633efa..a2851eb4f 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlPackage.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlPackage.java @@ -65,8 +65,15 @@ public List getXmlClasses() { private List initializeXmlClasses() { List result = new ArrayList<>(); + String name = m_name; + if (name == null) { + // A tag carrying no name attribute. Reported the same way as an unreadable + // package below, rather than through the NullPointerException this used to raise. + Utils.log("XmlPackage", 1, "Ignoring a tag that carries no name."); + return result; + } try { - String[] classes = PackageUtils.findClassesInPackage(m_name, m_include, m_exclude); + String[] classes = PackageUtils.findClassesInPackage(name, m_include, m_exclude); int index = 0; for (String className : classes) { diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlWeaver.java b/testng-core-api/src/main/java/org/testng/xml/XmlWeaver.java index 7157853c2..7fbaa35c7 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlWeaver.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlWeaver.java @@ -36,8 +36,7 @@ private static IWeaveXml instantiateIfRequired() { return instance; } Class clazz = ClassHelper.forName(getClassName()); - boolean isValid = clazz != null && IWeaveXml.class.isAssignableFrom(clazz); - if (!isValid) { + if (clazz == null || !IWeaveXml.class.isAssignableFrom(clazz)) { String msg = "In order for " + getClassName() diff --git a/testng-core/src/main/java/org/testng/internal/BaseClassFinder.java b/testng-core/src/main/java/org/testng/internal/BaseClassFinder.java index ddf774fd4..3aeac6be3 100644 --- a/testng-core/src/main/java/org/testng/internal/BaseClassFinder.java +++ b/testng-core/src/main/java/org/testng/internal/BaseClassFinder.java @@ -32,8 +32,8 @@ protected void putIClass(Class cls, IClass iClass) { protected IClass findOrCreateIClass( ITestContext context, Class cls, - XmlClass xmlClass, - IObject.IdentifiableObject instance, + @Nullable XmlClass xmlClass, + IObject.@Nullable IdentifiableObject instance, IAnnotationFinder annotationFinder, ITestObjectFactory objectFactory) { diff --git a/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java b/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java index 654c1354a..51f0ccb00 100644 --- a/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java +++ b/testng-core/src/main/java/org/testng/internal/BaseTestMethod.java @@ -90,7 +90,7 @@ public abstract class BaseTestMethod private int m_interceptedPriority; private @Nullable XmlTest m_xmlTest; - private final IObject.IdentifiableObject m_instance; + private final IObject.@Nullable IdentifiableObject m_instance; private final Map m_testMethodToRetryAnalyzer = new ConcurrentHashMap<>(); protected final ITestObjectFactory m_objectFactory; @@ -100,7 +100,7 @@ public BaseTestMethod( String methodName, ConstructorOrMethod com, IAnnotationFinder annotationFinder, - IObject.IdentifiableObject instance) { + IObject.@Nullable IdentifiableObject instance) { m_objectFactory = objectFactory; m_methodClass = com.getDeclaringClass(); m_method = com; @@ -133,7 +133,7 @@ public Class getRealClass() { /** {@inheritDoc} */ @Override - public void setTestClass(ITestClass tc) { + public void setTestClass(@Nullable ITestClass tc) { if (tc == null) { throw new IllegalArgumentException("test class cannot be null"); } @@ -175,7 +175,7 @@ public boolean isInstanceInstantiated() { } @Override - public UUID getInstanceId() { + public @Nullable UUID getInstanceId() { return Optional.ofNullable(m_instance) .map(IObject.IdentifiableObject::getInstanceId) .orElse(null); @@ -187,6 +187,19 @@ public long[] getInstanceHashCodes() { return IObject.instanceHashCodes(m_testClass); } + /** + * The instance wrapper a clone of this method should carry: the same identity, or {@code null} + * when this method carries no instance at all. + */ + protected IObject.@Nullable IdentifiableObject cloneInstance() { + Object instance = getInstance(); + UUID instanceId = getInstanceId(); + if (instance == null || instanceId == null) { + return null; + } + return new IObject.IdentifiableObject(instance, instanceId); + } + /** * {@inheritDoc} * @@ -465,15 +478,15 @@ protected void initGroups(Class annotationClass) protected void initBeforeAfterGroups( Class annotationClass, String[] groups) { - String[] groupsAtMethodLevel = + String @Nullable [] groupsAtMethodLevel = calculateGroupsToUseConsideringValuesAndGroupValues(annotationClass, groups); // @BeforeGroups and @AfterGroups annotation cannot be used at Class level. So its always null setGroups(getStringArray(groupsAtMethodLevel, null)); initRestOfGroupDependencies(annotationClass); } - private String[] calculateGroupsToUseConsideringValuesAndGroupValues( - Class annotationClass, String[] groups) { + private String @Nullable [] calculateGroupsToUseConsideringValuesAndGroupValues( + Class annotationClass, String @Nullable [] groups) { if (groups == null || groups.length == 0) { ITestOrConfiguration annotation = getAnnotationFinder().findAnnotation(getConstructorOrMethod(), annotationClass); @@ -521,7 +534,7 @@ private void initRestOfGroupDependencies(Class a setMethodsDependedUpon(methodsDependedUpon); } - private static Map> calculateXmlGroupDependencies(XmlTest xmlTest) { + private static Map> calculateXmlGroupDependencies(@Nullable XmlTest xmlTest) { Map> result = new HashMap<>(); if (xmlTest == null) { return result; @@ -654,7 +667,7 @@ public void addMethodDependedUpon(String method) { /** {@inheritDoc} */ @Override - public void setMissingGroup(String group) { + public void setMissingGroup(@Nullable String group) { m_missingGroup = group; } @@ -669,7 +682,7 @@ public int getThreadPoolSize() { public void setThreadPoolSize(int threadPoolSize) {} @Override - public void setDescription(String description) { + public void setDescription(@Nullable String description) { m_description = description; } @@ -847,7 +860,7 @@ public void setInterceptedPriority(int priority) { return m_xmlTest; } - public void setXmlTest(XmlTest xmlTest) { + public void setXmlTest(@Nullable XmlTest xmlTest) { m_xmlTest = xmlTest; } @@ -863,7 +876,11 @@ public Class[] getParameterTypes() { @Override public Map findMethodParameters(XmlTest test) { - return XmlTestUtils.findMethodParameters(test, getTestClass().getName(), getMethodName()); + // No test class bound yet means no tag can match, which XmlTestUtils answers with + // the suite and parameters on their own. + ITestClass testClass = getTestClass(); + return XmlTestUtils.findMethodParameters( + test, testClass == null ? null : testClass.getName(), getMethodName()); } @Override @@ -873,7 +890,7 @@ public String getQualifiedName() { @Override @Deprecated - public IParameterInfo getFactoryMethodParamsInfo() { + public @Nullable IParameterInfo getFactoryMethodParamsInfo() { return getFactoryParameterInfo(); } @@ -916,7 +933,7 @@ private static boolean isNotParameterisedTest(ITestResult tr) { return Optional.ofNullable(tr.getParameters()).orElse(new Object[0]).length == 0; } - private IRetryAnalyzer computeRetryAnalyzerInstanceToUse(ITestResult tr) { + private @Nullable IRetryAnalyzer computeRetryAnalyzerInstanceToUse(ITestResult tr) { if (m_retryAnalyzer != null) { return m_retryAnalyzer; } diff --git a/testng-core/src/main/java/org/testng/internal/ClassImpl.java b/testng-core/src/main/java/org/testng/internal/ClassImpl.java index c831e88ff..7fdb5e9ef 100644 --- a/testng-core/src/main/java/org/testng/internal/ClassImpl.java +++ b/testng-core/src/main/java/org/testng/internal/ClassImpl.java @@ -30,17 +30,17 @@ public class ClassImpl implements IClass, IObject { private final List identifiableObjects = new ArrayList<>(); private final Map, IClass> m_classes; private long @Nullable [] m_instanceHashCodes; - private final IObject.IdentifiableObject m_instance; + private final IObject.@Nullable IdentifiableObject m_instance; private final ITestObjectFactory m_objectFactory; private @Nullable String m_testName = null; - private final XmlClass m_xmlClass; + private final @Nullable XmlClass m_xmlClass; private final ITestContext m_testContext; public ClassImpl( ITestContext context, Class cls, - XmlClass xmlClass, - IObject.IdentifiableObject instance, + @Nullable XmlClass xmlClass, + IObject.@Nullable IdentifiableObject instance, Map, IClass> classes, IAnnotationFinder annotationFinder, ITestObjectFactory objectFactory) { @@ -51,8 +51,9 @@ public ClassImpl( m_annotationFinder = annotationFinder; m_instance = instance; m_objectFactory = objectFactory; - if (IObject.IdentifiableObject.unwrap(instance) instanceof ITest) { - m_testName = ((ITest) instance.getInstance()).getTestName(); + Object unwrapped = IObject.IdentifiableObject.unwrap(instance); + if (unwrapped instanceof ITest) { + m_testName = ((ITest) unwrapped).getTestName(); } if (m_testName == null) { ITestAnnotation annotation = m_annotationFinder.findAnnotation(cls, ITestAnnotation.class); @@ -88,11 +89,12 @@ public XmlTest getXmlTest() { } @Override - public XmlClass getXmlClass() { + public @Nullable XmlClass getXmlClass() { return m_xmlClass; } - private IObject.IdentifiableObject getDefaultInstance(boolean create, String errMsgPrefix) { + private IObject.@Nullable IdentifiableObject getDefaultInstance( + boolean create, String errMsgPrefix) { if (m_defaultInstance == null) { if (m_instance != null) { m_defaultInstance = m_instance; @@ -170,7 +172,9 @@ private static int computeHashCode(IdentifiableObject identifiable) { // derive a stable one from its unique instance id instead. return identifiable.getInstanceId().hashCode(); } - return IParameterInfo.embeddedInstance(instance).hashCode(); + return java.util.Objects.requireNonNull( + IParameterInfo.embeddedInstance(instance), "the factory instance is not available") + .hashCode(); } private DetailedAttributes newDetailedAttributes(boolean create, String errMsgPrefix) { diff --git a/testng-core/src/main/java/org/testng/internal/ClonedMethod.java b/testng-core/src/main/java/org/testng/internal/ClonedMethod.java index b92a13cda..d81de758d 100644 --- a/testng-core/src/main/java/org/testng/internal/ClonedMethod.java +++ b/testng-core/src/main/java/org/testng/internal/ClonedMethod.java @@ -19,7 +19,7 @@ public class ClonedMethod implements ITestNGMethod { private final ITestNGMethod m_method; - private final Method m_javaMethod; + private final ConstructorOrMethod m_constructorOrMethod; private @Nullable String m_id; private int m_currentInvocationCount; @@ -28,7 +28,7 @@ public class ClonedMethod implements ITestNGMethod { public ClonedMethod(ITestNGMethod method, Method javaMethod) { m_method = method; - m_javaMethod = javaMethod; + m_constructorOrMethod = new ConstructorOrMethod(javaMethod); } @Override @@ -118,7 +118,7 @@ public long getInvocationTimeOut() { @Override public String getMethodName() { - return m_javaMethod.getName(); + return m_constructorOrMethod.getName(); } @Override @@ -127,7 +127,7 @@ public String[] getMethodsDependedUpon() { } @Override - public String getMissingGroup() { + public @Nullable String getMissingGroup() { return null; } @@ -146,7 +146,7 @@ public boolean hasMoreInvocation() { @Override public Class getRealClass() { - return m_javaMethod.getDeclaringClass(); + return m_constructorOrMethod.getDeclaringClass(); } @Override @@ -265,7 +265,7 @@ public void setDate(long date) { } @Override - public void setId(String id) { + public void setId(@Nullable String id) { m_id = id; } @@ -297,7 +297,7 @@ public boolean skipFailedInvocations() { @Override public ClonedMethod clone() { - return new ClonedMethod(m_method, m_javaMethod); + return new ClonedMethod(m_method, m_constructorOrMethod.requireMethod()); } @Override @@ -354,7 +354,7 @@ public XmlTest getXmlTest() { @Override public ConstructorOrMethod getConstructorOrMethod() { - return null; + return m_constructorOrMethod; } @Override diff --git a/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java b/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java index 443c30f98..2936c3342 100644 --- a/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java +++ b/testng-core/src/main/java/org/testng/internal/ConfigurationGroupMethods.java @@ -81,10 +81,6 @@ public List getAfterGroupMethods(ITestNGMethod testMethod) { Set methodGroups = new HashSet<>(Arrays.asList(testMethod.getGroups())); try (AutoCloseableLock ignore = afterGroups.lock()) { - if (m_afterGroupsMap == null) { - m_afterGroupsMap = initializeAfterGroupsMap(); - } - return methodGroups.stream() .filter(t -> isLastMethodForGroup(t, testMethod)) .map(t -> retrieve(afterGroupsThatHaveAlreadyRun, m_afterGroupsMethods, t)) @@ -105,7 +101,7 @@ private boolean isAfterGroupAllowedToRunAfterTestMethod( .allMatch( t -> testMethodGroups.contains(t) - || !CollectionUtils.hasElements(m_afterGroupsMap.get(t))); + || !CollectionUtils.hasElements(afterGroupsMap().get(t))); } public void removeBeforeGroups(String[] groups) { @@ -131,7 +127,7 @@ public void removeAfterGroups(Collection groups) { * figure out when is the right time to invoke afterGroups methods. */ private boolean isLastMethodForGroup(String group, ITestNGMethod method) { - List methodsInGroup = m_afterGroupsMap.get(group); + List methodsInGroup = afterGroupsMap().get(group); if (null == methodsInGroup || methodsInGroup.isEmpty()) { return true; @@ -143,6 +139,16 @@ private boolean isLastMethodForGroup(String group, ITestNGMethod method) { return methodsInGroup.isEmpty(); } + /** The group-to-methods map, built on first use. Callers hold {@code afterGroups}. */ + private Map> afterGroupsMap() { + Map> cached = m_afterGroupsMap; + if (cached == null) { + cached = initializeAfterGroupsMap(); + m_afterGroupsMap = cached; + } + return cached; + } + private Map> initializeAfterGroupsMap() { Map> result = new ConcurrentHashMap<>(); for (ITestNGMethod m : m_allMethods) { diff --git a/testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java b/testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java index 5905710e5..bac5e791f 100644 --- a/testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java +++ b/testng-core/src/main/java/org/testng/internal/ConfigurationMethod.java @@ -492,7 +492,7 @@ public ConfigurationMethod clone() { getBeforeGroups(), getAfterGroups(), false /* do not call init() */, - new IObject.IdentifiableObject(getInstance(), getInstanceId())); + cloneInstance()); clone.m_testClass = getTestClass(); clone.setDate(getDate()); clone.setGroups(getGroups()); diff --git a/testng-core/src/main/java/org/testng/internal/DataProviderMethod.java b/testng-core/src/main/java/org/testng/internal/DataProviderMethod.java index da5aff480..6b5eee66a 100644 --- a/testng-core/src/main/java/org/testng/internal/DataProviderMethod.java +++ b/testng-core/src/main/java/org/testng/internal/DataProviderMethod.java @@ -14,7 +14,7 @@ class DataProviderMethod implements IDataProviderMethod { protected @Nullable Method method; private final IDataProviderAnnotation annotation; - DataProviderMethod(Object instance, Method method, IDataProviderAnnotation annotation) { + DataProviderMethod(@Nullable Object instance, Method method, IDataProviderAnnotation annotation) { this.instance = instance; this.method = method; this.annotation = annotation; diff --git a/testng-core/src/main/java/org/testng/internal/DataProviderMethodRemovable.java b/testng-core/src/main/java/org/testng/internal/DataProviderMethodRemovable.java index 8faad2347..68d9bce01 100644 --- a/testng-core/src/main/java/org/testng/internal/DataProviderMethodRemovable.java +++ b/testng-core/src/main/java/org/testng/internal/DataProviderMethodRemovable.java @@ -7,7 +7,8 @@ /** Represents an @{@link org.testng.annotations.DataProvider} annotated method. */ class DataProviderMethodRemovable extends DataProviderMethod { - DataProviderMethodRemovable(Object instance, Method method, IDataProviderAnnotation annotation) { + DataProviderMethodRemovable( + @Nullable Object instance, Method method, IDataProviderAnnotation annotation) { super(instance, method, annotation); } diff --git a/testng-core/src/main/java/org/testng/internal/DynamicGraph.java b/testng-core/src/main/java/org/testng/internal/DynamicGraph.java index b66f75d97..dbadec54d 100644 --- a/testng-core/src/main/java/org/testng/internal/DynamicGraph.java +++ b/testng-core/src/main/java/org/testng/internal/DynamicGraph.java @@ -8,6 +8,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -309,9 +310,9 @@ int getLowestEdgeWeight(Set nodes) { int lowestWeight = Integer.MAX_VALUE; for (T node : intersection) { - Map weightMap = m_outgoingEdges.get(node); - + // The intersection was taken against this very key set, so the lookup cannot miss. // Not catching NoSuchElementException, because that would indicate our graph is corrupt. + Map weightMap = Objects.requireNonNull(m_outgoingEdges.get(node)); lowestWeight = Math.min(lowestWeight, Collections.min(weightMap.values())); } return lowestWeight; diff --git a/testng-core/src/main/java/org/testng/internal/DynamicGraphHelper.java b/testng-core/src/main/java/org/testng/internal/DynamicGraphHelper.java index fe81fd896..af20ddfbd 100644 --- a/testng-core/src/main/java/org/testng/internal/DynamicGraphHelper.java +++ b/testng-core/src/main/java/org/testng/internal/DynamicGraphHelper.java @@ -6,7 +6,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; import org.testng.DependencyMap; import org.testng.ITestNGMethod; @@ -33,7 +33,7 @@ public static DynamicGraph createDynamicGraph( // Keep track of whether we have group dependencies. If we do, preserve-order needs // to be ignored since group dependencies create inter-class dependencies which can // end up creating cycles when combined with preserve-order. - final AtomicReference hasDependencies = new AtomicReference<>(false); + final AtomicBoolean hasDependencies = new AtomicBoolean(false); Arrays.stream(methods) .forEach( m -> { diff --git a/testng-core/src/main/java/org/testng/internal/FactoryMethod.java b/testng-core/src/main/java/org/testng/internal/FactoryMethod.java index 542aef83a..dd2e89b28 100644 --- a/testng-core/src/main/java/org/testng/internal/FactoryMethod.java +++ b/testng-core/src/main/java/org/testng/internal/FactoryMethod.java @@ -9,7 +9,9 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; +import org.jspecify.annotations.Nullable; import org.testng.DataProviderHolder; import org.testng.IDataProviderInterceptor; import org.testng.IDataProviderListener; @@ -31,18 +33,19 @@ /** This class represents a method annotated with @Factory */ public class FactoryMethod extends BaseTestMethod { - private final IFactoryAnnotation factoryAnnotation; - private final Object m_instance; + private final @Nullable IFactoryAnnotation factoryAnnotation; + private final @Nullable Object m_instance; private final ITestContext m_testContext; - private String m_factoryCreationFailedMessage = null; + private @Nullable String m_factoryCreationFailedMessage = null; private final DataProviderHolder holder; private final boolean m_lazy; - public String getFactoryCreationFailedMessage() { + public @Nullable String getFactoryCreationFailedMessage() { return m_factoryCreationFailedMessage; } - private void init(Object instance, IAnnotationFinder annotationFinder, ConstructorOrMethod com) { + private void init( + @Nullable Object instance, IAnnotationFinder annotationFinder, ConstructorOrMethod com) { IListenersAnnotation annotation = annotationFinder.findAnnotation(com.getDeclaringClass(), IListenersAnnotation.class); if (annotation == null) { @@ -75,7 +78,7 @@ private void init(Object instance, IAnnotationFinder annotationFinder, Construct // constructor outside of this package. FactoryMethod( ConstructorOrMethod com, - IObject.IdentifiableObject identifiable, + IObject.@Nullable IdentifiableObject identifiable, IAnnotationFinder annotationFinder, ITestContext testContext, ITestObjectFactory objectFactory, @@ -89,10 +92,8 @@ private void init(Object instance, IAnnotationFinder annotationFinder, Construct Utils.checkReturnType(com.getMethod(), Object[].class, IInstanceInfo[].class); Class declaringClass = com.getDeclaringClass(); if (instance != null && !declaringClass.isAssignableFrom(instance.getClass())) { - if (instance instanceof IParameterInfo) { - instance = ((IParameterInfo) instance).getInstance(); - } - Class cls = instance.getClass(); + Object embedded = IParameterInfo.embeddedInstance(instance); + Class cls = (embedded != null ? embedded : instance).getClass(); String msg = "Found a default constructor and also a Factory method when working with " + declaringClass.getName() @@ -208,7 +209,9 @@ public IParameterInfo[] invoke() { Iterator parameterIterator = parameterHolder.parameters; try { - List indices = factoryAnnotation.getIndices(); + List indices = + Objects.requireNonNull(factoryAnnotation, "no @Factory annotation on a factory method") + .getIndices(); int position = 0; IFactory factory = new FactoryDescriptor(getConstructorOrMethod(), m_lazy); while (parameterIterator.hasNext()) { diff --git a/testng-core/src/main/java/org/testng/internal/FilteredParameters.java b/testng-core/src/main/java/org/testng/internal/FilteredParameters.java index 410c267d1..dc5322122 100644 --- a/testng-core/src/main/java/org/testng/internal/FilteredParameters.java +++ b/testng-core/src/main/java/org/testng/internal/FilteredParameters.java @@ -6,7 +6,14 @@ import org.testng.ITestNGMethod; import org.testng.TestNGException; -class FilteredParameters implements Iterator { +/** + * Hides the parameter rows an {@code indices} restriction excludes, by answering {@code null} for + * them rather than skipping them: the consumers count the skipped rows to keep the reported + * parameter index aligned with the data provider's own numbering. See {@code MethodRunner:47} and + * {@code MethodRunner:121}, which increment {@code parametersIndex} on each null, and {@code + * FactoryMethod:216}. + */ +class FilteredParameters implements Iterator { private int index = 0; private boolean hasWarn = false; diff --git a/testng-core/src/main/java/org/testng/internal/Graph.java b/testng-core/src/main/java/org/testng/internal/Graph.java index 9936fbb0f..ff109cbed 100644 --- a/testng-core/src/main/java/org/testng/internal/Graph.java +++ b/testng-core/src/main/java/org/testng/internal/Graph.java @@ -33,7 +33,7 @@ public class Graph { // A map of nodes that are not the predecessors of any node // (not needed for the algorithm but convenient to calculate // the parallel/sequential lists in TestNG). - private Map> m_independentNodes = null; + private @Nullable Map> m_independentNodes = null; public Graph(Comparator> comparator) { this.comparator = comparator; @@ -54,7 +54,7 @@ public Set getPredecessors(T node) { } public boolean isIndependent(T object) { - return m_independentNodes.containsKey(object); + return initializeIndependentNodes().containsKey(object); } private @Nullable Node findNode(T object) { @@ -68,9 +68,9 @@ public void addPredecessor(T tm, T predecessor) { } else { node.addPredecessor(predecessor); // Remove these two nodes from the independent list - initializeIndependentNodes(); - m_independentNodes.remove(predecessor); - m_independentNodes.remove(tm); + Map> independentNodes = initializeIndependentNodes(); + independentNodes.remove(predecessor); + independentNodes.remove(tm); log(() -> " REMOVED " + predecessor + " FROM INDEPENDENT OBJECTS"); } } @@ -81,7 +81,7 @@ private Collection> getNodes() { /** @return All the nodes that don't have any order with each other. */ public Set getIndependentNodes() { - return m_independentNodes.keySet(); + return initializeIndependentNodes().keySet(); } /** @return All the nodes that have an order with each other, sorted in one of the valid sorts. */ @@ -141,9 +141,10 @@ public void topologicalSort() { dumpSortedNodes(sorted); } - private void initializeIndependentNodes() { - if (null == m_independentNodes) { - m_independentNodes = + private Map> initializeIndependentNodes() { + Map> independentNodes = m_independentNodes; + if (null == independentNodes) { + independentNodes = new ArrayList<>(m_nodes.values()) .stream() .sorted(comparator) @@ -153,7 +154,9 @@ private void initializeIndependentNodes() { Function.identity(), (a, b) -> a, Maps::newLinkedHashMap)); + m_independentNodes = independentNodes; } + return independentNodes; } private void dumpSortedNodes(List sorted) { diff --git a/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java b/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java index 9b9efe494..466931a2f 100644 --- a/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java +++ b/testng-core/src/main/java/org/testng/internal/IInstanceIdentity.java @@ -3,16 +3,18 @@ import java.util.Arrays; import java.util.Objects; import java.util.UUID; +import org.jspecify.annotations.Nullable; public interface IInstanceIdentity { /** * @return - A {@link UUID} that represents a unique id which is associated with - * every test class object. + * every test class object, or {@code null} when the implementation carries no instance. */ + @Nullable UUID getInstanceId(); - static Object getInstanceId(Object object) { + static @Nullable Object getInstanceId(Object object) { if (object instanceof IInstanceIdentity) { return ((IInstanceIdentity) object).getInstanceId(); } diff --git a/testng-core/src/main/java/org/testng/internal/IObject.java b/testng-core/src/main/java/org/testng/internal/IObject.java index 15cab568a..2091a5ffd 100644 --- a/testng-core/src/main/java/org/testng/internal/IObject.java +++ b/testng-core/src/main/java/org/testng/internal/IObject.java @@ -32,7 +32,7 @@ public interface IObject { * @param object - The object that should be inspected for its compatibility with {@link IObject}. * @return - An array representing the hash codes of the corresponding instances. */ - static long[] instanceHashCodes(Object object) { + static long[] instanceHashCodes(@Nullable Object object) { return cast(object).map(IObject::getInstanceHashCodes).orElse(new long[] {}); } @@ -42,7 +42,7 @@ static long[] instanceHashCodes(Object object) { * @return - An array (can be empty is instance compatibility fails) of {@link IdentifiableObject} * objects. */ - static IdentifiableObject[] objects(Object object, boolean create) { + static IdentifiableObject[] objects(@Nullable Object object, boolean create) { return objects(object, create, ""); } @@ -54,7 +54,8 @@ static IdentifiableObject[] objects(Object object, boolean create) { * @return - An array (can be empty is instance compatibility fails) of {@link IdentifiableObject} * objects. */ - static IdentifiableObject[] objects(Object object, boolean create, String errorMsgPrefix) { + static IdentifiableObject[] objects( + @Nullable Object object, boolean create, String errorMsgPrefix) { return cast(object) .map(it -> it.getObjects(create, errorMsgPrefix)) .orElse(new IdentifiableObject[] {}); @@ -65,7 +66,7 @@ static IdentifiableObject[] objects(Object object, boolean create, String errorM * @return - If the incoming object is an instance of {@link IObject} then the cast instance is * wrapped within {@link Optional} else it would be an {@link Optional#empty()} */ - static Optional cast(Object object) { + static Optional cast(@Nullable Object object) { if (object instanceof IObject) { return Optional.of((IObject) object); } diff --git a/testng-core/src/main/java/org/testng/internal/ITestClassConfigInfo.java b/testng-core/src/main/java/org/testng/internal/ITestClassConfigInfo.java index c8695791b..3daefe088 100644 --- a/testng-core/src/main/java/org/testng/internal/ITestClassConfigInfo.java +++ b/testng-core/src/main/java/org/testng/internal/ITestClassConfigInfo.java @@ -3,6 +3,7 @@ import java.util.ArrayList; import java.util.List; import java.util.UUID; +import org.jspecify.annotations.Nullable; import org.testng.ITestClass; import org.testng.ITestNGMethod; @@ -23,9 +24,9 @@ public interface ITestClassConfigInfo { * @param instanceId the per-instance id (UUID) of the test class instance * @return All before class methods of instance */ - List getInstanceBeforeClassMethods(UUID instanceId); + List getInstanceBeforeClassMethods(@Nullable UUID instanceId); - List getInstanceAfterClassMethods(UUID instanceId); + List getInstanceAfterClassMethods(@Nullable UUID instanceId); static List allBeforeClassMethods(ITestClass tc) { if (tc instanceof ITestClassConfigInfo) { diff --git a/testng-core/src/main/java/org/testng/internal/MethodGroupsHelper.java b/testng-core/src/main/java/org/testng/internal/MethodGroupsHelper.java index fd93f3a9a..ac6b6fdba 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodGroupsHelper.java +++ b/testng-core/src/main/java/org/testng/internal/MethodGroupsHelper.java @@ -7,12 +7,14 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Predicate; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.jspecify.annotations.Nullable; import org.testng.ITestClass; import org.testng.ITestNGMethod; import org.testng.annotations.IConfigurationAnnotation; @@ -41,7 +43,7 @@ static void collectMethodsByGroup( boolean unique) { for (ITestNGMethod tm : methods) { boolean in = false; - Method m = tm.getConstructorOrMethod().getMethod(); + Method m = tm.getConstructorOrMethod().requireMethod(); // // @Test method // @@ -60,7 +62,10 @@ static void collectMethodsByGroup( // @Configuration method // else { - IConfigurationAnnotation annotation = AnnotationHelper.findConfiguration(finder, m); + IConfigurationAnnotation annotation = + Objects.requireNonNull( + AnnotationHelper.findConfiguration(finder, m), + "a configuration method always carries a @Before/@After annotation"); if (annotation.getAlwaysRun()) { if (!unique || MethodGroupsHelper.isMethodAlreadyNotPresent(outIncludedMethods, tm)) { in = true; @@ -85,7 +90,7 @@ static void collectMethodsByGroup( } private static boolean includeMethod( - ITestOrConfiguration annotation, + @Nullable ITestOrConfiguration annotation, RunInfo runInfo, ITestNGMethod tm, boolean forTests, @@ -215,7 +220,8 @@ protected static void findGroupTransitiveClosure( outGroups.addAll(runningGroups.keySet()); } - private static ITestNGMethod findMethodNamed(String tm, List allMethods) { + private static @Nullable ITestNGMethod findMethodNamed( + String tm, List allMethods) { return allMethods.stream() .filter(m -> m.getQualifiedName().equals(tm)) .findFirst() diff --git a/testng-core/src/main/java/org/testng/internal/MethodHelper.java b/testng-core/src/main/java/org/testng/internal/MethodHelper.java index db5dd38d3..c102df74e 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodHelper.java +++ b/testng-core/src/main/java/org/testng/internal/MethodHelper.java @@ -77,7 +77,7 @@ public static ITestNGMethod[] collectAndOrderMethods( results.set( sortMethods(forTests, includedMethods, comparator) .toArray(new ITestNGMethod[] {}))); - return results.get(); + return Objects.requireNonNull(results.get(), "the sorted methods were never published"); } /** @@ -225,7 +225,8 @@ public static ITestNGMethod[] findDependedUponMethods(ITestNGMethod m, ITestNGMe * @param testngMethod TestNG method * @param regExp regex representing a method and/or related class name */ - private static Method findMethodByName(ITestNGMethod testngMethod, String regExp) { + private static @Nullable Method findMethodByName( + ITestNGMethod testngMethod, @Nullable String regExp) { if (regExp == null) { return null; } @@ -275,7 +276,7 @@ public static boolean isEnabled(@Nullable ITestOrConfiguration test) { return null == test || test.getEnabled(); } - public static boolean isAlwaysRun(IConfigurationAnnotation configurationAnnotation) { + public static boolean isAlwaysRun(@Nullable IConfigurationAnnotation configurationAnnotation) { if (null == configurationAnnotation) { return false; } @@ -340,11 +341,11 @@ private static Graph topologicalSort( String[] methodsDependedUpon = m.getMethodsDependedUpon(); if (methodsDependedUpon.length > 0) { ITestNGMethod[] methodsNamed; + Object instanceId = IInstanceIdentity.getInstanceId(m); // Method has instance - if (IInstanceIdentity.getInstanceId(m) != null) { - // Get other methods with the same instance - List instanceMethods = - testInstances.get(IInstanceIdentity.getInstanceId(m)); + List instanceMethods = + instanceId == null ? null : testInstances.get(instanceId); + if (instanceMethods != null) { try { // Search for other methods that depends upon with the same instance methodsNamed = MethodHelper.findDependedUponMethods(m, instanceMethods); @@ -566,7 +567,8 @@ private static boolean isConfigurationMethod(ITestNGMethod tm) { || tm.isAfterMethodConfiguration(); } - protected static String calculateMethodCanonicalName(Class methodClass, String methodName) { + protected static @Nullable String calculateMethodCanonicalName( + Class methodClass, String methodName) { Set methods = ClassHelper.getAvailableMethods(methodClass); // TESTNG-139 return methods.stream() .filter(method -> methodName.equals(method.getName())) diff --git a/testng-core/src/main/java/org/testng/internal/MethodInheritance.java b/testng-core/src/main/java/org/testng/internal/MethodInheritance.java index 4b77602d7..b5866bd28 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodInheritance.java +++ b/testng-core/src/main/java/org/testng/internal/MethodInheritance.java @@ -100,8 +100,9 @@ public static void fixMethodInheritance(ITestNGMethod[] methods, boolean before) l.add(method); } else { Class subClass = findSubClass(map, methodClass); - if (null != subClass) { - l = map.get(subClass); + List subClassMethods = subClass == null ? null : map.get(subClass); + if (null != subClassMethods) { + l = subClassMethods; l.add(method); map.remove(subClass); map.put(methodClass, l); diff --git a/testng-core/src/main/java/org/testng/internal/MethodInstance.java b/testng-core/src/main/java/org/testng/internal/MethodInstance.java index 34ef65534..6f6dd683d 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodInstance.java +++ b/testng-core/src/main/java/org/testng/internal/MethodInstance.java @@ -2,6 +2,7 @@ import java.util.Comparator; import java.util.List; +import org.jspecify.annotations.Nullable; import org.testng.IMethodInstance; import org.testng.ITestNGMethod; import org.testng.collections.Objects; @@ -43,7 +44,7 @@ public int compare(IMethodInstance o1, IMethodInstance o2) { XmlTest test2 = o2.getMethod().getTestClass().getXmlTest(); // If the two methods are not in the same , we can't compare them - if (!test1.getName().equals(test2.getName())) { + if (!java.util.Objects.equals(test1.getName(), test2.getName())) { return 0; } @@ -83,7 +84,8 @@ public int compare(IMethodInstance o1, IMethodInstance o2) { return result; } - private XmlInclude findXmlInclude(List includedMethods, String methodName) { + private @Nullable XmlInclude findXmlInclude( + List includedMethods, String methodName) { for (XmlInclude xi : includedMethods) { if (xi.getName().equals(methodName)) { return xi; diff --git a/testng-core/src/main/java/org/testng/internal/MethodSorting.java b/testng-core/src/main/java/org/testng/internal/MethodSorting.java index 632e4ad6f..d4b702ff2 100644 --- a/testng-core/src/main/java/org/testng/internal/MethodSorting.java +++ b/testng-core/src/main/java/org/testng/internal/MethodSorting.java @@ -46,7 +46,7 @@ private int objectEquality(ITestNGMethod a, ITestNGMethod b) { // sorting never forces a lazy @Factory instance to be created during collection. Object one = IInstanceIdentity.getInstanceId(a); Object two = IInstanceIdentity.getInstanceId(b); - if (IInstanceIdentity.isIdentityAware(one, two)) { + if (one != null && two != null && IInstanceIdentity.isIdentityAware(one, two)) { return ((UUID) one).compareTo((UUID) two); } return Integer.compare(Objects.hashCode(one), Objects.hashCode(two)); diff --git a/testng-core/src/main/java/org/testng/internal/Parameters.java b/testng-core/src/main/java/org/testng/internal/Parameters.java index fb119cbfb..8daab4e81 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 java.util.Objects; import org.jspecify.annotations.Nullable; import org.testng.DataProviderHolder; import org.testng.IDataProviderInterceptor; @@ -164,12 +165,12 @@ public static Object[] createInstantiationParameters( public static Object[] createConfigurationParameters( Method m, Map params, - Object[] parameterValues, - ITestNGMethod currentTestMethod, + Object @Nullable [] parameterValues, + @Nullable ITestNGMethod currentTestMethod, IAnnotationFinder finder, XmlSuite xmlSuite, ITestContext ctx, - ITestResult testResult) { + @Nullable ITestResult testResult) { Method currentTestMeth = currentTestMethod != null ? currentTestMethod.getConstructorOrMethod().getMethod() : null; @@ -190,7 +191,7 @@ public static Object[] createConfigurationParameters( name); } - private static Class retrieveConfigAnnotation(Method m) { + private static @Nullable Class retrieveConfigAnnotation(Method m) { return annotationList.stream() .filter(annotation -> m.getAnnotation(annotation) != null) .findAny() @@ -402,7 +403,7 @@ private static Parameter[] extractParameters(ConstructorOrMethod method) { if (method.getMethod() != null) { return ReflectionRecipes.getMethodParameters(method.getMethod()); } - return ReflectionRecipes.getConstructorParameters(method.getConstructor()); + return ReflectionRecipes.getConstructorParameters(method.requireConstructor()); } private static boolean canInject(String annotation) { @@ -509,13 +510,13 @@ private static String prettyFormat(List> classes) { return builder.toString(); } - private static IDataProviderMethod findDataProvider( + private static @Nullable IDataProviderMethod findDataProvider( ITestObjectFactory objectFactory, - Object instance, + @Nullable Object instance, ITestClass clazz, ConstructorOrMethod m, IAnnotationFinder finder, - ITestContext context) { + @Nullable ITestContext context) { IDataProviderMethod result = null; IDataProvidable dp = findDataProviderInfo(clazz, m, finder); @@ -562,11 +563,11 @@ private static IDataProviderMethod findDataProvider( * Find the data provider info (data provider name and class) on either @Test(dataProvider), * @Factory(dataProvider) on a method or @Factory(dataProvider) on a constructor. */ - private static IDataProvidable findDataProviderInfo( + private static @Nullable IDataProvidable findDataProviderInfo( ITestClass clazz, ConstructorOrMethod m, IAnnotationFinder finder) { if (m.getMethod() == null) { // @Factory(dataProvider) on a constructor - return AnnotationHelper.findFactory(finder, m.getConstructor()); + return AnnotationHelper.findFactory(finder, m.requireConstructor()); } // @Test(dataProvider) on a method @@ -629,15 +630,15 @@ private static boolean isDataProviderNameEmpty(ITestAnnotation annotation) { } /** Find a method that has a @DataProvider(name=name) */ - private static IDataProviderMethod findDataProvider( + private static @Nullable IDataProviderMethod findDataProvider( ITestObjectFactory objectFactory, - Object instance, + @Nullable Object instance, ITestClass clazz, IAnnotationFinder finder, String name, - Class dataProviderClass, + @Nullable Class dataProviderClass, boolean isDynamicDataProvider, - ITestContext context) { + @Nullable ITestContext context) { IDataProviderMethod result = null; Class cls = clazz.getRealClass(); @@ -650,8 +651,7 @@ private static IDataProviderMethod findDataProvider( for (Method m : ClassHelper.getAvailableMethods(cls)) { IDataProviderAnnotation dp = finder.findAnnotation(m, IDataProviderAnnotation.class); - boolean proceed = null != dp && name.equals(getDataProviderName(dp, m)); - if (!proceed) { + if (dp == null || !name.equals(getDataProviderName(dp, m))) { continue; } Object instanceToUse = instance; @@ -692,7 +692,7 @@ private static String[] extractOptionalValues( if (consMethod.getMethod() != null) { return finder.findOptionalValues(consMethod.getMethod()); } - return finder.findOptionalValues(consMethod.getConstructor()); + return finder.findOptionalValues(consMethod.requireConstructor()); } private static Object[] createParameters( @@ -751,11 +751,11 @@ public static ParameterHolder handleParameters( ITestObjectFactory objectFactory, ITestNGMethod testMethod, Map allParameterNames, - Object instance, + @Nullable Object instance, MethodParameters methodParams, XmlSuite xmlSuite, IAnnotationFinder annotationFinder, - Object fedInstance, + @Nullable Object fedInstance, DataProviderHolder holder) { return handleParameters( objectFactory, @@ -780,11 +780,11 @@ public static ParameterHolder handleParameters( ITestObjectFactory objectFactory, ITestNGMethod testMethod, Map allParameterNames, - Object instance, + @Nullable Object instance, MethodParameters methodParams, XmlSuite xmlSuite, IAnnotationFinder annotationFinder, - Object fedInstance, + @Nullable Object fedInstance, DataProviderHolder holder, String annotationName) { /* @@ -812,13 +812,17 @@ public static ParameterHolder handleParameters( allParameterNames.put(n, n); } Class retryClass = dataProviderMethod.retryUsing(); - boolean shouldRetry = !retryClass.equals(IRetryDataProvider.DisableDataProviderRetries.class); + boolean shouldRetry; IRetryDataProvider retry = null; - if (shouldRetry) { + if (!retryClass.equals(IRetryDataProvider.DisableDataProviderRetries.class)) { IObjectDispenser dispenser = Dispenser.newInstance(objectFactory); BasicAttributes basic = new BasicAttributes(testMethod.getTestClass(), retryClass); CreationAttributes attributes = new CreationAttributes(methodParams.context, basic, null); - retry = (IRetryDataProvider) dispenser.dispense(attributes); + retry = + (IRetryDataProvider) + Objects.requireNonNull( + dispenser.dispense(attributes), + "could not instantiate the data provider retry analyzer"); } CloseableIterator initParams = null; @@ -837,7 +841,7 @@ public static ParameterHolder handleParameters( .getInstance(), /* a test instance or null if the data provider is static*/ dataProviderMethod.getMethod(), testMethod, - methodParams.context, + methodParams.requireContext(), fedInstance, annotationFinder); shouldRetry = false; @@ -846,7 +850,7 @@ public static ParameterHolder handleParameters( for (IDataProviderListener each : holder.getListeners()) { each.onDataProviderFailure(testMethod, methodParams.context, e); } - if (shouldRetry) { + if (retry != null) { shouldRetry = retry.retry(dataProviderMethod); thrownException = e; } else { @@ -870,7 +874,8 @@ public static ParameterHolder handleParameters( // FilteredParameters and any interceptors, so the resource can be released later - including // if the setup below (listeners / filtering / interceptors) throws before a ParameterHolder // takes ownership of it. - CloseableIterator closeableSource = initParams; + CloseableIterator closeableSource = + Objects.requireNonNull(initParams, "the data provider produced no iterator"); try { for (IDataProviderListener dataProviderListener : holder.getListeners()) { dataProviderListener.afterDataProviderExecution( @@ -884,7 +889,7 @@ public static ParameterHolder handleParameters( Iterator filteredParameters = new FilteredParameters( - initParams, testMethod, dataProviderMethod.getName(), allIndices); + closeableSource, testMethod, dataProviderMethod.getName(), allIndices); testMethod.setMoreInvocationChecker(filteredParameters::hasNext); for (IDataProviderInterceptor interceptor : holder.getInterceptors()) { @@ -1001,15 +1006,25 @@ public MethodParameters( parameterValues = pv; testResult = tr; } + + /** + * The test context, for the callees that dereference it. The two-argument constructor builds + * parameters for a constructor injection, which happens before any context exists. + */ + ITestContext requireContext() { + return Objects.requireNonNull(context, "these method parameters carry no test context"); + } } private static final class ImmutableDataProvidable implements IDataProvidable { private final String dataProvider; - private final Class dataProviderClass; + private final @Nullable Class dataProviderClass; private final String dataProviderDynamicClass; private ImmutableDataProvidable( - String dataProvider, Class dataProviderClass, String dataProviderDynamicClass) { + String dataProvider, + @Nullable Class dataProviderClass, + String dataProviderDynamicClass) { this.dataProvider = dataProvider; this.dataProviderClass = dataProviderClass; this.dataProviderDynamicClass = @@ -1025,12 +1040,12 @@ public String getDataProvider() { public void setDataProvider(String v) {} @Override - public Class getDataProviderClass() { + public @Nullable Class getDataProviderClass() { return dataProviderClass; } @Override - public void setDataProviderClass(Class v) {} + public void setDataProviderClass(@Nullable Class v) {} @Override public String getDataProviderDynamicClass() { diff --git a/testng-core/src/main/java/org/testng/internal/ScriptSelectorFactory.java b/testng-core/src/main/java/org/testng/internal/ScriptSelectorFactory.java index 2fe45e334..39f5d8eef 100644 --- a/testng-core/src/main/java/org/testng/internal/ScriptSelectorFactory.java +++ b/testng-core/src/main/java/org/testng/internal/ScriptSelectorFactory.java @@ -2,6 +2,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.ServiceLoader; import javax.script.ScriptEngineFactory; import org.testng.TestNGException; @@ -39,6 +40,8 @@ public static ScriptMethodSelector getScriptSelector(XmlScript script) { + "https://github.com/cbeust/testng/wiki/Supported-script-engines"); } - return new ScriptMethodSelector(engineFactory.getScriptEngine(), script.getExpression()); + return new ScriptMethodSelector( + engineFactory.getScriptEngine(), + Objects.requireNonNull(script.getExpression(), "the