Execute before/after methods in session tests in correct instance #903

The SessionTestExtension intercepts test execution to deploy it to a
remote executor starting a dedicated Eclipse application / session. In
both the host and the remote-execution environment, a test runner
executes the method and the interceptor decides how it is processed.
Currently, the before/after methods of the test class are not properly
considered, thus they are executed on both the host and the remote
system, which can lead to unintended behavior

This change improves consistency and comprehensibility of the session
test execution. To this end, it converts the SessionTestExtension into
an interface and splits it up into two realizations, one for the host
and one for the remote execution, each ensuring that test methods are
properly processed in their environment.
It also ensures that before/after methods are, by default, only executed
remotely, i.e., where the actual test method is executed, as these
methods usually do some recurring preparation/cleanup work that needs to
be done on the same test class instances state than on which the test is
executed. In addition, the `ExecuteInHost` annotation is introduced,
which can be attached to any before/after and ordinary test method to
execute it in the host instance instead of the remote session. This can
be used for general setup of the test instance at the host, but also for
adding some reconfiguration or validation work done on the host between
multiple sessions.

Contributes to
#903

Author: HeikoKlare

resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/TestBug294854.java

@@ -32,6 +32,7 @@
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.tests.harness.session.CustomSessionWorkspace;
+import org.eclipse.core.tests.harness.session.ExecuteInHost;
 import org.eclipse.core.tests.harness.session.SessionShouldError;
 import org.eclipse.core.tests.harness.session.SessionTestExtension;
 import org.junit.jupiter.api.BeforeEach;
@@ -83,6 +84,7 @@ private static boolean checkProjectIsOpen(String name) {
 	}
 
 	@BeforeEach
+	@ExecuteInHost
 	public void resetWorkspace(TestInfo testInfo) throws IOException {
 		if (testInfo.getTags().contains(RESET_WORKSPACE_BEFORE_TAG)) {
 			Path newWorkspace = Files.createTempDirectory(null);

resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/TestBug323833.java

@@ -30,6 +30,7 @@
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.Platform.OS;
 import org.eclipse.core.tests.harness.session.CustomSessionWorkspace;
+import org.eclipse.core.tests.harness.session.ExecuteInHost;
 import org.eclipse.core.tests.harness.session.SessionTestExtension;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.MethodOrderer;
@@ -52,6 +53,7 @@ public class TestBug323833 {
 			.withCustomization(sessionWorkspace).create();
 
 	@AfterAll
+	@ExecuteInHost
 	public static void restoreFileWriabilityForCleanup() throws CoreException, IOException {
 		sessionWorkspace.getWorkspaceDirectory().resolve(READONLY_FILE_NAME).toFile().setWritable(true, false);
 	}

resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/TestWorkspaceEncodingExistingWorkspace.java

@@ -23,6 +23,7 @@
 import org.eclipse.core.resources.IWorkspace;
 import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.tests.harness.session.CustomSessionWorkspace;
+import org.eclipse.core.tests.harness.session.ExecuteInHost;
 import org.eclipse.core.tests.harness.session.SessionTestExtension;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -40,6 +41,7 @@ public class TestWorkspaceEncodingExistingWorkspace {
 			.withCustomization(sessionWorkspace).create();
 
 	@BeforeEach
+	@ExecuteInHost
 	public void setUpWorkspace() throws IOException {
 		Path projectsTree = sessionWorkspace.getWorkspaceDirectory().resolve(".metadata/.plugins/org.eclipse.core.resources/.projects");
 		Files.createDirectories(projectsTree);

resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/TestWorkspaceEncodingWithJvmArgs.java

@@ -18,6 +18,7 @@
 
 import org.eclipse.core.resources.IWorkspace;
 import org.eclipse.core.resources.ResourcesPlugin;
+import org.eclipse.core.tests.harness.session.ExecuteInHost;
 import org.eclipse.core.tests.harness.session.SessionTestExtension;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -35,6 +36,7 @@ public class TestWorkspaceEncodingWithJvmArgs {
 			.withCustomization(SessionTestExtension.createCustomWorkspace()).create();
 
 	@BeforeEach
+	@ExecuteInHost
 	public void setUpSession() {
 		sessionTestExtension.setSystemProperty("file.encoding", "UTF-16");
 	}

resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/TestWorkspaceEncodingWithPluginCustomization.java

@@ -23,6 +23,7 @@
 import org.eclipse.core.resources.IWorkspace;
 import org.eclipse.core.resources.ResourcesPlugin;
 import org.eclipse.core.tests.harness.FileSystemHelper;
+import org.eclipse.core.tests.harness.session.ExecuteInHost;
 import org.eclipse.core.tests.harness.session.SessionTestExtension;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -41,6 +42,7 @@ public class TestWorkspaceEncodingWithPluginCustomization {
 			.withCustomization(SessionTestExtension.createCustomWorkspace()).create();
 
 	@BeforeEach
+	@ExecuteInHost
 	public void setUpSession() throws IOException {
 		// create plugin_customization.ini file
 		File file = new File(FILE_NAME);

runtime/tests/org.eclipse.core.tests.harness/src/org/eclipse/core/tests/harness/session/ExecuteInHost.java

@@ -0,0 +1,29 @@
+/*******************************************************************************
+ * Copyright (c) 2024 Vector Informatik GmbH and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *******************************************************************************/
+package org.eclipse.core.tests.harness.session;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Can be attached to any test method, before method or after method in a
+ * session test class (i.e., one using a {@link SessionTestExtension}). It
+ * defines that the annotated method is to be executed in the host Eclipse and
+ * not in the remote Eclipse application started to run a test in a separate
+ * session.
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.METHOD)
+public @interface ExecuteInHost {
+	// Marker annotation
+}

runtime/tests/org.eclipse.core.tests.harness/src/org/eclipse/core/tests/harness/session/SessionTestExtension.java

@@ -10,7 +10,6 @@
  *******************************************************************************/
 package org.eclipse.core.tests.harness.session;
 
-import java.lang.reflect.Method;
 import java.util.HashSet;
 import java.util.Objects;
 import java.util.Set;
@@ -19,14 +18,9 @@
 import org.eclipse.core.tests.harness.session.customization.CustomSessionWorkspaceImpl;
 import org.eclipse.core.tests.harness.session.customization.SessionCustomization;
 import org.eclipse.core.tests.harness.session.samples.SampleSessionTests;
-import org.eclipse.core.tests.session.Setup;
-import org.eclipse.core.tests.session.SetupManager;
-import org.eclipse.core.tests.session.SetupManager.SetupException;
 import org.junit.jupiter.api.TestInstance.Lifecycle;
 import org.junit.jupiter.api.TestMethodOrder;
-import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.api.extension.InvocationInterceptor;
-import org.junit.jupiter.api.extension.ReflectiveInvocationContext;
 
 /**
  * A JUnit 5 extension that will execute every test method in a class in its own
@@ -57,25 +51,9 @@
  * @see SessionShouldError
  *
  */
-public class SessionTestExtension implements InvocationInterceptor {
+public interface SessionTestExtension extends InvocationInterceptor {
 	public static final String CORE_TEST_APPLICATION = "org.eclipse.pde.junit.runtime.coretestapplication"; //$NON-NLS-1$
 
-	private final RemoteTestExecutor testExecutor;
-
-	private final Setup setup;
-
-	private final Set<SessionCustomization> sessionCustomizations = new HashSet<>();
-
-	private SessionTestExtension(String pluginId, String applicationId) {
-		try {
-			this.setup = SetupManager.getInstance().getDefaultSetup();
-			setup.setSystemProperty("org.eclipse.update.reconcile", "false");
-			testExecutor = new RemoteTestExecutor(setup, applicationId, pluginId);
-		} catch (SetupException e) {
-			throw new IllegalStateException("unable to create setup", e);
-		}
-	}
-
 	/**
 	 * Creates a builder for the session test extension. Make sure to finally call
 	 * {@link SessionTestExtensionBuilder#create()} to create a
@@ -194,16 +172,15 @@ public SessionTestExtensionBuilder withCustomization(CustomSessionConfiguration
 		 * this builder}
 		 */
 		public SessionTestExtension create() {
-			SessionTestExtension extension = new SessionTestExtension(storedPluginId, storedApplicationId);
+			if (RemoteTestExecutor.isRemoteExecution()) {
+				return new SessionTestExtensionRemote();
+			}
+			SessionTestExtensionHost extension = new SessionTestExtensionHost(storedPluginId, storedApplicationId);
 			storedSessionCustomizations.forEach(customization -> extension.addSessionCustomization(customization));
 			return extension;
 		}
 	}
 
-	private void addSessionCustomization(SessionCustomization sessionCustomization) {
-		this.sessionCustomizations.add(sessionCustomization);
-	}
-
 	/**
 	 * {@return a custom workspace configuration that, by default, uses a temporary
 	 * folder to store the workspace files}
@@ -228,9 +205,7 @@ public static CustomSessionConfiguration createCustomConfiguration() {
 	 * @param value the Eclipse argument value to set, may be {@code null} to remove
 	 *              the key
 	 */
-	public void setEclipseArgument(String key, String value) {
-		setup.setEclipseArgument(key, value);
-	}
+	public void setEclipseArgument(String key, String value);
 
 	/**
 	 * Sets the given system property to the given value for sessions executed with
@@ -240,44 +215,6 @@ public void setEclipseArgument(String key, String value) {
 	 * @param value the system property value to set, may be {@code null} to remove
 	 *              the key
 	 */
-	public void setSystemProperty(String key, String value) {
-		setup.setSystemProperty(key, value);
-	}
-
-	@Override
-	public void interceptTestMethod(Invocation<Void> invocation, ReflectiveInvocationContext<Method> invocationContext,
-			ExtensionContext extensionContext) throws Throwable {
-		/**
-		 * Ensure that we do not recursively make a remote call if we are already in
-		 * remote execution
-		 */
-		if (RemoteTestExecutor.isRemoteExecution()) {
-			invocation.proceed();
-			return;
-		}
-		String testClass = extensionContext.getTestClass().get().getName();
-		String testMethod = extensionContext.getTestMethod().get().getName();
-
-		boolean shouldFail = extensionContext.getTestMethod().get().getAnnotation(SessionShouldError.class) != null;
-		invocation.skip();
-		try {
-			prepareSession();
-			testExecutor.executeRemotely(testClass, testMethod, shouldFail);
-		} finally {
-			cleanupSession();
-		}
-	}
-
-	private void prepareSession() throws Exception {
-		for (SessionCustomization customization : sessionCustomizations) {
-			customization.prepareSession(setup);
-		}
-	}
-
-	private void cleanupSession() throws Exception {
-		for (SessionCustomization customization : sessionCustomizations) {
-			customization.cleanupSession(setup);
-		}
-	}
+	public void setSystemProperty(String key, String value);
 
 }

runtime/tests/org.eclipse.core.tests.harness/src/org/eclipse/core/tests/harness/session/SessionTestExtensionHost.java

@@ -0,0 +1,143 @@
+/*******************************************************************************
+ * Copyright (c) 2024 Vector Informatik GmbH and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *******************************************************************************/
+package org.eclipse.core.tests.harness.session;
+
+import java.lang.reflect.Method;
+import java.util.HashSet;
+import java.util.Set;
+import org.eclipse.core.tests.harness.session.customization.SessionCustomization;
+import org.eclipse.core.tests.session.Setup;
+import org.eclipse.core.tests.session.SetupManager;
+import org.eclipse.core.tests.session.SetupManager.SetupException;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.ReflectiveInvocationContext;
+
+/**
+ * The implementation of the {@link SessionTestExtension} to be instantiated on
+ * the host that is executing the session tests. It executes the test methods
+ * remotely in a dedicated session.
+ */
+class SessionTestExtensionHost implements SessionTestExtension {
+	public static final String CORE_TEST_APPLICATION = "org.eclipse.pde.junit.runtime.coretestapplication"; //$NON-NLS-1$
+
+	private final RemoteTestExecutor testExecutor;
+
+	private final Setup setup;
+
+	private final Set<SessionCustomization> sessionCustomizations = new HashSet<>();
+
+	SessionTestExtensionHost(String pluginId, String applicationId) {
+		try {
+			this.setup = SetupManager.getInstance().getDefaultSetup();
+			setup.setSystemProperty("org.eclipse.update.reconcile", "false");
+			testExecutor = new RemoteTestExecutor(setup, applicationId, pluginId);
+		} catch (SetupException e) {
+			throw new IllegalStateException("unable to create setup", e);
+		}
+	}
+
+	void addSessionCustomization(SessionCustomization sessionCustomization) {
+		this.sessionCustomizations.add(sessionCustomization);
+	}
+
+	/**
+	 * Sets the given Eclipse program argument to the given value for sessions
+	 * executed with this extension.
+	 *
+	 * @param key   the Eclipse argument key, must not be {@code null}
+	 * @param value the Eclipse argument value to set, may be {@code null} to remove
+	 *              the key
+	 */
+	@Override
+	public void setEclipseArgument(String key, String value) {
+		setup.setEclipseArgument(key, value);
+	}
+
+	/**
+	 * Sets the given system property to the given value for sessions executed with
+	 * this extension.
+	 *
+	 * @param key   the system property key, must not be {@code null}
+	 * @param value the system property value to set, may be {@code null} to remove
+	 *              the key
+	 */
+	@Override
+	public void setSystemProperty(String key, String value) {
+		setup.setSystemProperty(key, value);
+	}
+
+	@Override
+	public void interceptTestMethod(Invocation<Void> invocation, ReflectiveInvocationContext<Method> invocationContext,
+			ExtensionContext extensionContext) throws Throwable {
+		if (!skipIfNotExecuteInHost(invocation, invocationContext)) {
+			return;
+		}
+
+		Class<?> testClass = extensionContext.getTestClass().get();
+		Method testMethod = extensionContext.getTestMethod().get();
+
+		boolean shouldFail = extensionContext.getTestMethod().get().getAnnotation(SessionShouldError.class) != null;
+		try {
+			prepareSession();
+			testExecutor.executeRemotely(testClass.getName(), testMethod.getName(), shouldFail);
+		} finally {
+			cleanupSession();
+		}
+	}
+
+	private boolean skipIfNotExecuteInHost(Invocation<Void> invocation,
+			ReflectiveInvocationContext<Method> invocationContext) throws Throwable {
+		boolean shouldExecuteInHost = invocationContext.getExecutable().getAnnotation(ExecuteInHost.class) != null;
+		if (!shouldExecuteInHost) {
+			invocation.skip();
+			return true;
+		}
+		invocation.proceed();
+		return false;
+	}
+
+	private void prepareSession() throws Exception {
+		for (SessionCustomization customization : sessionCustomizations) {
+			customization.prepareSession(setup);
+		}
+	}
+
+	private void cleanupSession() throws Exception {
+		for (SessionCustomization customization : sessionCustomizations) {
+			customization.cleanupSession(setup);
+		}
+	}
+
+	@Override
+	public void interceptAfterAllMethod(Invocation<Void> invocation,
+			ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
+		skipIfNotExecuteInHost(invocation, invocationContext);
+	}
+
+	@Override
+	public void interceptAfterEachMethod(Invocation<Void> invocation,
+			ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
+		skipIfNotExecuteInHost(invocation, invocationContext);
+	}
+
+	@Override
+	public void interceptBeforeAllMethod(Invocation<Void> invocation,
+			ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
+		skipIfNotExecuteInHost(invocation, invocationContext);
+	}
+
+	@Override
+	public void interceptBeforeEachMethod(Invocation<Void> invocation,
+			ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext) throws Throwable {
+		skipIfNotExecuteInHost(invocation, invocationContext);
+	}
+
+}