diff --git a/testng-cli/src/main/java/org/testng/cli/AbstractCliRunner.java b/testng-cli/src/main/java/org/testng/cli/AbstractCliRunner.java index 5b01020d96..456c5d8b50 100644 --- a/testng-cli/src/main/java/org/testng/cli/AbstractCliRunner.java +++ b/testng-cli/src/main/java/org/testng/cli/AbstractCliRunner.java @@ -1,5 +1,6 @@ package org.testng.cli; +import org.jspecify.annotations.Nullable; import org.testng.ITestListener; import org.testng.ITestNGCliRunner; import org.testng.TestNG; @@ -27,7 +28,7 @@ public abstract class AbstractCliRunner implements ITestNGCliRunner { protected abstract CliOptions parse(String[] argv); @Override - public TestNG run(String[] argv, ITestListener listener) { + public TestNG run(String[] argv, @Nullable ITestListener listener) { TestNG result = new TestNG(); if (null != listener) { diff --git a/testng-cli/src/main/java/org/testng/cli/CliOptions.java b/testng-cli/src/main/java/org/testng/cli/CliOptions.java index b4e2d44b7d..ad645643cd 100644 --- a/testng-cli/src/main/java/org/testng/cli/CliOptions.java +++ b/testng-cli/src/main/java/org/testng/cli/CliOptions.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.List; +import org.jspecify.annotations.Nullable; import org.testng.xml.XmlSuite; /** @@ -71,52 +72,52 @@ public class CliOptions { public List suiteFiles = new ArrayList<>(); /** Level of verbosity. */ - public Integer verbose; + public @Nullable Integer verbose; /** Comma-separated list of group names to be run. */ - public String groups; + public @Nullable String groups; /** Comma-separated list of group names to exclude. */ - public String excludedGroups; + public @Nullable String excludedGroups; /** Output directory. */ - public String outputDirectory; + public @Nullable String outputDirectory; /** * List of {@code .class} files or list of class names implementing {@code ITestListener} or * {@code ISuiteListener}. */ - public String listener; + public @Nullable String listener; /** An implementation of {@code ListenerComparator} that orders listener execution. */ - public String listenerComparator; + public @Nullable String listenerComparator; /** List of {@code .class} files or list of class names implementing {@code IMethodSelector}. */ - public String methodSelectors; + public @Nullable String methodSelectors; /** Fully qualified class name that implements {@code org.testng.ITestObjectFactory}. */ - public String objectFactory; + public @Nullable String objectFactory; /** Parallel mode (methods, tests or classes). */ - public XmlSuite.ParallelMode parallelMode; + public XmlSuite.@Nullable ParallelMode parallelMode; /** Configuration failure policy (skip or continue). */ - public String configFailurePolicy; + public @Nullable String configFailurePolicy; /** Number of threads to use when running tests in parallel. */ - public Integer threadCount; + public @Nullable Integer threadCount; /** Number of threads to use when running data providers. */ - public Integer dataProviderThreadCount; + public @Nullable Integer dataProviderThreadCount; /** Default name of test suite, if not specified in suite definition file or source code. */ - public String suiteName; + public @Nullable String suiteName; /** Default name of test, if not specified in suite definition file or source code. */ - public String testName; + public @Nullable String testName; /** Extended configuration for custom report listener. */ - public String reporter; + public @Nullable String reporter; /** * Whether to use the default listeners. This is a {@code String} because the option has an arity @@ -124,28 +125,28 @@ public class CliOptions { */ public String useDefaultListeners = "true"; - public Boolean skipFailedInvocationCounts; + public @Nullable Boolean skipFailedInvocationCounts; /** The list of test classes. */ - public String testClass; + public @Nullable String testClass; /** The list of test names to run. */ - public String testNames; + public @Nullable String testNames; /** Ignore missed test names given by {@code -testnames} and continue to run existing tests. */ public boolean ignoreMissedTestNames = false; /** A jar file containing the tests. */ - public String testJar; + public @Nullable String testJar; /** The full path to the xml file inside the jar file, only valid with {@code -testjar}. */ public String xmlPathInJar = XML_PATH_IN_JAR_DEFAULT; /** The factory used to create tests. */ - public String testRunnerFactory; + public @Nullable String testRunnerFactory; /** The factory used to create TestNG listeners. */ - public String listenerFactory; + public @Nullable String listenerFactory; /** Comma separated list of test methods. */ public List commandLineMethods = new ArrayList<>(); @@ -160,10 +161,10 @@ public class CliOptions { public Boolean alwaysRunListeners = Boolean.TRUE; /** The threadpool executor factory implementation that TestNG should use. */ - public String threadPoolFactoryClass; + public @Nullable String threadPoolFactoryClass; /** The dependency injector factory implementation that TestNG should use. */ - public String dependencyInjectorFactoryClass; + public @Nullable String dependencyInjectorFactoryClass; /** Should TestNG fail execution if all tests were skipped and nothing was run. */ public Boolean failIfAllTestsSkipped = false; diff --git a/testng-cli/src/main/java/org/testng/cli/CliParseException.java b/testng-cli/src/main/java/org/testng/cli/CliParseException.java index aa65abb6cb..4e82141813 100644 --- a/testng-cli/src/main/java/org/testng/cli/CliParseException.java +++ b/testng-cli/src/main/java/org/testng/cli/CliParseException.java @@ -1,5 +1,6 @@ package org.testng.cli; +import org.jspecify.annotations.Nullable; import org.testng.TestNGException; /** @@ -16,7 +17,12 @@ public CliParseException(String message) { super(message); } - public CliParseException(String message, Throwable cause) { + /** + * Only this overload takes a nullable message: it exists to wrap another exception, and {@link + * Throwable#getMessage()} is allowed to return {@code null}. A front end raising a parse failure + * on its own has a message to give and uses {@link #CliParseException(String)}. + */ + public CliParseException(@Nullable String message, Throwable cause) { super(message, cause); } } diff --git a/testng-cli/src/main/java/org/testng/cli/package-info.java b/testng-cli/src/main/java/org/testng/cli/package-info.java new file mode 100644 index 0000000000..7689d0f11d --- /dev/null +++ b/testng-cli/src/main/java/org/testng/cli/package-info.java @@ -0,0 +1,5 @@ +/** The parser agnostic command line contract, and the configuration it drives. */ +@NullMarked +package org.testng.cli; + +import org.jspecify.annotations.NullMarked; diff --git a/testng-collections/src/main/java/org/testng/collections/CollectionUtils.java b/testng-collections/src/main/java/org/testng/collections/CollectionUtils.java index bf47ee9a08..da62f1df66 100644 --- a/testng-collections/src/main/java/org/testng/collections/CollectionUtils.java +++ b/testng-collections/src/main/java/org/testng/collections/CollectionUtils.java @@ -3,16 +3,17 @@ import java.util.Collection; import java.util.Iterator; import java.util.Map; +import org.jspecify.annotations.Nullable; public final class CollectionUtils { private CollectionUtils() {} - public static boolean hasElements(Collection c) { + public static boolean hasElements(@Nullable Collection c) { return c != null && !c.isEmpty(); } - public static boolean hasElements(Map c) { + public static boolean hasElements(@Nullable Map c) { return c != null && !c.isEmpty(); } 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 e859cd8008..7216196c90 100644 --- a/testng-collections/src/main/java/org/testng/collections/MultiMap.java +++ b/testng-collections/src/main/java/org/testng/collections/MultiMap.java @@ -7,6 +7,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; +import org.jspecify.annotations.Nullable; public abstract class MultiMap> { protected final Map m_objects; @@ -52,7 +53,7 @@ public String toString() { Set indices = keySet(); for (K i : indices) { result.append("\n ").append(i).append(" <-- "); - for (Object o : m_objects.get(i)) { + for (Object o : get(i)) { result.append(o).append(" "); } } @@ -71,7 +72,13 @@ public boolean remove(K key, V value) { return get(key).remove(value); } - public C removeAll(K key) { + /** + * Drops a key and every value held for it. + * + * @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) { return m_objects.remove(key); } diff --git a/testng-collections/src/main/java/org/testng/collections/Objects.java b/testng-collections/src/main/java/org/testng/collections/Objects.java index a42d417cfe..7097f1671a 100644 --- a/testng-collections/src/main/java/org/testng/collections/Objects.java +++ b/testng-collections/src/main/java/org/testng/collections/Objects.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.List; +import org.jspecify.annotations.Nullable; import org.testng.util.Strings; public final class Objects { @@ -51,17 +52,17 @@ public ToStringHelper omitEmptyStrings() { return this; } - public ToStringHelper add(String name, String value) { + public ToStringHelper add(String name, @Nullable String value) { values.add(new ValueHolder(name, s(value))); return this; } - public ToStringHelper add(String name, Object value) { + public ToStringHelper add(String name, @Nullable Object value) { values.add(new ValueHolder(name, s(value))); return this; } - private String s(Object o) { + private String s(@Nullable Object o) { return o != null ? (o.toString().isEmpty() ? "\"\"" : o.toString()) : "{null}"; } diff --git a/testng-collections/src/main/java/org/testng/collections/package-info.java b/testng-collections/src/main/java/org/testng/collections/package-info.java new file mode 100644 index 0000000000..f1753b1a7f --- /dev/null +++ b/testng-collections/src/main/java/org/testng/collections/package-info.java @@ -0,0 +1,5 @@ +/** Small collection factories and containers shared across TestNG. */ +@NullMarked +package org.testng.collections; + +import org.jspecify.annotations.NullMarked; diff --git a/testng-jcommander/src/main/java/org/testng/cli/jcommander/Converter.java b/testng-jcommander/src/main/java/org/testng/cli/jcommander/Converter.java index 3ec84a005b..edcf793301 100644 --- a/testng-jcommander/src/main/java/org/testng/cli/jcommander/Converter.java +++ b/testng-jcommander/src/main/java/org/testng/cli/jcommander/Converter.java @@ -6,6 +6,7 @@ import java.io.File; import java.io.FileWriter; import java.io.IOException; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashSet; @@ -26,8 +27,11 @@ */ public class Converter { + // Pre-seeded rather than left null: JCommander reuses a main parameter list it finds in place + // and clears it before the first value, so the empty list changes nothing -- and `required` is + // enforced from the parameter description, not from this field being null. @Parameter(description = "file1 [file2 file3...]", required = true) - private List m_files; + private List m_files = new ArrayList<>(); @Parameter(names = "-d", description = "The directory where the file(s) will be created") private String m_outputDirectory = "."; diff --git a/testng-jcommander/src/main/java/org/testng/cli/jcommander/JCommanderOptions.java b/testng-jcommander/src/main/java/org/testng/cli/jcommander/JCommanderOptions.java index a8efcabd8c..c0f538e1f7 100644 --- a/testng-jcommander/src/main/java/org/testng/cli/jcommander/JCommanderOptions.java +++ b/testng-jcommander/src/main/java/org/testng/cli/jcommander/JCommanderOptions.java @@ -3,6 +3,7 @@ import com.beust.jcommander.Parameter; import java.util.ArrayList; import java.util.List; +import org.jspecify.annotations.Nullable; import org.testng.cli.CliOptions; import org.testng.xml.XmlSuite; @@ -20,20 +21,20 @@ public class JCommanderOptions { @Parameter( names = {CliOptions.LOG, CliOptions.VERBOSE}, description = "Level of verbosity") - public Integer verbose; + public @Nullable Integer verbose; @Parameter( names = CliOptions.GROUPS, description = "Comma-separated list of group names to be run") - public String groups; + public @Nullable String groups; @Parameter( names = CliOptions.EXCLUDED_GROUPS, description = "Comma-separated list of group names to " + " exclude") - public String excludedGroups; + public @Nullable String excludedGroups; @Parameter(names = CliOptions.OUTPUT_DIRECTORY, description = "Output directory") - public String outputDirectory; + public @Nullable String outputDirectory; /** Accepted for backwards compatibility; TestNG has never acted on it. */ @Parameter( @@ -48,60 +49,60 @@ public class JCommanderOptions { description = "List of .class files or list of class names" + " implementing ITestListener or ISuiteListener") - public String listener; + public @Nullable String listener; @Parameter( names = CliOptions.LISTENER_COMPARATOR, description = "An implementation of ListenerComparator that will be used by TestNG to determine order of execution for listeners") - public String listenerComparator; + public @Nullable String listenerComparator; @Parameter( names = CliOptions.METHOD_SELECTORS, description = "List of .class files or list of class " + "names implementing IMethodSelector") - public String methodSelectors; + public @Nullable String methodSelectors; @Parameter( names = CliOptions.OBJECT_FACTORY, description = "Fully qualified class name that implements org.testng.ITestObjectFactory which can be used to create test class and listener instances.") - public String objectFactory; + public @Nullable String objectFactory; @Parameter(names = CliOptions.PARALLEL, description = "Parallel mode (methods, tests or classes)") - public XmlSuite.ParallelMode parallelMode; + public XmlSuite.@Nullable ParallelMode parallelMode; @Parameter( names = CliOptions.CONFIG_FAILURE_POLICY, description = "Configuration failure policy (skip or continue)") - public String configFailurePolicy; + public @Nullable String configFailurePolicy; @Parameter( names = CliOptions.THREAD_COUNT, description = "Number of threads to use when running tests " + "in parallel") - public Integer threadCount; + public @Nullable Integer threadCount; @Parameter( names = CliOptions.DATA_PROVIDER_THREAD_COUNT, description = "Number of threads to use when " + "running data providers") - public Integer dataProviderThreadCount; + public @Nullable Integer dataProviderThreadCount; @Parameter( names = CliOptions.SUITE_NAME, description = "Default name of test suite, if not specified " + "in suite definition file or source code") - public String suiteName; + public @Nullable String suiteName; @Parameter( names = CliOptions.TEST_NAME, description = "Default name of test, if not specified in suite" + "definition file or source code") - public String testName; + public @Nullable String testName; @Parameter( names = CliOptions.REPORTER, description = "Extended configuration for custom report listener") - public String reporter; + public @Nullable String reporter; @Parameter( names = CliOptions.USE_DEFAULT_LISTENERS, @@ -109,13 +110,13 @@ public class JCommanderOptions { public String useDefaultListeners = "true"; @Parameter(names = CliOptions.SKIP_FAILED_INVOCATION_COUNTS, hidden = true) - public Boolean skipFailedInvocationCounts; + public @Nullable Boolean skipFailedInvocationCounts; @Parameter(names = CliOptions.TEST_CLASS, description = "The list of test classes") - public String testClass; + public @Nullable String testClass; @Parameter(names = CliOptions.TEST_NAMES, description = "The list of test names to run") - public String testNames; + public @Nullable String testNames; @Parameter( names = CliOptions.IGNORE_MISSED_TEST_NAMES, @@ -124,7 +125,7 @@ public class JCommanderOptions { public boolean ignoreMissedTestNames = false; @Parameter(names = CliOptions.TEST_JAR, description = "A jar file containing the tests") - public String testJar; + public @Nullable String testJar; @Parameter( names = CliOptions.XML_PATH_IN_JAR, @@ -135,12 +136,12 @@ public class JCommanderOptions { @Parameter( names = {CliOptions.TEST_RUNNER_FACTORY, "-testRunFactory"}, description = "The factory used to create tests") - public String testRunnerFactory; + public @Nullable String testRunnerFactory; @Parameter( names = CliOptions.LISTENER_FACTORY, description = "The factory used to create TestNG listeners") - public String listenerFactory; + public @Nullable String listenerFactory; @Parameter(names = CliOptions.METHODS, description = "Comma separated of test methods") public List commandLineMethods = new ArrayList<>(); @@ -164,12 +165,12 @@ public class JCommanderOptions { @Parameter( names = CliOptions.THREAD_POOL_FACTORY_CLASS, description = "The threadpool executor factory implementation that TestNG should use.") - public String threadPoolFactoryClass; + public @Nullable String threadPoolFactoryClass; @Parameter( names = CliOptions.DEPENDENCY_INJECTOR_FACTORY, description = "The dependency injector factory implementation that TestNG should use.") - public String dependencyInjectorFactoryClass; + public @Nullable String dependencyInjectorFactoryClass; @Parameter( names = CliOptions.FAIL_IF_ALL_TESTS_SKIPPED, diff --git a/testng-jcommander/src/main/java/org/testng/cli/jcommander/package-info.java b/testng-jcommander/src/main/java/org/testng/cli/jcommander/package-info.java new file mode 100644 index 0000000000..c507654c07 --- /dev/null +++ b/testng-jcommander/src/main/java/org/testng/cli/jcommander/package-info.java @@ -0,0 +1,5 @@ +/** The JCommander backed command line front end. */ +@NullMarked +package org.testng.cli.jcommander; + +import org.jspecify.annotations.NullMarked; diff --git a/testng-jcommander/src/test/java/org/testng/cli/jcommander/ConverterMainParameterTest.java b/testng-jcommander/src/test/java/org/testng/cli/jcommander/ConverterMainParameterTest.java new file mode 100644 index 0000000000..5ea0ec5f92 --- /dev/null +++ b/testng-jcommander/src/test/java/org/testng/cli/jcommander/ConverterMainParameterTest.java @@ -0,0 +1,42 @@ +package org.testng.cli.jcommander; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.beust.jcommander.JCommander; +import com.beust.jcommander.ParameterException; +import java.lang.reflect.Field; +import java.util.List; +import org.testng.annotations.Test; + +/** + * {@link Converter}'s main parameter starts out as an empty list rather than as {@code null}, so + * that the class can be null-marked without annotating a field JCommander always fills. Both halves + * of "the seeding is invisible" are pinned here, because both are JCommander's behaviour rather + * than TestNG's: a seeded list is reused and cleared, and {@code required} is decided from the + * parameter description rather than from the field being null. + */ +public class ConverterMainParameterTest { + + @Test + public void aSeededMainParameterStillEnforcesRequired() { + assertThatThrownBy(() -> new JCommander(new Converter()).parse()) + .isInstanceOf(ParameterException.class); + } + + @Test + public void aSeededMainParameterCollectsExactlyTheValuesGiven() throws Exception { + Converter converter = new Converter(); + + new JCommander(converter).parse("a.xml", "b.yaml"); + + assertThat(mainParameterOf(converter)).containsExactly("a.xml", "b.yaml"); + } + + @SuppressWarnings("unchecked") + private static List mainParameterOf(Converter converter) throws Exception { + Field field = Converter.class.getDeclaredField("m_files"); + field.setAccessible(true); + return (List) field.get(converter); + } +}