Being first here also means being last once {@code * TestListenerHelper#runPostConfigurationListeners} reverses the order, which is where it * belongs: whatever an internal listener drops about a finished configuration, it drops after the - * reporters have had it. + * reporters have had it. Last among these listeners, that is, and only while nothing reorders + * them -- so the guarantee is worth having but is not one to build on. {@code ListenerComparator} + * sorts this list before it is reversed and can move it anywhere; the preferential listeners are + * merged in after the regular ones; and the invoker appends {@link ConfigurationListener} after + * the reversal, which means the result is filed into {@code m_passedConfigurations} -- where the + * reports read it -- only once every listener here has been told. * * @param icl - An internal listener the other listeners depend on. */ diff --git a/testng-core/src/main/java/org/testng/internal/reporters/ParameterSnapshotReader.java b/testng-core/src/main/java/org/testng/internal/reporters/ParameterSnapshotReader.java index e34db03c9..1a63c8602 100644 --- a/testng-core/src/main/java/org/testng/internal/reporters/ParameterSnapshotReader.java +++ b/testng-core/src/main/java/org/testng/internal/reporters/ParameterSnapshotReader.java @@ -26,7 +26,12 @@ * only ever sees the run's reporters, so a class that is not one -- {@code XMLSuiteResultWriter} * and the {@code jq} panels are the ones to watch, since they hold the reads but are not reporters * -- could otherwise wear this and be quietly passed over, leaving its report to fall back to - * {@link org.testng.ITestResult#getParameters()} with nothing to say it had. + * {@link org.testng.ITestResult#getParameters()} with nothing to say it had. A read that arrives + * this late is worse off than the fallback suggests: the request below is also what tells the store + * to hold what the live reporters are finished with, so a reporter missing from this scan reads a + * store that dropped exactly the results it came for -- every configuration method that passed. + * That is what {@code Main} will have to declare when the {@code jq} report is migrated; a default + * run hides it, because {@code XMLReporter} has already asked. * *
Internal, and otherwise empty: implementing it is a statement about TestNG's own reporting,
* not an extension point. A third party reporter that wants the snapshots implements {@link
@@ -50,7 +55,7 @@ public interface ParameterSnapshotReader extends IReporter {
static void requestCaptureIfAnyReads(
Collection reporters, Collection Capture is opt-in: rendering a value runs the user's {@code toString()}, which is pure cost
* when no reporter is verbose enough to print it. A consumer says so once with {@link
- * #requestCapture()}.
+ * #requestCapture()}, or with {@link #requestCaptureHeldUntilReporting()} when it will read the
+ * store after the invocations are over -- which is also what decides whether anything may be
+ * dropped along the way. See {@link #discard}.
*/
public final class ParameterSnapshots {
@@ -40,6 +42,7 @@ public final class ParameterSnapshots {
private final Map Which is not the same as reading from inside the invocation lifecycle, tempting as that
+ * shorthand is: {@code TextReporter} reads at {@code onFinish}, once every configuration of its
+ * context has already succeeded. What makes both callers of this safe is narrower -- neither
+ * lists {@link org.testng.ITestContext#getPassedConfigurations()}, which is the only thing {@code
+ * discard} is ever offered. A reporter that starts listing them needs {@link
+ * #requestCaptureHeldUntilReporting()} instead, whenever it reads.
+ */
public void requestCapture() {
captureRequested = true;
}
/**
- * The same, for a reporter that has a suite rather than a store: a suite without one is a suite
- * whose snapshots nobody is going to take, which is not the reporter's business.
+ * The same, for a reporter that reads the store once the invocations of the run are over: nothing
+ * is dropped for such a run, since a snapshot the live reporters are finished with is one it has
+ * not seen yet.
+ *
+ * Kept as a second flag rather than as one ordered value on purpose. Both are monotone writes
+ * of {@code true} and neither clears the other, so a run that has both kinds of reader -- the
+ * default one above {@code -verbose 4} -- gets the same answer whichever asks first. Folding them
+ * into one field updated to a maximum would turn two race-free writes into a read-modify-write,
+ * and {@code Call it from {@link org.testng.ITestListener#onStart(org.testng.ITestContext)}, which is
* early enough: a context starts before its own {@code @BeforeTest} configurations and before any
@@ -104,6 +134,20 @@ public static void requestCaptureFor(@Nullable ISuite suite) {
}
}
+ /**
+ * The same, for a reporter that will read the store once the invocations of the run are over. It
+ * has no invocation lifecycle to ask from, so the run asks on its behalf; see {@link
+ * ParameterSnapshotReader#requestCaptureIfAnyReads}.
+ *
+ * @param suite - A suite about to run, none of whose invocations has started.
+ */
+ public static void requestCaptureHeldUntilReportingFor(@Nullable ISuite suite) {
+ ParameterSnapshots snapshots = of(suite);
+ if (snapshots != null) {
+ snapshots.requestCaptureHeldUntilReporting();
+ }
+ }
+
/**
* Captures what {@code result} was invoked with, unless it already has been. Must be called from
* a lifecycle point the invocation has not run past yet.
@@ -180,22 +224,34 @@ public void captureIfAbsent(ITestResult result) {
}
/**
- * Drops what was captured for a result nothing will be reported about anymore -- a configuration
- * method that succeeded, which no reporter lists once the run is over. It is the caller's
- * business to know that the live reporters are done with it; see {@link
- * ParameterSnapshotRecorder#onConfigurationSuccess}. Guarded like {@link #captureIfAbsent}:
- * {@code @BeforeMethod} / {@code @AfterMethod} invocations are the most frequent events in a run,
- * and there is nothing to drop when nothing is being captured.
- *
- * @param result - The result nothing will be reported about anymore.
+ * Offers back what was captured for a result every live reporter is done with -- a configuration
+ * method that succeeded, which each of them prints as it passes and none of them lists again.
+ *
+ * Half of that decision is the caller's and half is not, which is why it is taken here. The
+ * caller knows the reporters that sit in the invocation lifecycle have had it; only the store
+ * knows whether a reporter that has not run yet still wants it, and one that reads at {@code
+ * generateReport} does: {@code testng-results.xml} lists the configurations that passed. So a run
+ * that asked through {@link #requestCaptureHeldUntilReporting()} keeps everything until {@link
+ * #detachFrom} releases it, and a run whose readers are all live ones drops as it goes.
+ *
+ * Guarded like {@link #captureIfAbsent} for the same reason: {@code @BeforeMethod} /
+ * {@code @AfterMethod} invocations are the most frequent events in a run, and there is nothing to
+ * drop when nothing is being captured.
+ *
+ * @param result - The result the live reporters have finished with.
*/
public void discard(ITestResult result) {
- if (!captureRequested) {
+ if (!discardsWhatIsDone()) {
return;
}
snapshots.remove(new ResultKey(result));
}
+ /** Named so that {@code discard} not discarding does not read as a bug. */
+ private boolean discardsWhatIsDone() {
+ return captureRequested && !heldUntilReporting;
+ }
+
/** @return - Whether anything is still held, which is what {@link #detachFrom} leaves behind. */
public boolean isEmpty() {
return snapshots.isEmpty();
diff --git a/testng-core/src/test/java/org/testng/internal/reporters/ParameterSnapshotsTest.java b/testng-core/src/test/java/org/testng/internal/reporters/ParameterSnapshotsTest.java
index a3c1efc5c..e0ac3c871 100644
--- a/testng-core/src/test/java/org/testng/internal/reporters/ParameterSnapshotsTest.java
+++ b/testng-core/src/test/java/org/testng/internal/reporters/ParameterSnapshotsTest.java
@@ -8,6 +8,7 @@
import java.lang.reflect.Proxy;
import java.util.Arrays;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Consumer;
import org.testng.ITestClass;
import org.testng.ITestNGMethod;
import org.testng.ITestResult;
@@ -83,6 +84,51 @@ public void discardedResultsAreForgotten() {
assertThat(snapshots.find(result)).isNull();
}
+ @Test(
+ description =
+ "A store something reads once the invocations are over keeps what the live reporters are"
+ + " done with")
+ public void aStoreReadAfterTheInvocationsKeepsWhatIsOfferedBack() {
+ ITestResult result = resultOf(new CountingParameter("value"));
+ ParameterSnapshots snapshots = new ParameterSnapshots();
+ snapshots.requestCaptureHeldUntilReporting();
+ snapshots.captureIfAbsent(result);
+
+ snapshots.discard(result);
+
+ assertThat(requireNonNull(snapshots.find(result)).renderedValues()).containsExactly("value");
+ }
+
+ @Test(description = "A live reader arriving after a late one does not make the store drop again")
+ public void aLiveRequestDoesNotCancelALateOne() {
+ assertKeptAfterDiscard(
+ snapshots -> {
+ snapshots.requestCaptureHeldUntilReporting();
+ snapshots.requestCapture();
+ });
+ }
+
+ @Test(description = "A late reader arriving after a live one stops the store dropping")
+ public void aLateRequestDoesNotCancelALiveOne() {
+ assertKeptAfterDiscard(
+ snapshots -> {
+ snapshots.requestCapture();
+ snapshots.requestCaptureHeldUntilReporting();
+ });
+ }
+
+ /** Asks in the given order, then offers a captured result back and expects it kept. */
+ private static void assertKeptAfterDiscard(Consumer It is the one result the store is ever offered back, because it is the one a reporter sitting
+ * in the invocation lifecycle is finished with the moment it succeeds. Whether that offer is taken
+ * depends on the other kind of reader: {@code testng-results.xml} lists the configurations that
+ * passed, and it is written once every invocation of the run is over.
+ *
+ * Both cases are measured the same way -- a probe collects the configurations as they succeed
+ * and reads the store back from {@link IReporter#generateReport}, which is the last moment one is
+ * readable -- so what differs between them is only which request was made.
+ */
+public class PassedConfigurationSnapshotTest extends SimpleBaseTest {
+
+ @Test(
+ description =
+ "A reporter that reads once the invocations are over still finds the snapshot of a"
+ + " configuration that passed")
+ public void aLateReaderStillFindsThePassedConfiguration() {
+ LateProbe probe = new LateProbe();
+
+ run(probe);
+
+ assertThat(probe.failure).isNull();
+ assertThat(probe.renderedWhenTheReportersRan).containsExactly("[before-configuration]");
+ }
+
+ @Test(
+ description =
+ "A run whose only readers are live ones still drops it, so the store does not grow for a"
+ + " run that gains nothing from it")
+ public void aRunWithNoLateReaderStillDiscards() {
+ LifecycleProbe probe = new LifecycleProbe();
+
+ run(probe);
+
+ assertThat(probe.failure).isNull();
+ // Announced, captured, printed, and gone: the null is the discard, not a capture that never
+ // happened -- the case above proves the same invocation is snapshotted when someone reads late.
+ assertThat(probe.renderedWhenTheReportersRan).containsExactly((String) null);
+ }
+
+ @Test(
+ description =
+ "A store still holding a passed configuration when the reporters are done is released"
+ + " all the same -- the first case where detachFrom has anything to drop")
+ public void aStoreThatKeptAPassedConfigurationIsStillReleased() {
+ LateProbe probe = new LateProbe();
+
+ run(probe);
+
+ assertThat(probe.failure).isNull();
+ assertThat(probe.suite).isNotNull();
+ assertThat(ParameterSnapshots.of(probe.suite)).isNull();
+ assertThat(probe.snapshots).isNotNull();
+ assertThat(probe.snapshots.isEmpty()).isTrue();
+ }
+
+ private static void run(Probe probe) {
+ TestNG testng = create(PassingConfigurationParameterSample.class);
+ testng.addListener(probe);
+ testng.run();
+ }
+
+ /**
+ * Collects the configurations that passed while they are announced, and reads back what the store
+ * still held for them once every context had finished.
+ */
+ private abstract static class Probe implements IConfigurationListener, IReporter {
+
+ private final List Its own counter rather than the one next door, since adding a configuration method to {@link
+ * RenderingCountSample} would move the counts the tests of that sample assert on.
+ */
+public class CountedConfigurationParameterSample {
+
+ private static final AtomicInteger RENDERINGS = new AtomicInteger();
+
+ /**
+ * The count is of the class, as {@link RenderingCountSample#renderings()} explains: a caller
+ * reads it either side of the run it is measuring and takes the difference.
+ *
+ * @return - How many times a parameter of this sample has been rendered, ever.
+ */
+ public static int renderings() {
+ return RENDERINGS.get();
+ }
+
+ @DataProvider(name = "counted")
+ public static Object[][] counted() {
+ return new Object[][] {{new CountedParameter()}};
+ }
+
+ /** Handed the row its test method will run with, and passing -- it changes nothing. */
+ @BeforeMethod
+ public void prepare(Object[] parameters) {}
+
+ @Test(dataProvider = "counted")
+ public void report(CountedParameter parameter) {}
+
+ /** Reports being rendered, and is otherwise the plainest parameter there is. */
+ public static final class CountedParameter {
+
+ @Override
+ public String toString() {
+ RENDERINGS.incrementAndGet();
+ return "counted";
+ }
+ }
+}
diff --git a/testng-core/src/test/java/test/reports/XmlReporterParametersTest.java b/testng-core/src/test/java/test/reports/XmlReporterParametersTest.java
index fa1befdca..7c02d5775 100644
--- a/testng-core/src/test/java/test/reports/XmlReporterParametersTest.java
+++ b/testng-core/src/test/java/test/reports/XmlReporterParametersTest.java
@@ -20,9 +20,11 @@
import org.testng.reporters.RuntimeBehavior;
import org.testng.reporters.TextReporter;
import org.testng.reporters.XMLReporter;
+import org.testng.reporters.snapshot.CountedConfigurationParameterSample;
import org.testng.reporters.snapshot.NonCloneableParameterSample;
import org.testng.reporters.snapshot.ParallelParameterSample;
import org.testng.reporters.snapshot.ParameterShapesSample;
+import org.testng.reporters.snapshot.PassingConfigurationParameterSample;
import org.testng.reporters.snapshot.RenderingCountSample;
import org.testng.reporters.snapshot.WrongArgumentCountSample;
import org.w3c.dom.Document;
@@ -93,6 +95,20 @@ public void parallelInvocationsKeepTheirOwnValue() {
singletonList("row-3"));
}
+ @Test(
+ description =
+ "A configuration method that passed is reported with what it was announced with, which"
+ + " means its snapshot outlived the invocation that took it")
+ public void passedConfigurationParametersKeepTheirInvocationTimeValue() {
+ List