From cfb3c5f0f1aa45861b2bf07d13d7a2efca1d82ea Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Mon, 9 Oct 2023 17:44:23 +0200 Subject: [PATCH] Fix test for save state cleanup on snapshot save / bug 297635 #460 The existing test TestBug297635 relies on reflection to test some internal state change of the SavedState class that saves temporary states until some save operation. The test was prone to fail because it relied on internal state changes that depend an specific overall system state (e.g., have an unsaved workspace state, so that no concurrent automatic snapshot save is allowed to occur during test execution). It used reflection to access an internal, highly volatile state. The bug for which the test case serves as a regression test was due to missing cleanup triggered by SavedState.forgetSavedTree(). Instead of checking for internal state changes performed by the cleanup, the rewritten test only checks for a call of the according method. To this end, it temporarily inserts a spy on the SaveManager. Since a Workspace and SaveManager are not easy to set up in an isolated way for testing purposes, the test still relies on reflection, but only to inject a spy on the SaveManager rather than to validate internal states. Since the test is not required to be run as a session test anymore, it is moved to the ordinary regression resource tests. Fixes https://github.com/eclipse-platform/eclipse.platform/issues/460. --- .../META-INF/MANIFEST.MF | 1 + .../regression/AllRegressionTests.java | 5 +- .../resources/regression/Bug_297635.java | 157 +++++++++++++++ .../resources/session/AllSessionTests.java | 2 +- .../resources/session/TestBug297635.java | 190 ------------------ 5 files changed, 162 insertions(+), 193 deletions(-) create mode 100644 resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_297635.java delete mode 100644 resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/TestBug297635.java diff --git a/resources/tests/org.eclipse.core.tests.resources/META-INF/MANIFEST.MF b/resources/tests/org.eclipse.core.tests.resources/META-INF/MANIFEST.MF index cd9807aa5ae..253adf99e69 100644 --- a/resources/tests/org.eclipse.core.tests.resources/META-INF/MANIFEST.MF +++ b/resources/tests/org.eclipse.core.tests.resources/META-INF/MANIFEST.MF @@ -34,6 +34,7 @@ Require-Bundle: org.eclipse.core.resources, org.eclipse.core.filesystem, org.eclipse.core.runtime, org.eclipse.pde.junit.runtime;bundle-version="3.5.0" +Import-Package: org.mockito Bundle-ActivationPolicy: lazy Bundle-RequiredExecutionEnvironment: JavaSE-17 Eclipse-BundleShape: dir diff --git a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/AllRegressionTests.java b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/AllRegressionTests.java index fd7fe2a6cab..eb6e316a161 100644 --- a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/AllRegressionTests.java +++ b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/AllRegressionTests.java @@ -25,8 +25,9 @@ Bug_029116.class, Bug_029671.class, Bug_029851.class, Bug_032076.class, Bug_044106.class, Bug_092108.class, Bug_097608.class, Bug_098740.class, Bug_126104.class, Bug_127562.class, Bug_132510.class, Bug_134364.class, Bug_147232.class, Bug_160251.class, Bug_165892.class, Bug_192631.class, Bug_226264.class, Bug_231301.class, - Bug_233939.class, Bug_265810.class, Bug_264182.class, Bug_288315.class, Bug_303517.class, Bug_329836.class, - Bug_331445.class, Bug_332543.class, Bug_378156.class, IFileTest.class, IFolderTest.class, IProjectTest.class, + Bug_233939.class, Bug_265810.class, Bug_264182.class, Bug_297635.class, Bug_288315.class, Bug_303517.class, + Bug_329836.class, Bug_331445.class, Bug_332543.class, Bug_378156.class, + IFileTest.class, IFolderTest.class, IProjectTest.class, IResourceTest.class, IWorkspaceTest.class, LocalStoreRegressionTests.class, NLTest.class, PR_1GEAB3C_Test.class, PR_1GH2B0N_Test.class, PR_1GHOM0N_Test.class, Bug_530868.class, Bug_185247_recursiveLinks.class, diff --git a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_297635.java b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_297635.java new file mode 100644 index 00000000000..b7a497eb239 --- /dev/null +++ b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_297635.java @@ -0,0 +1,157 @@ +/******************************************************************************* + * Copyright (c) 2010, 2015 IBM Corporation and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * IBM Corporation - initial API and implementation + * Alexander Kurtakov - Bug 459343 + *******************************************************************************/ +package org.eclipse.core.tests.resources.regression; + +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +import java.lang.reflect.Field; +import java.util.function.Consumer; +import org.eclipse.core.internal.resources.SaveManager; +import org.eclipse.core.resources.ISaveContext; +import org.eclipse.core.resources.ISaveParticipant; +import org.eclipse.core.resources.IWorkspace; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.Platform; +import org.eclipse.core.tests.harness.BundleTestingHelper; +import org.eclipse.core.tests.resources.ResourceTest; +import org.eclipse.core.tests.resources.content.ContentTypeTest; +import org.osgi.framework.Bundle; +import org.osgi.framework.BundleContext; +import org.osgi.framework.BundleException; + +/** + * Tests regression of bug 297635 + */ +public class Bug_297635 extends ResourceTest { + + @Override + protected void setUp() throws Exception { + super.setUp(); + BundleWithSaveParticipant.install(); + saveFull(); + } + + @Override + protected void tearDown() throws Exception { + try { + BundleWithSaveParticipant.uninstall(); + } finally { + super.tearDown(); + } + } + + public void testCleanSaveStateBySaveParticipantOnSnapshotSave() throws Exception { + executeWithSaveManagerSpy(saveManagerSpy -> { + try { + saveSnapshot(saveManagerSpy); + } catch (CoreException e) { + } + verify(saveManagerSpy).forgetSavedTree(BundleWithSaveParticipant.getBundleName()); + }); + } + + private void saveFull() throws CoreException { + getWorkspace().save(true, getMonitor()); + } + + private void saveSnapshot(SaveManager saveManager) throws CoreException { + saveManager.save(ISaveContext.SNAPSHOT, true, null, getMonitor()); + } + + private void executeWithSaveManagerSpy(Consumer executeOnSpySaveManager) throws Exception { + IWorkspace workspace = getWorkspace(); + String saveManagerFieldName = "saveManager"; + SaveManager originalSaveManager = (SaveManager) getField(workspace, saveManagerFieldName); + SaveManager spySaveManager = spy(originalSaveManager); + try { + setField(workspace, saveManagerFieldName, spySaveManager); + executeOnSpySaveManager.accept(spySaveManager); + } finally { + setField(workspace, saveManagerFieldName, originalSaveManager); + } + } + + private static Object getField(Object object, String fieldName) throws Exception { + Field field = object.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + return field.get(object); + } + + private static void setField(Object object, String fieldName, Object value) throws Exception { + Field field = object.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(object, value); + } + + private static final class BundleWithSaveParticipant { + private static String TEST_BUNDLE_LOCATION = "content/bundle01"; + + private static Bundle bundle; + + private static ISaveParticipant saveParticipant = new ISaveParticipant() { + @Override + public void doneSaving(ISaveContext context) { + // nothing to do + } + + @Override + public void prepareToSave(ISaveContext context) { + context.needDelta(); + context.needSaveNumber(); + } + + @Override + public void rollback(ISaveContext context) { + // nothing to do + } + + @Override + public void saving(ISaveContext context) { + // nothing to do + } + }; + + public static String getBundleName() { + if (bundle == null) { + throw new IllegalStateException("Bundle has not been installed"); + } + return bundle.getSymbolicName(); + } + + public static void uninstall() throws BundleException { + if (bundle != null) { + bundle.uninstall(); + } + } + + public static void install() throws Exception { + bundle = BundleTestingHelper.installBundle("", getContext(), + ContentTypeTest.TEST_FILES_ROOT + TEST_BUNDLE_LOCATION); + BundleTestingHelper.resolveBundles(getContext(), new Bundle[] { bundle }); + bundle.start(Bundle.START_TRANSIENT); + registerSaveParticipant(bundle); + } + + private static BundleContext getContext() { + return Platform.getBundle(PI_RESOURCES_TESTS).getBundleContext(); + } + + private static void registerSaveParticipant(Bundle saveParticipantsBundle) throws CoreException { + getWorkspace().addSaveParticipant(saveParticipantsBundle.getSymbolicName(), saveParticipant); + } + + } +} diff --git a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/AllSessionTests.java b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/AllSessionTests.java index c97f899e85a..97fa896b60d 100644 --- a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/AllSessionTests.java +++ b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/AllSessionTests.java @@ -25,7 +25,7 @@ TestBug12575.class, WorkspaceDescriptionTest.class, TestBug30015.class, TestMasterTableCleanup.class, ProjectPreferenceSessionTest.class, TestBug113943.class, TestCreateLinkedResourceInHiddenProject.class, - Bug_266907.class, TestBug297635.class, TestBug323833.class, + Bug_266907.class, TestBug323833.class, org.eclipse.core.tests.resources.regression.TestMultipleBuildersOfSameType.class, org.eclipse.core.tests.resources.usecase.SnapshotTest.class, ProjectDescriptionDynamicTest.class, TestBug202384.class, TestBug369177.class, TestBug316182.class, TestBug294854.class, TestBug426263.class, diff --git a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/TestBug297635.java b/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/TestBug297635.java deleted file mode 100644 index 372845e7104..00000000000 --- a/resources/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/resources/session/TestBug297635.java +++ /dev/null @@ -1,190 +0,0 @@ -/******************************************************************************* - * Copyright (c) 2010, 2015 IBM Corporation and others. - * - * This program and the accompanying materials - * are made available under the terms of the Eclipse Public License 2.0 - * which accompanies this distribution, and is available at - * https://www.eclipse.org/legal/epl-2.0/ - * - * SPDX-License-Identifier: EPL-2.0 - * - * Contributors: - * IBM Corporation - initial API and implementation - * Alexander Kurtakov - Bug 459343 - *******************************************************************************/ -package org.eclipse.core.tests.resources.session; - -import java.io.IOException; -import java.lang.reflect.Field; -import java.net.MalformedURLException; -import java.util.Map; -import junit.framework.Test; -import org.eclipse.core.internal.resources.SaveManager; -import org.eclipse.core.internal.resources.SavedState; -import org.eclipse.core.internal.resources.Workspace; -import org.eclipse.core.resources.IFile; -import org.eclipse.core.resources.IProject; -import org.eclipse.core.resources.ISaveContext; -import org.eclipse.core.resources.ISaveParticipant; -import org.eclipse.core.resources.ResourcesPlugin; -import org.eclipse.core.runtime.CoreException; -import org.eclipse.core.runtime.Platform; -import org.eclipse.core.tests.harness.BundleTestingHelper; -import org.eclipse.core.tests.resources.AutomatedResourceTests; -import org.eclipse.core.tests.resources.ResourceTest; -import org.eclipse.core.tests.resources.content.ContentTypeTest; -import org.eclipse.core.tests.session.WorkspaceSessionTestSuite; -import org.osgi.framework.Bundle; -import org.osgi.framework.BundleContext; -import org.osgi.framework.BundleException; - -/** - * Tests regression of bug 297635 - */ -public class TestBug297635 extends ResourceTest implements ISaveParticipant { - - private static final String BUNDLE01_ID = "org.eclipse.bundle01"; - private static final String FILE = "file1.txt"; - private static final String ANOTHER_FILE = "file2.txt"; - - public static Test suite() { - return new WorkspaceSessionTestSuite(AutomatedResourceTests.PI_RESOURCES_TESTS, TestBug297635.class); - } - - public BundleContext getContext() { - return Platform.getBundle(PI_RESOURCES_TESTS).getBundleContext(); - } - - public void testBug() throws Exception { - installBundle(); - - addSaveParticipant(); - - IProject project = getProject("Project1"); - ensureExistsInWorkspace(project, true); - createFileInProject(FILE, project); - - saveFull(); - - reinstallBundle(); - - project = getProject("Project1"); - createFileInProject(ANOTHER_FILE, project); - - Map savedStates = getSavedStatesFromSaveManager(); - - addSaveParticipant(); - - assertStateTreesIsNotNull(savedStates.get(BUNDLE01_ID)); - - saveSnapshot(); - - assertStateTreesIsNull(savedStates.get(BUNDLE01_ID)); - } - - private void assertStateTreesIsNotNull(SavedState savedState) throws Exception { - assertStateTrees(savedState, false); - } - - private void assertStateTreesIsNull(SavedState savedState) throws Exception { - assertStateTrees(savedState, true); - } - - private IFile createFileInProject(String fileName, IProject project) { - IFile file = project.getFile(fileName); - ensureExistsInWorkspace(file, getRandomContents()); - return file; - } - - private IProject getProject(String projectName) { - return getWorkspace().getRoot().getProject(projectName); - } - - private void installBundle() throws BundleException, MalformedURLException, IOException { - Bundle b = BundleTestingHelper.installBundle("1", getContext(), - ContentTypeTest.TEST_FILES_ROOT + "content/bundle01"); - BundleTestingHelper.resolveBundles(getContext(), new Bundle[] { b }); - b.start(Bundle.START_TRANSIENT); - } - - private void addSaveParticipant() throws CoreException { - getWorkspace().addSaveParticipant(BUNDLE01_ID, TestBug297635.this); - } - - private void saveFull() throws CoreException { - getWorkspace().save(true, getMonitor()); - } - - private void reinstallBundle() throws BundleException, MalformedURLException, IOException { - /* - * install the bundle again. We need to restart the org.eclipse.core.resources - * bundle to read the tree file again. We rely on the fact that the - * core.resources bundle doesn't save the tree when it is stopped - */ - Bundle coreResourcesBundle = Platform.getBundle(ResourcesPlugin.PI_RESOURCES); - coreResourcesBundle.stop(Bundle.STOP_TRANSIENT); - Bundle b = BundleTestingHelper.installBundle("1", getContext(), - ContentTypeTest.TEST_FILES_ROOT + "content/bundle01"); - BundleTestingHelper.resolveBundles(getContext(), new Bundle[] { b }); - coreResourcesBundle.start(Bundle.START_TRANSIENT); - } - - @SuppressWarnings("unchecked") - private Map getSavedStatesFromSaveManager() - throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException { - // get access to SaveManager#savedStates to verify that tress are being kept - // there - Field field = SaveManager.class.getDeclaredField("savedStates"); - field.setAccessible(true); - return (Map) field.get(((Workspace) getWorkspace()).getSaveManager()); - } - - private void saveSnapshot() throws CoreException { - ((Workspace) getWorkspace()).getSaveManager().save(ISaveContext.SNAPSHOT, true, null, getMonitor()); - } - - private void assertStateTrees(SavedState savedState, boolean isNull) - throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException { - Object oldTree = null; - Object newTree = null; - - Field oldTreeField = SavedState.class.getDeclaredField("oldTree"); - oldTreeField.setAccessible(true); - oldTree = oldTreeField.get(savedState); - - Field newTreeField = SavedState.class.getDeclaredField("newTree"); - newTreeField.setAccessible(true); - newTree = newTreeField.get(savedState); - - if (isNull) { - assertNull(oldTree); - assertNull(newTree); - } else { - assertNotNull(oldTree); - assertNotNull(newTree); - } - } - - // ISaveParticipant methods - - @Override - public void doneSaving(ISaveContext context) { - // nothing to do - } - - @Override - public void prepareToSave(ISaveContext context) { - context.needDelta(); - context.needSaveNumber(); - } - - @Override - public void rollback(ISaveContext context) { - // nothing to do - } - - @Override - public void saving(ISaveContext context) { - // nothing to do - } -}