-
Notifications
You must be signed in to change notification settings - Fork 123
Add missing dependency org.eclipse.pde.api.tools.annotations to org.eclipse.pde.api.tools.tests #2226
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
Closed
Closed
Add missing dependency org.eclipse.pde.api.tools.annotations to org.eclipse.pde.api.tools.tests #2226
Changes from all commits
Commits
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
This file contains hidden or 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 |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ Require-Bundle: org.eclipse.core.runtime, | |
| org.eclipse.jdt.core.tests.builder;bundle-version="[3.8.0,4.0.0)", | ||
| org.eclipse.jdt.core.tests.compiler;bundle-version="[3.8.0,4.0.0)", | ||
| org.eclipse.ant.core, | ||
| org.eclipse.pde.api.tools.annotations, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I forgot that one in... |
||
| org.eclipse.pde.ui.tests | ||
| Bundle-RequiredExecutionEnvironment: JavaSE-21 | ||
| Export-Package: org.eclipse.pde.api.tools.anttasks.tests;uses:="org.eclipse.core.resources", | ||
|
|
||
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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.
org.eclipse.pde.api.tools.annotationsshould never be imported, it is automatically added to all PDE projects as in implicit dependency if it has the api tools nature.Also I don't see that anything has recently changed here so why should it be necessary now but not before?
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.
Should I add it to the plugin test launch configuration then?
No idea, though I doubt the test was always failing locally. Maybe its due to your change that trimmed the plugins in a plugin launch.
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.
It depends a bit, this is a compile time dependency so why is it required at all? But if it helps it might be a good way to make progress, but adding actually unused items to manifest will sooner or later make trouble as we try to actively purge unused items there.
As far as I know there are some already prepared launches for some test (@merks ?) so probably one needs to use those.
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.
There are quite some launch configurations that are checked in but I doubt they are well maintained:
I too wondered what changed that there should be a new requirement...
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.
Its a part of the test project dependencies:
Without it the compile of the test project doesn't work, by the looks of it. Other test projects also require it.
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.
There are things like this as ways to define "dependencies" that are not supposed to be runtime dependencies:
Probably the
additional-bundleswould be appropriate.But, given this is a test, I'm also not sure that we needed to be purists about that its runtime dependency should be, though if they are auto-removed later, that would be unhelpful.
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 would see if we can instead remove the imports from the test-bundles... these should no longer be needed at all (one would start with one of them to see if that's true).
In any case one would expect that pde.api tools already has a requirement (otherwise it can't process them!) so should be included in a launch that tests API tools... and it seem to work in ibuilds and maven.
Even though it would be not directly harmful, we I think we should not hide bugs/problems in PDE even though it seems easy enough. And as mentioned its likely that such unused imports are removed sooner or later then.
Uh oh!
There was an error while loading. Please reload this page.
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.
@laeubi any idea why this wouldn't work during the test (launched from Eclipse)? Is it because the launch itself doesn't have the plug-in? And so automatically adding doesn't work?
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 tried, still
DependentUsageTestsfails when ran from Eclipse. E.g.:(there are test classes that import types from
org.eclipse.pde.api.tools.annotations)The launch doesn't have the bundle selected:
I'm not seeing
org.eclipse.pde.api.tools.annotationsbeing required by any plug-in:After removing the mentions in test projects, I also don't see package imports:
The diff: