-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update classpath settings: overhaul to fix multiple issues #497
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
236 changes: 119 additions & 117 deletions
236
ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/ClasspathComputer.java
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
209 changes: 209 additions & 0 deletions
209
...ipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/classpathupdater/ClasspathUpdaterTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,209 @@ | ||
| package org.eclipse.pde.ui.tests.classpathupdater; | ||
|
|
||
| import static org.junit.Assert.assertEquals; | ||
| import static org.junit.Assert.assertNotNull; | ||
| import static org.junit.Assert.assertTrue; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.util.Map; | ||
| import java.util.Optional; | ||
| import java.util.function.Function; | ||
| import java.util.function.Predicate; | ||
|
|
||
| import org.eclipse.core.resources.IProject; | ||
| import org.eclipse.core.runtime.IPath; | ||
| import org.eclipse.jdt.core.IClasspathAttribute; | ||
| import org.eclipse.jdt.core.IClasspathEntry; | ||
| import org.eclipse.jdt.core.IJavaProject; | ||
| import org.eclipse.jdt.core.JavaCore; | ||
| import org.eclipse.jdt.core.JavaModelException; | ||
| import org.eclipse.pde.core.plugin.IPluginModelBase; | ||
| import org.eclipse.pde.core.plugin.PluginRegistry; | ||
| import org.eclipse.pde.internal.core.ClasspathComputer; | ||
| import org.eclipse.pde.internal.core.ICoreConstants; | ||
| import org.eclipse.pde.internal.core.PDECore; | ||
| import org.eclipse.pde.internal.ui.wizards.tools.UpdateClasspathJob; | ||
| import org.eclipse.pde.ui.tests.util.ProjectUtils; | ||
| import org.junit.After; | ||
| import org.junit.BeforeClass; | ||
| import org.junit.ClassRule; | ||
| import org.junit.Test; | ||
| import org.junit.rules.TestRule; | ||
|
|
||
| public class ClasspathUpdaterTest { | ||
|
|
||
| private static IProject project; | ||
| private static IJavaProject jProject; | ||
|
|
||
| private static IClasspathEntry[] originalClasspath; | ||
| private static String originalTestPluginPattern; | ||
| private static Boolean expectIsTest; | ||
| @ClassRule | ||
| public static final TestRule CLEAR_WORKSPACE = ProjectUtils.DELETE_ALL_WORKSPACE_PROJECTS_BEFORE_AND_AFTER; | ||
|
|
||
| @BeforeClass | ||
| public static void setUpBeforeClass() throws Exception { | ||
| project = ProjectUtils.importTestProject("tests/projects/classpathupdater"); | ||
| assertTrue("Project was not created", project.exists()); | ||
| jProject = JavaCore.create(project); | ||
| originalClasspath = jProject.getRawClasspath(); | ||
| originalTestPluginPattern = PDECore.getDefault().getPreferencesManager() | ||
| .getString(ICoreConstants.TEST_PLUGIN_PATTERN); | ||
| expectIsTest = null; | ||
| } | ||
|
|
||
| @After | ||
| public void restoreOriginalClasspath() throws Exception { | ||
| jProject.setRawClasspath(originalClasspath, null); | ||
| } | ||
|
|
||
| @After | ||
| public void restoreOriginalTestPluginPattern() { | ||
| PDECore.getDefault().getPreferencesManager().setValue(ICoreConstants.TEST_PLUGIN_PATTERN, | ||
| originalTestPluginPattern); | ||
| expectIsTest = null; | ||
| } | ||
|
|
||
| private void setTestPluginPattern() { | ||
| PDECore.getDefault().getPreferencesManager().setValue(ICoreConstants.TEST_PLUGIN_PATTERN, project.getName()); | ||
| expectIsTest = Boolean.TRUE; | ||
| } | ||
|
|
||
| @Test | ||
| public void test_PreserveAttributes() throws Exception { | ||
| runUpdateClasspathJob(); | ||
| assertClasspathAttribute("JavaSE-17", IClasspathAttribute.MODULE, true, Boolean::valueOf); | ||
| assertClasspathAttribute("library.jar", IClasspathAttribute.TEST, true, Boolean::valueOf); | ||
| assertClasspathProperty("library.jar", "exported=true", e -> e.isExported()); | ||
| assertClasspathProperty("library.jar", "no source", e -> e.getSourceAttachmentPath() == null); | ||
| assertClasspathAttribute("A.jar", IClasspathAttribute.TEST, null, Boolean::valueOf); | ||
| assertClasspathProperty("A.jar", "exported=false", e -> !e.isExported()); | ||
| assertClasspathProperty("A.jar", "default source", | ||
| e -> "Asrc.zip".equals(nullOr(e.getSourceAttachmentPath(), IPath::lastSegment))); | ||
| assertClasspathProperty("src", "exported=false", e -> !e.isExported()); | ||
| assertClasspathAttribute("src", IClasspathAttribute.IGNORE_OPTIONAL_PROBLEMS, true, Boolean::valueOf); | ||
| assertClasspathAttribute("src", IClasspathAttribute.TEST, true, Boolean::valueOf); | ||
| assertClasspathProperty("tests", "exported=false", e -> !e.isExported()); | ||
| assertClasspathAttribute("tests", IClasspathAttribute.IGNORE_OPTIONAL_PROBLEMS, null, Boolean::valueOf); | ||
| assertClasspathAttribute("tests", IClasspathAttribute.TEST, expectIsTest, Boolean::valueOf); | ||
| assertClasspathOrder("A.jar", "src", "org.eclipse.pde.core.requiredPlugins", "JavaSE-17", "SOMEVAR", | ||
| "library.jar", "tests"); | ||
| } | ||
|
|
||
| @Test | ||
| public void test_PreserveAttributes_WithTestPluginName() throws Exception { | ||
| setTestPluginPattern(); | ||
| test_PreserveAttributes(); | ||
| } | ||
|
|
||
| @Test | ||
| public void test_CreateFromScratch() throws Exception { | ||
| jProject.setRawClasspath(new IClasspathEntry[0], null); | ||
|
|
||
| runUpdateClasspathJob(); | ||
| assertClasspathAttribute("JavaSE-17", IClasspathAttribute.MODULE, null, Boolean::valueOf); | ||
| assertClasspathAttribute("library.jar", IClasspathAttribute.TEST, null, Boolean::valueOf); | ||
| assertClasspathProperty("library.jar", "exported=true", e -> e.isExported()); | ||
| assertClasspathProperty("library.jar", "no source", e -> e.getSourceAttachmentPath() == null); | ||
| assertClasspathAttribute("A.jar", IClasspathAttribute.TEST, null, Boolean::valueOf); | ||
| assertClasspathProperty("A.jar", "exported=true", e -> e.isExported()); | ||
| assertClasspathProperty("A.jar", "default source", | ||
| e -> "Asrc.zip".equals(nullOr(e.getSourceAttachmentPath(), IPath::lastSegment))); | ||
| assertClasspathProperty("src", "exported=false", e -> !e.isExported()); | ||
| assertClasspathAttribute("src", IClasspathAttribute.IGNORE_OPTIONAL_PROBLEMS, null, Boolean::valueOf); | ||
| assertClasspathAttribute("src", IClasspathAttribute.TEST, expectIsTest, Boolean::valueOf); | ||
| assertClasspathProperty("tests", "exported=false", e -> !e.isExported()); | ||
| assertClasspathAttribute("tests", IClasspathAttribute.IGNORE_OPTIONAL_PROBLEMS, null, Boolean::valueOf); | ||
| assertClasspathAttribute("tests", IClasspathAttribute.TEST, expectIsTest, Boolean::valueOf); | ||
| assertClasspathOrder("JavaSE-17", "org.eclipse.pde.core.requiredPlugins", "library.jar", "A.jar", "src", | ||
| "tests"); | ||
| } | ||
|
|
||
| @Test | ||
| public void test_CreateFromScatch_WithTestPluginName() throws Exception { | ||
| setTestPluginPattern(); | ||
| test_CreateFromScratch(); | ||
| } | ||
|
|
||
| @Test | ||
| public void test_ChangeSourceAttachment() throws Exception { | ||
| Map<String, IPath> sourceMap = Map.of( // | ||
| "A.jar", IPath.fromOSString("library.jar"), // | ||
| "library.jar", IPath.fromOSString("A.jar")); | ||
|
|
||
| IPluginModelBase model = PluginRegistry.findModel(project.getProject()); | ||
|
|
||
| IClasspathEntry[] cp = ClasspathComputer.getClasspath(project, model, sourceMap, false, false); | ||
| jProject.setRawClasspath(cp, null); | ||
|
|
||
| assertClasspathAttribute("JavaSE-17", IClasspathAttribute.MODULE, true, Boolean::valueOf); | ||
| assertClasspathAttribute("library.jar", IClasspathAttribute.TEST, true, Boolean::valueOf); | ||
| assertClasspathProperty("library.jar", "exported=true", e -> e.isExported()); | ||
| assertClasspathProperty("library.jar", "overridden source", | ||
| e -> "A.jar".equals(nullOr(e.getSourceAttachmentPath(), IPath::lastSegment))); | ||
| assertClasspathAttribute("A.jar", IClasspathAttribute.TEST, null, Boolean::valueOf); | ||
| assertClasspathProperty("A.jar", "exported=false", e -> !e.isExported()); | ||
| assertClasspathProperty("A.jar", "overridden source", | ||
| e -> "library.jar".equals(nullOr(e.getSourceAttachmentPath(), IPath::lastSegment))); | ||
| assertClasspathProperty("src", "exported=false", e -> !e.isExported()); | ||
| assertClasspathAttribute("src", IClasspathAttribute.IGNORE_OPTIONAL_PROBLEMS, true, Boolean::valueOf); | ||
| assertClasspathAttribute("src", IClasspathAttribute.TEST, true, Boolean::valueOf); | ||
| assertClasspathProperty("tests", "exported=false", e -> !e.isExported()); | ||
| assertClasspathAttribute("tests", IClasspathAttribute.IGNORE_OPTIONAL_PROBLEMS, null, Boolean::valueOf); | ||
| assertClasspathAttribute("tests", IClasspathAttribute.TEST, expectIsTest, Boolean::valueOf); | ||
| assertClasspathOrder("A.jar", "src", "org.eclipse.pde.core.requiredPlugins", "JavaSE-17", "SOMEVAR", | ||
| "library.jar", "tests"); | ||
| } | ||
|
|
||
| @Test | ||
| public void test_ChangeSourceAttachment_WithTestPluginName() throws Exception { | ||
| setTestPluginPattern(); | ||
| test_ChangeSourceAttachment(); | ||
| } | ||
|
|
||
| private void runUpdateClasspathJob() throws InterruptedException { | ||
| IPluginModelBase model = PluginRegistry.findModel(project.getProject()); | ||
| UpdateClasspathJob job = new UpdateClasspathJob(new IPluginModelBase[] { model }); | ||
| job.schedule(); | ||
| job.join(); | ||
| assertTrue("Update Classpath Job failed", job.getResult().isOK()); | ||
| } | ||
|
|
||
| private <T> void assertClasspathAttribute(String entryName, String attrName, T expectedValue, | ||
| Function<String, T> parser) throws JavaModelException { | ||
| IClasspathEntry entry = findClasspathEntry(entryName); | ||
| assertNotNull("Classpath entry for " + entryName + " is missing", entry); | ||
| String attrValue = findClasspathAttributeValue(entry, attrName); | ||
| T current = attrValue != null ? parser.apply(attrValue) : null; // null: attribute not set | ||
| assertEquals("Classpath entry for '" + entry.getPath().lastSegment() + "': attribute '" + attrName | ||
| + "' is not '" + expectedValue + "'", expectedValue, current); | ||
| } | ||
|
|
||
| private String findClasspathAttributeValue(IClasspathEntry entry, String name) { | ||
| return Arrays.stream(entry.getExtraAttributes()) // | ||
| .filter(a -> name.equals(a.getName())).map(IClasspathAttribute::getValue) // | ||
| .findFirst().orElse(null); | ||
| } | ||
|
|
||
| private void assertClasspathProperty(String entryName, String expectedValue, Predicate<IClasspathEntry> checker) | ||
| throws JavaModelException { | ||
| IClasspathEntry entry = findClasspathEntry(entryName); | ||
| assertTrue("Classpath entry for '" + entryName + "' has not set '" + expectedValue + "'", checker.test(entry)); | ||
| } | ||
|
|
||
| private void assertClasspathOrder(String... names) throws Exception { | ||
| var actualNames = Arrays.stream(jProject.getRawClasspath()).map(e -> e.getPath().lastSegment()).toList(); | ||
| assertEquals(Arrays.asList(names), actualNames); | ||
| } | ||
|
|
||
| private IClasspathEntry findClasspathEntry(String name) throws JavaModelException { | ||
| Optional<IClasspathEntry> entry = Arrays.stream(jProject.getRawClasspath()) | ||
| .filter(e -> name.equals(e.getPath().lastSegment())).findFirst(); | ||
| assertTrue("Classpath entry for " + name + " is missing", entry.isPresent()); | ||
| return entry.get(); | ||
| } | ||
|
|
||
| private static <T, R> R nullOr(T obj, Function<T, R> f) { | ||
| return obj == null ? null : f.apply(obj); | ||
| } | ||
| } | ||
23 changes: 23 additions & 0 deletions
23
ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.classpath
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <classpath> | ||
| <classpathentry kind="lib" path="A.jar"/> | ||
| <classpathentry kind="src" output="testbin" path="src"> | ||
| <attributes> | ||
| <attribute name="ignore_optional_problems" value="true"/> | ||
| <attribute name="test" value="true"/> | ||
| </attributes> | ||
| </classpathentry> | ||
| <classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/> | ||
| <classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-17"> | ||
| <attributes> | ||
| <attribute name="module" value="true"/> | ||
| </attributes> | ||
| </classpathentry> | ||
| <classpathentry kind="var" path="SOMEVAR"/> | ||
| <classpathentry exported="true" kind="lib" path="library.jar"> | ||
| <attributes> | ||
| <attribute name="test" value="true"/> | ||
| </attributes> | ||
| </classpathentry> | ||
| <classpathentry kind="output" path="bin"/> | ||
| </classpath> |
29 changes: 29 additions & 0 deletions
29
ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.project
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <projectDescription> | ||
| <name>classpathupdater</name> | ||
| <comment></comment> | ||
| <projects> | ||
| </projects> | ||
| <buildSpec> | ||
| <buildCommand> | ||
| <name>org.eclipse.jdt.core.javabuilder</name> | ||
| <arguments> | ||
| </arguments> | ||
| </buildCommand> | ||
| <buildCommand> | ||
| <name>org.eclipse.pde.ManifestBuilder</name> | ||
| <arguments> | ||
| </arguments> | ||
| </buildCommand> | ||
| <buildCommand> | ||
| <name>org.eclipse.pde.SchemaBuilder</name> | ||
| <arguments> | ||
| </arguments> | ||
| </buildCommand> | ||
| </buildSpec> | ||
| <natures> | ||
| <nature>org.eclipse.pde.PluginNature</nature> | ||
| <nature>org.eclipse.jdt.core.javanature</nature> | ||
| <nature>org.eclipse.pde.ui.tests.testNature</nature> | ||
| </natures> | ||
| </projectDescription> |
9 changes: 9 additions & 0 deletions
9
...eclipse.pde.ui.tests/tests/projects/classpathupdater/.settings/org.eclipse.jdt.core.prefs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| eclipse.preferences.version=1 | ||
| org.eclipse.jdt.core.compiler.codegen.targetPlatform=17 | ||
| org.eclipse.jdt.core.compiler.compliance=17 | ||
| org.eclipse.jdt.core.compiler.problem.assertIdentifier=error | ||
| org.eclipse.jdt.core.compiler.problem.enablePreviewFeatures=disabled | ||
| org.eclipse.jdt.core.compiler.problem.enumIdentifier=error | ||
| org.eclipse.jdt.core.compiler.problem.reportPreviewFeatures=warning | ||
| org.eclipse.jdt.core.compiler.release=enabled | ||
| org.eclipse.jdt.core.compiler.source=17 |
Binary file not shown.
Binary file not shown.
9 changes: 9 additions & 0 deletions
9
ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/META-INF/MANIFEST.MF
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| Manifest-Version: 1.0 | ||
| Bundle-ManifestVersion: 2 | ||
| Bundle-Name: Classpathupdater | ||
| Bundle-SymbolicName: classpathupdater | ||
| Bundle-Version: 1.0.0.qualifier | ||
| Bundle-ClassPath: library.jar, | ||
| A.jar, | ||
| . | ||
| Require-Bundle: org.eclipse.pde.core |
21 changes: 21 additions & 0 deletions
21
ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/build.properties
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| ############################################################################### | ||
| # Copyright (c) 2012 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 | ||
| ############################################################################### | ||
| output.. = testbin/,\ | ||
| bin/ | ||
| source.. = src/,\ | ||
| tests/ | ||
| bin.includes = META-INF/,\ | ||
| .,\ | ||
| library.jar, \ | ||
| A.jar |
Binary file added
BIN
+2.12 KB
ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/library.jar
Binary file not shown.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just added this clean-up rule so that it is ensured that the workspace is clean initially and after the test and interferences are not possible.