Skip to content

Commit

Permalink
Execute before/after methods in session tests in correct instance ecl…
Browse files Browse the repository at this point in the history
…ipse-platform#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
eclipse-platform#903
  • Loading branch information
HeikoKlare committed Jul 4, 2024
1 parent 390d8aa commit 89e0125
Show file tree
Hide file tree
Showing 11 changed files with 268 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,6 +36,7 @@ public class TestWorkspaceEncodingWithJvmArgs {
.withCustomization(SessionTestExtension.createCustomWorkspace()).create();

@BeforeEach
@ExecuteInHost
public void setUpSession() {
sessionTestExtension.setSystemProperty("file.encoding", "UTF-16");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand All @@ -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
Expand All @@ -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);

}
Original file line number Diff line number Diff line change
@@ -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);
}

}
Loading

0 comments on commit 89e0125

Please sign in to comment.