[chore] Bump AssertJ to 3.19.0, polish tests and isolate tckTest (rea…

…ctor#2706)

AssertJ has introduced a lot of interesting new assertions, including
direct Stream assertions. On the other hand, bumping the version does
include a few breaking changes.

One of these introduces a conflict with the version of TestNG that we
use, because AssertJ now prioritize the TestNG class when attempting
to produce an _assumption_ exception, but the `SkipException` that
we have in scope at runtime is too old.

That drove the change of isolating TestNG dependency from the rest of
the tests, by separating the only part that uses it (the TCK tests)
in their own testSet.

Additional polishing of tests was made to suppress warnings and adapt
to changes in AssertJ 3.19.

build.gradle

@@ -75,7 +75,7 @@ ext {
 
 	// Testing
 	jUnitPlatformVersion = '5.6.0'
-	assertJVersion = '3.11.1'
+	assertJVersion = '3.19.0' //needs to be manually synchronized in buildSrc
 	mockitoVersion = '2.23.0'
 	awaitilityVersion = '3.1.2'
 	throwingFunctionVersion = '1.5.0'
@@ -139,7 +139,6 @@ configure(subprojects) { p ->
 	test {
 		include '**/*Tests.*'
 		include '**/*Test.*'
-		include '**/*Spec.*'
 	}
 
 	//all test tasks will show FAILED for each test method,

buildSrc/build.gradle

@@ -24,7 +24,7 @@ repositories {
 }
 
 dependencies {
-	testImplementation("org.assertj:assertj-core:3.11.1")
+	testImplementation("org.assertj:assertj-core:3.19.0")
 	testImplementation platform("org.junit:junit-bom:5.6.0")
 	testImplementation "org.junit.jupiter:junit-jupiter-api"
 	testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine"

reactor-core/build.gradle

@@ -45,6 +45,7 @@ ext {
 testSets {
 	blockHoundTest
 	withMicrometerTest
+	tckTest
 }
 
 configurations {
@@ -55,7 +56,7 @@ configurations {
 dependencies {
 	// Reactive Streams
 	api "org.reactivestreams:reactive-streams:${reactiveStreamsVersion}"
-	testImplementation ("org.reactivestreams:reactive-streams-tck:${reactiveStreamsVersion}") {
+	tckTestImplementation ("org.reactivestreams:reactive-streams-tck:${reactiveStreamsVersion}") {
 		// without this exclusion, testng brings an old version of junit which *embeds* an old version of hamcrest
 		// which gets picked up first and that we don't want. TCK runs fine w/o (old) junit 4.
 		exclude group: 'junit', module: 'junit'
@@ -179,17 +180,9 @@ task loops(type: Test, group: 'verification') {
 	}
 }
 
-task reactivestreamsTCK(type: Test, group: 'verification') {
-	include '**/*Verification.*'
-	doFirst {
-		println "Additional tests from `${name}` (${includes})"
-	}
-}
-
 tasks.withType(Test).all {
-	if (it.name == "reactivestreamsTCK") {
-		useTestNG()
-	} else {
+	if (it.name == "test") {
+		//configure tag support for the core test task
 		def tags = rootProject.findProperty("junit-tags")
 		if (tags != null) {
 			println "junit5 tags for core: $tags"
@@ -201,6 +194,18 @@ tasks.withType(Test).all {
 			useJUnitPlatform()
 		}
 	}
+	else if (it.name != "tckTest") {
+		//default to JunitPlatform
+		useJUnitPlatform()
+	}
+}
+
+tckTest {
+	useTestNG()
+	include '**/*Verification.*'
+	doFirst {
+		println "Additional tests from `${name}` (${includes})"
+	}
 }
 
 
@@ -210,17 +215,21 @@ blockHoundTest {
 	maxParallelForks = 1
 }
 
-//inherit basic test task + common configuration in root
-//always depend on testNoMicrometer, skip reactivestreamsTCK on Travis, skip loops when not releasing
-//note that this way the tasks can be run individually
+jcstress {
+	mode = 'quick' //quick, default, tough
+}
+
+// inherit basic test task + common configuration in root
+// always depend on withMicrometerTest, blockHoundTest and tckTest, skip loops when not releasing
+// note that this way the tasks can be run individually
 check {
 	dependsOn withMicrometerTest
-	if (!detectedCiServers.contains("TRAVIS")) {
-		dependsOn reactivestreamsTCK
-	}
+	dependsOn blockHoundTest
+	dependsOn tckTest
 	if (!version.endsWith('BUILD-SNAPSHOT')) {
 		dependsOn loops
 	}
+	dependsOn tasks.jcstress
 }
 
 //TODO all java9 / stubs / java-specific stuff should go in a convention plugin ?
@@ -243,10 +252,6 @@ jacocoTestReport.dependsOn test
 check.dependsOn jacocoTestReport
 check.dependsOn japicmp
 
-jcstress {
-	mode = 'quick' //quick, default, tough
-}
-tasks.check.dependsOn(tasks.jcstress)
 
 if (JavaVersion.current().java9Compatible) {
 	// SharedSecretsCallSiteSupplierFactory is a Java 8 specific optimization.

reactor-core/src/test/java/reactor/core/QueueSubscriptionTest.java

@@ -79,7 +79,7 @@ public String toString() {
 				.isThrownBy(() -> {
 					assertThat(queueSubscription).isNull();
 				})
-		.withMessage("\n" + "Expecting:\n" + " <ThisIsNotAQueue>\n" + "to be equal to:\n" + " <null>\n" + "but was not.");
+		.withMessage("\n" + "expected: null\n" + "but was : ThisIsNotAQueue");
 	}
 
 }

reactor-core/src/test/java/reactor/core/ScannableTest.java

@@ -214,11 +214,11 @@ public void taggedFluxTest() {
 		                               .hide();
 
 		final Stream<Tuple2<String, String>> scannedTags1 = Scannable.from(tagged1).tags();
-		assertThat(scannedTags1.iterator())
+		assertThat(scannedTags1)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"));
 
 		final Stream<Tuple2<String, String>> scannedTags2 = Scannable.from(tagged2).tags();
-		assertThat(scannedTags2.iterator())
+		assertThat(scannedTags2)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"), Tuples.of( "2", "Two"));
 	}
 
@@ -235,11 +235,11 @@ public void taggedHideFluxTest() {
 		                               .hide();
 
 		final Stream<Tuple2<String, String>> scannedTags1 = Scannable.from(tagged1).tags();
-		assertThat(scannedTags1.iterator())
+		assertThat(scannedTags1)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"));
 
 		final Stream<Tuple2<String, String>> scannedTags2 = Scannable.from(tagged2).tags();
-		assertThat(scannedTags2.iterator())
+		assertThat(scannedTags2)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"), Tuples.of( "2", "Two"));
 	}
 
@@ -251,7 +251,7 @@ public void taggedAppendedFluxTest() {
 				    .tag("2", "Two");
 
 		final Stream<Tuple2<String, String>> scannedTags = Scannable.from(tagged1).tags();
-		assertThat(scannedTags.iterator())
+		assertThat(scannedTags)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"), Tuples.of( "2", "Two"));
 	}
 
@@ -264,7 +264,7 @@ public void taggedAppendedHideFluxTest() {
 				    .tag("2", "Two");
 
 		final Stream<Tuple2<String, String>> scannedTags = Scannable.from(tagged1).tags();
-		assertThat(scannedTags.iterator())
+		assertThat(scannedTags)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"), Tuples.of( "2", "Two"));
 	}
 
@@ -351,11 +351,11 @@ public void taggedMonoTest() {
 		                               .hide();
 
 		final Stream<Tuple2<String, String>> scannedTags1 = Scannable.from(tagged1).tags();
-		assertThat(scannedTags1.iterator())
+		assertThat(scannedTags1)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"));
 
 		final Stream<Tuple2<String, String>> scannedTags2 = Scannable.from(tagged2).tags();
-		assertThat(scannedTags2.iterator())
+		assertThat(scannedTags2)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"), Tuples.of( "2", "Two"));
 	}
 
@@ -372,11 +372,11 @@ public void taggedHideMonoTest() {
 		                               .hide();
 
 		final Stream<Tuple2<String, String>> scannedTags1 = Scannable.from(tagged1).tags();
-		assertThat(scannedTags1.iterator())
+		assertThat(scannedTags1)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"));
 
 		final Stream<Tuple2<String, String>> scannedTags2 = Scannable.from(tagged2).tags();
-		assertThat(scannedTags2.iterator())
+		assertThat(scannedTags2)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"), Tuples.of( "2", "Two"));
 	}
 
@@ -388,7 +388,7 @@ public void taggedAppendedMonoTest() {
 				    .tag("2", "Two");
 
 		final Stream<Tuple2<String, String>> scannedTags = Scannable.from(tagged1).tags();
-		assertThat(scannedTags.iterator())
+		assertThat(scannedTags)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"), Tuples.of( "2", "Two"));
 	}
 
@@ -401,7 +401,7 @@ public void taggedAppendedHideMonoTest() {
 				    .tag("2", "Two");
 
 		final Stream<Tuple2<String, String>> scannedTags = Scannable.from(tagged1).tags();
-		assertThat(scannedTags.iterator())
+		assertThat(scannedTags)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"), Tuples.of( "2", "Two"));
 	}
 
@@ -456,10 +456,10 @@ public void taggedParallelFluxTest() {
 		                               .hide();
 
 		Stream<Tuple2<String, String>> scannedTags1 = Scannable.from(tagged1).tags();
-		assertThat(scannedTags1.iterator()).containsExactlyInAnyOrder(Tuples.of("1", "One"));
+		assertThat(scannedTags1).containsExactlyInAnyOrder(Tuples.of("1", "One"));
 
 		Stream<Tuple2<String, String>> scannedTags2 = Scannable.from(tagged2).tags();
-		assertThat(scannedTags2.iterator())
+		assertThat(scannedTags2)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"), Tuples.of( "2", "Two"));
 	}
 
@@ -471,7 +471,7 @@ public void taggedAppendedParallelFluxTest() {
 				    .tag("2", "Two");
 
 		final Stream<Tuple2<String, String>> scannedTags = Scannable.from(tagged1).tags();
-		assertThat(scannedTags.iterator())
+		assertThat(scannedTags)
 				.containsExactlyInAnyOrder(Tuples.of("1", "One"), Tuples.of( "2", "Two"));
 	}
 

reactor-core/src/test/java/reactor/core/publisher/FluxFlatMapTest.java

@@ -1681,6 +1681,7 @@ public void errorModeContinueDelayErrorsWithCallable() {
 	public void errorModeContinueInternalErrorStopStrategy() {
 		for (int iterations = 0; iterations < 1000; iterations++) {
 			AtomicInteger i = new AtomicInteger();
+			@SuppressWarnings("unchecked")
 			TestPublisher<Integer>[] inners = new TestPublisher[]{
 					TestPublisher.createNoncompliant(TestPublisher.Violation.CLEANUP_ON_TERMINATE),
 					TestPublisher.createNoncompliant(TestPublisher.Violation.CLEANUP_ON_TERMINATE)
@@ -1709,6 +1710,7 @@ public void errorModeContinueInternalErrorStopStrategy() {
 	public void errorModeContinueInternalErrorStopStrategyAsync() {
 		for (int iterations = 0; iterations < 1000; iterations++) {
 			AtomicInteger i = new AtomicInteger();
+			@SuppressWarnings("unchecked")
 			TestPublisher<Integer>[] inners = new TestPublisher[]{
 				TestPublisher.createNoncompliant(TestPublisher.Violation.CLEANUP_ON_TERMINATE),
 				TestPublisher.createNoncompliant(TestPublisher.Violation.CLEANUP_ON_TERMINATE)

reactor-core/src/test/java/reactor/core/publisher/FluxFlattenIterableTest.java

@@ -16,7 +16,7 @@
 
 package reactor.core.publisher;
 
-import static org.assertj.core.api.Java6Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThat;
 
 import java.time.Duration;
 import java.util.ArrayList;

reactor-core/src/test/java/reactor/core/publisher/FluxNameFuseableTest.java

@@ -48,7 +48,7 @@ public void scanOperator() throws Exception {
 		//noinspection unchecked
 		final Stream<Tuple2<String, String>> scannedTags = test.scan(Scannable.Attr.TAGS);
 		assertThat(scannedTags).isNotNull();
-		assertThat(scannedTags.iterator()).containsExactlyInAnyOrder(tag1, tag2);
+		assertThat(scannedTags).containsExactlyInAnyOrder(tag1, tag2);
 	}
 
 	@Test

reactor-core/src/test/java/reactor/core/publisher/FluxNameTest.java

@@ -47,7 +47,7 @@ public void scanOperator() throws Exception {
 
 		final Stream<Tuple2<String, String>> scannedTags = test.scan(Scannable.Attr.TAGS);
 		assertThat(scannedTags).isNotNull();
-		assertThat(scannedTags.iterator()).containsExactlyInAnyOrder(tag1, tag2);
+		assertThat(scannedTags).containsExactlyInAnyOrder(tag1, tag2);
 	}
 
 	@Test

reactor-core/src/test/java/reactor/core/publisher/FluxOnAssemblyTest.java

@@ -241,6 +241,7 @@ public void parallelFluxCheckpointDescriptionAndForceStack() {
 		Iterator<String> lines = seekToBacktrace(debugStack);
 
 		assertThat(lines)
+				.toIterable()
 				.startsWith(
 						"|_ ParallelFlux.checkpoint ⇢ at reactor.core.publisher.FluxOnAssemblyTest.parallelFluxCheckpointDescriptionAndForceStack(FluxOnAssemblyTest.java:" + (baseline + 4) + ")",
 						"Stack trace:"

reactor-core/src/test/java/reactor/core/publisher/FluxSwitchMapTest.java

@@ -28,6 +28,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+@SuppressWarnings("deprecation")
 public class FluxSwitchMapTest {
 
 	@Test

reactor-core/src/test/java/reactor/core/publisher/FluxSwitchOnFirstTest.java

@@ -944,11 +944,12 @@ public void shouldReturnNormallyIfExceptionIsThrownOnNextDuringSwitching() {
                     .expectError(NullPointerException.class)
                     .verifyThenAssertThat(Duration.ofSeconds(5))
                     .hasOperatorErrorsSatisfying(c ->
-                        assertThat(c)
-                                  .hasOnlyOneElementSatisfying(t -> {
-                                      assertThat(t.getT1()).containsInstanceOf(NullPointerException.class);
-                                      assertThat(t.getT2()).isEqualTo(expectedCause);
-                                  })
+                            assertThat(c)
+                                    .singleElement()
+                                    .satisfies(t -> {
+                                        assertThat(t.getT1()).containsInstanceOf(NullPointerException.class);
+                                        assertThat(t.getT2()).isEqualTo(expectedCause);
+                                    })
                     );
 
 
@@ -1018,7 +1019,8 @@ public void shouldReturnNormallyIfExceptionIsThrownOnNextDuringSwitchingConditio
             .verifyThenAssertThat()
             .hasOperatorErrorsSatisfying(c ->
                 assertThat(c)
-                        .hasOnlyOneElementSatisfying(t -> {
+                        .singleElement()
+                        .satisfies(t -> {
                             assertThat(t.getT1()).containsInstanceOf(NullPointerException.class);
                             assertThat(t.getT2()).isEqualTo(expectedCause);
                         })

reactor-core/src/test/java/reactor/core/publisher/LambdaMonoSubscriberTest.java

@@ -31,7 +31,6 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.*;
-import static org.testng.Assert.assertTrue;
 
 public class LambdaMonoSubscriberTest {
 

reactor-core/src/test/java/reactor/core/publisher/MonoCreateTest.java

@@ -347,6 +347,7 @@ public void ensuresElementsIsDiscarded() {
 			Hooks.onNextDropped(collector::add);
 			AssertSubscriber<Object> assertSubscriber = new AssertSubscriber<>(Operators.enableOnDiscard(null, collector::add), 1);
 
+			@SuppressWarnings("unchecked")
 			MonoSink<Object>[] sinks = new MonoSink[1];
 
 			Mono.create(sink -> sinks[0] = sink)

reactor-core/src/test/java/reactor/core/publisher/MonoFilterTest.java

@@ -17,7 +17,7 @@
 package reactor.core.publisher;
 
 import org.junit.jupiter.api.Test;
-import org.testng.Assert;
+
 import reactor.core.Exceptions;
 import reactor.test.StepVerifier;
 import reactor.test.subscriber.AssertSubscriber;

reactor-core/src/test/java/reactor/core/publisher/MonoNameFuseableTest.java

@@ -47,7 +47,7 @@ public void scanOperator() throws Exception {
 
 		final Stream<Tuple2<String, String>> scannedTags = test.scan(Scannable.Attr.TAGS);
 		assertThat(scannedTags).isNotNull();
-		assertThat(scannedTags.iterator()).containsExactlyInAnyOrder(tag1, tag2);
+		assertThat(scannedTags).containsExactlyInAnyOrder(tag1, tag2);
 	}
 
 	@Test

reactor-core/src/test/java/reactor/core/publisher/MonoNameTest.java

@@ -47,7 +47,7 @@ public void scanOperator() throws Exception {
 
 		final Stream<Tuple2<String, String>> scannedTags = test.scan(Scannable.Attr.TAGS);
 		assertThat(scannedTags).isNotNull();
-		assertThat(scannedTags.iterator()).containsExactlyInAnyOrder(tag1, tag2);
+		assertThat(scannedTags).containsExactlyInAnyOrder(tag1, tag2);
 	}
 
 	@Test

reactor-core/src/test/java/reactor/core/publisher/MonoProcessorTest.java

@@ -36,7 +36,6 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class MonoProcessorTest {