Skip to content

Commit

Permalink
Fix intermittent test failures in Workspace(Product)ModelManagerTest
Browse files Browse the repository at this point in the history 
... due to race conditions and not thread-safe but concurrently accessed
models.

Fixes eclipse-pde#21
Fixes eclipse-pde#125
Fixes eclipse-pde#255
Fixes eclipse-pde#300
Fixes eclipse-pde#375
Fixes eclipse-pde#359
Fixes eclipse-pde#494
  • Loading branch information
@HannesWell
HannesWell committed Mar 30, 2023
1 parent bf28bcf commit f928f7f
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 21 deletions.
51 changes: 35 additions & 16 deletions ...lipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/WorkspaceModelManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.eclipse.pde.internal.core.WorkspaceModelManager;
import org.eclipse.pde.internal.core.WorkspacePluginModelManager;
import org.eclipse.pde.internal.core.project.PDEProject;
import org.eclipse.pde.ui.tests.runtime.TestUtils;
import org.eclipse.pde.ui.tests.util.ProjectUtils;
import org.junit.*;
import org.junit.rules.TestRule;
Expand Down Expand Up @@ -61,9 +62,9 @@ public void tearDown() {
public void testGetModel_projectCreated() throws CoreException {
TestWorkspaceModelManager mm = createWorkspaceModelManager();
IProject project = getWorkspaceProject("plugin.a");
assertNull(mm.getModel(project));
assertNull(getPluginModel(project, mm));
createModelProject("plugin.a", "1.0.0");
IPluginModelBase model = mm.getModel(project);
IPluginModelBase model = getPluginModel(project, mm);
assertExistingModel("plugin.a", "1.0.0", model);
}

Expand All @@ -72,7 +73,7 @@ public void testGetModel_workspaceStartUpWithExistingProject() throws CoreExcept
// simulate start-up with workspace with existing, open projects
IProject existingProject = createModelProject("plugin.a", "1.0.0");
TestWorkspaceModelManager mm = createWorkspaceModelManager(false);
IPluginModelBase model = mm.getModel(existingProject);
IPluginModelBase model = getPluginModel(existingProject, mm);
assertExistingModel("plugin.a", "1.0.0", model);
}

Expand All @@ -82,7 +83,7 @@ public void testChangeEvents_singleModelCreated() throws CoreException {
List<IModelProviderEvent> events = new ArrayList<>();
mm.addModelProviderListener(events::add);

IPluginModelBase model = mm.getModel(createModelProject("plugin.a", "1.0.0"));
IPluginModelBase model = getPluginModel(createModelProject("plugin.a", "1.0.0"), mm);

assertEquals(1, events.size());
IModelProviderEvent event = events.get(0);
Expand All @@ -105,7 +106,7 @@ public void testChangeEvents_singleModelCreated() throws CoreException {
public void testChangeEvents_singleModelRemoved() throws CoreException {
TestWorkspaceModelManager mm = createWorkspaceModelManager();
IProject project = createModelProject("plugin.a", "1.0.0");
IPluginModelBase model = mm.getModel(project);
IPluginModelBase model = getPluginModel(project, mm);
List<IModelProviderEvent> events = new ArrayList<>();
mm.addModelProviderListener(events::add);

Expand All @@ -130,7 +131,7 @@ public void testBundleRootHandling_projectCreatedWithNonDefaultBundleRoot() thro
description.setBundleRoot(bundleRootPath);
});

IPluginModelBase model = mm.getModel(project);
IPluginModelBase model = getPluginModel(project, mm);
assertExistingModel("plugin.a", "1.0.0", model);
assertFalse(manifest(project).exists());
assertTrue(project.getFile("other/root/META-INF/MANIFEST.MF").exists());
Expand All @@ -148,11 +149,11 @@ public void testOpenAndClose_projectWithChangedBundleRoot() throws Exception {

project.close(null);

assertNull(mm.getModel(project));
assertNull(getPluginModel(project, mm));

project.open(null);

IPluginModelBase model = mm.getModel(project);
IPluginModelBase model = getPluginModel(project, mm);
assertExistingModel("plugin.a", "2.0.0", model);
assertEquals(manifestIn(project, "otherRoot"), model.getUnderlyingResource());
}
Expand All @@ -167,7 +168,7 @@ public void testDelete_projectWithChangedBundleRoot() throws Exception {
manifest(project).delete(true, null);

project.delete(true, null);
assertNull(mm.getModel(project));
assertNull(getPluginModel(project, mm));
}

@Test
Expand All @@ -179,19 +180,19 @@ public void testBundleRootHandling_bundleRootChangedFromDefaultToOthersAndRevers

setBundleRoot(project, "otherRoot");

IPluginModelBase model1 = mm.getModel(project);
IPluginModelBase model1 = getPluginModel(project, mm);
assertExistingModel("plugin.a", "2.0.0", model1);
assertEquals(manifestIn(project, "otherRoot"), model1.getUnderlyingResource());

setBundleRoot(project, "root2");

IPluginModelBase model2 = mm.getModel(project);
IPluginModelBase model2 = getPluginModel(project, mm);
assertExistingModel("plugin.a", "3.0.0", model2);
assertEquals(manifestIn(project, "root2"), model2.getUnderlyingResource());

setBundleRoot(project, null);

IPluginModelBase model0 = mm.getModel(project);
IPluginModelBase model0 = getPluginModel(project, mm);
assertExistingModel("plugin.a", "1.0.0", model0);
assertEquals(manifest(project), model0.getUnderlyingResource());
}
Expand All @@ -204,11 +205,11 @@ public void testBundleRootHandling_bundleRootChangedFromNoneToOther() throws Exc
copyFile(manifest(project), manifestIn(project, "otherRoot"), replaceVersionTo("2.0.0"));

manifest(project).delete(true, null);
assertNull(mm.getModel(project));
assertNull(getPluginModel(project, mm));

setBundleRoot(project, "otherRoot");

IPluginModelBase model = mm.getModel(project);
IPluginModelBase model = getPluginModel(project, mm);
assertExistingModel("plugin.a", "2.0.0", model);
assertEquals(manifestIn(project, "otherRoot"), model.getUnderlyingResource());
}
Expand All @@ -222,11 +223,11 @@ public void testBundleRootHandling_bundleRootChangedFromNoneToDefault() throws E
setBundleRoot(project, "otherRoot");

manifestIn(project, "otherRoot").delete(true, null);
assertNull(mm.getModel(project));
assertNull(getPluginModel(project, mm));

setBundleRoot(project, null);

IPluginModelBase model = mm.getModel(project);
IPluginModelBase model = getPluginModel(project, mm);
assertExistingModel("plugin.a", "1.0.0", model);
assertEquals(manifest(project), model.getUnderlyingResource());
}
Expand Down Expand Up @@ -269,6 +270,7 @@ protected static IProject getWorkspaceProject(String name) {
private static IProject createModelProject(String symbolicName, String version) throws CoreException {
IProject project = ProjectUtils.createPluginProject(symbolicName, symbolicName, version);
project.build(IncrementalProjectBuilder.FULL_BUILD, null);
awaitJobs();
return project;
}

Expand All @@ -283,6 +285,7 @@ private static IFile manifestIn(IProject project, String path) {
private void setBundleRoot(IProject project, String path) throws CoreException {
PDEProject.setBundleRoot(project, path != null ? project.getFolder(path) : null);
project.build(IncrementalProjectBuilder.FULL_BUILD, null);
awaitJobs();
}

private void copyFile(IFile source, IFile target, UnaryOperator<String> modifications)
Expand All @@ -306,4 +309,20 @@ private static void assertExistingModel(String symbolicName, String version, IPl
assertEquals(version, model.getPluginBase().getVersion());
}

private IPluginModelBase getPluginModel(IProject project, TestWorkspaceModelManager manager) {
awaitJobs();
return manager.getModel(project);
}

static void awaitJobs() {
// TODO: Make PDE's Plugin/Feature/Product model thread-safe and this
// method obsolete
try {
// Big hammer to trigger all pending resource change events
ResourcesPlugin.getWorkspace().getRoot().refreshLocal(IResource.DEPTH_INFINITE, null);
} catch (CoreException e) { // ignore
}
TestUtils.waitForJobs(WorkspaceProductModelManagerTest.class.getName(), 100, 10000);
}

}
17 changes: 12 additions & 5 deletions ...de.ui.tests/src/org/eclipse/pde/core/tests/internal/WorkspaceProductModelManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ public void testProducts_productAddedAndRemoved() throws Exception {

createProduct(project, "aProduct");

Collection<IProductModel> products = mm.getModel(project);
Collection<IProductModel> products = getProductModel(project, mm);
assertSingleProductWithId("aProduct", products);

project.getFile("aProduct.product").delete(true, null);
assertNull(mm.getModel(project));
assertNull(getProductModel(project, mm));
}

@Test
Expand All @@ -71,7 +71,7 @@ public void testProducts_productAddedToDerivedFolder() throws Exception {

createProduct(project, "folder1/aProduct");

assertNull(mm.getModel(project));
assertNull(getProductModel(project, mm));
}

@Test
Expand All @@ -83,7 +83,7 @@ public void testProducts_productAddedToDerivedSourceFolder() throws Exception {
srcFolder.setDerived(true, null);
createProduct(project, "src/aProduct");

Collection<IProductModel> products = mm.getModel(project);
Collection<IProductModel> products = getProductModel(project, mm);
assertSingleProductWithId("aProduct", products);
}

Expand All @@ -97,7 +97,7 @@ public void testProducts_productAddedToDeriveFolderNestedInSrcFolder() throws Ex
folder.setDerived(true, null);
createProduct(project, "src/subFolder/aProduct");

Collection<IProductModel> products = mm.getModel(project);
Collection<IProductModel> products = getProductModel(project, mm);
assertSingleProductWithId("aProduct", products);
}

Expand Down Expand Up @@ -144,6 +144,7 @@ private static IProject createPluginProject(String symbolicName) throws CoreExce
IClasspathEntry cpEntry = JavaCore.newSourceEntry(srcFolder.getFullPath());
JavaCore.create(project).setRawClasspath(new IClasspathEntry[] { cpEntry }, null);
project.build(IncrementalProjectBuilder.FULL_BUILD, null);
WorkspaceModelManagerTest.awaitJobs();
return project;
}

Expand All @@ -152,4 +153,10 @@ private static void assertSingleProductWithId(String expectedId, Collection<IPro
IProductModel product = products.iterator().next();
assertEquals(expectedId, product.getProduct().getId());
}

private Collection<IProductModel> getProductModel(IProject project, TestWorkspaceProductModelManager manager) {
WorkspaceModelManagerTest.awaitJobs();
return manager.getModel(project);
}

}

0 comments on commit f928f7f

Please sign in to comment.