From 86277defc09e76e37de7c053461346b1881adf4b Mon Sep 17 00:00:00 2001
From: Sam Brannen <sbrannen@pivotal.io>
Date: Thu, 12 Mar 2020 15:24:18 +0100
Subject: [PATCH] Invoke TestInstancePreDestroyCallbacks in reverse
 registration order

Prior to this commit, when using PER_CLASS test instance lifecycle
semantics, TestInstancePreDestroyCallback extensions were invoked in
the order in which they were registered, which violates the general
"wrapping" principle for extensions invoked after a container or test.

This commit fixes this by invoking TestInstancePreDestroyCallback
extensions in reverse registration order when PER_CLASS test instance
lifecycle semantics have been configured.

Fixes #2209
---
 .../release-notes/release-notes-5.6.1.adoc      |  3 ++-
 .../descriptor/ClassBasedTestDescriptor.java    |  2 +-
 .../TestInstancePreDestroyCallbackTests.java    | 17 ++++++++++++++---
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.6.1.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.6.1.adoc
index 046ff8760d95..20a109ab6bb7 100644
--- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.6.1.adoc
+++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.6.1.adoc
@@ -31,7 +31,8 @@ on GitHub.
 
 ==== Bug Fixes
 
-* ❓
+* `TestInstancePreDestroyCallback` extensions are now invoked in reverse registration
+  order when `PER_CLASS` test instance lifecycle semantics have been configured.
 
 ==== Deprecations and Breaking Changes
 
diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java
index 9e08f5c2d307..72d4048157f3 100644
--- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java
+++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java
@@ -429,7 +429,7 @@ private void invokeTestInstancePreDestroyCallbacks(JupiterEngineExecutionContext
 		ExtensionContext extensionContext = context.getExtensionContext();
 		ThrowableCollector throwableCollector = context.getThrowableCollector();
 
-		context.getExtensionRegistry().stream(TestInstancePreDestroyCallback.class).forEach(
+		context.getExtensionRegistry().getReversedExtensions(TestInstancePreDestroyCallback.class).forEach(
 			extension -> throwableCollector.execute(() -> extension.preDestroyTestInstance(extensionContext)));
 	}
 
diff --git a/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/TestInstancePreDestroyCallbackTests.java b/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/TestInstancePreDestroyCallbackTests.java
index 5981a8576b69..26af331d941b 100644
--- a/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/TestInstancePreDestroyCallbackTests.java
+++ b/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/TestInstancePreDestroyCallbackTests.java
@@ -55,6 +55,7 @@ void instancePreDestroyCallbacksInNestedClasses() {
 			"beforeOuterMethod",
 				"beforeInnerMethod",
 					"testInner",
+				"bazPreDestroyCallbackTestInstance:InnerTestCase",
 				"barPreDestroyCallbackTestInstance:InnerTestCase",
 			"fooPreDestroyCallbackTestInstance:InnerTestCase"
 		);
@@ -77,7 +78,7 @@ void testSpecificTestInstancePreDestroyCallbackIsCalled() {
 
 	@Test
 	void classLifecyclePreDestroyCallbacks() {
-		executeTestsForClass(PerClassLifecyclePreDestroyCallbackWithTwoTestMethods.class).testEvents()//
+		executeTestsForClass(PerClassLifecyclePreDestroyCallbacksWithTwoTestMethods.class).testEvents()//
 				.assertStatistics(stats -> stats.started(2).succeeded(2));
 
 		// @formatter:off
@@ -86,7 +87,8 @@ void classLifecyclePreDestroyCallbacks() {
 				"test1",
 			"beforeEachMethod",
 				"test2",
-			"fooPreDestroyCallbackTestInstance:PerClassLifecyclePreDestroyCallbackWithTwoTestMethods"
+			"barPreDestroyCallbackTestInstance:PerClassLifecyclePreDestroyCallbacksWithTwoTestMethods",
+			"fooPreDestroyCallbackTestInstance:PerClassLifecyclePreDestroyCallbacksWithTwoTestMethods"
 		);
 		// @formatter:on
 	}
@@ -118,6 +120,7 @@ void testOuter() {
 
 		@Nested
 		@ExtendWith(BarInstancePreDestroyCallback.class)
+		@ExtendWith(BazInstancePreDestroyCallback.class)
 		class InnerTestCase extends Destroyable {
 
 			@BeforeEach
@@ -150,7 +153,8 @@ void test() {
 
 	@TestInstance(PER_CLASS)
 	@ExtendWith(FooInstancePreDestroyCallback.class)
-	static class PerClassLifecyclePreDestroyCallbackWithTwoTestMethods extends Destroyable {
+	@ExtendWith(BarInstancePreDestroyCallback.class)
+	static class PerClassLifecyclePreDestroyCallbacksWithTwoTestMethods extends Destroyable {
 
 		@BeforeEach
 		void beforeEachMethod() {
@@ -201,4 +205,11 @@ static class BarInstancePreDestroyCallback extends AbstractTestInstancePreDestro
 		}
 	}
 
+	static class BazInstancePreDestroyCallback extends AbstractTestInstancePreDestroyCallback {
+
+		BazInstancePreDestroyCallback() {
+			super("baz");
+		}
+	}
+
 }