Add TestWithDependenciesPlugin to build #22646
Conversation
This commit adds a MessyRestTestPlugin to the gradle build. It extends StandaloneRestTestPlugin. The main piece of functionality that it adds is to copy plugin-metadata from dependencies into the generated-resources for the current test source. This is necessary to ensure that permissions for dependencies are applied when running the tests. A current limitation is that the permissions are applied differently than in the distribution sources. When permissions are granted to all depedencies for a module or plugin, the permissions are granted to all dependencies on the classpath for tests besides a few hardcoded exclusions: - es core - es test framework - lucene test framework - randomized runner - junit library
Tim-Brooks
added
:Delivery/Build
|
can we have the name reflect what it does? I guess this thing does something other than being messy :) |
|
I named it following the pattern from the preexisting code. We already have a non-rest version. So I just took that convention. Is there something else you have in mind? RestTestWithDependenciesPlugin? RestTestWithESDependenciesPlugin? NonStandaloneRestTestPlugin? |
not really, everything you are proposing sounds good to me. |
Tim-Brooks
changed the title
|
Why do we need a new plugin? Can we just tweak the behavior of the existing standalone-rest-test plugin? |
|
Well - specifically I had modeled this PR on the relationship between MessyTestPlugin and StandaloneTestPlugin. It looks to me that MessyTestPlugin just extends StandaloneTestPlugin and adds the copying of dependency resources. @nik9000 told me that there was more reasons for the original MessyTestPlugin. One thing of note is that some modules which do not need/use the standalone-rest-test plugin (reindex which depends on transport-netty4 for testing) could use the functionality of copying resources of dependencies. Another approach that I had considered taking is not extending standalone-rest-test plugin and just creating a plugin to copy the resources. And that plugin can be applied as need be with no relation to other test plugins. Or I could just modify the standalone-rest-test plugin. But then I would need a way to add this functionality eventually to tests that do not use that plugin. |
I think this would be a good approach, as it would fit with the gradle model of making plugins to do one thing, and it would isolate the messiness. |
| @@ -32,6 +32,7 @@ import org.gradle.api.Project | |||
| public class RestTestPlugin implements Plugin<Project> { | |||
| List REQUIRED_PLUGINS = [ | |||
| 'elasticsearch.build', | |||
| 'elasticsearch.rest-test-with-dependencies', | |||
There was a problem hiding this comment.
This should be elasticsearch.test-with-dependencies?
There was a problem hiding this comment.
Thanks for pointing this out. I actually think that "test-with-dependencies" should not be in that list anymore. As it has no relation to the RestTestPlugin anymore.
| import org.gradle.api.tasks.Copy | ||
|
|
||
| /** | ||
| * A plugin to run messy REST tests. Messy tests are tests that depend on another |
There was a problem hiding this comment.
Update javadocs? This is not longer just for rest tests?
There was a problem hiding this comment.
I'll do that in just a minute. I'll also update the description of the PR to better reflect that changes.
Tim-Brooks
changed the title
The new plugin causes Eclipse to go a bit crazy. So we skip it for Eclipse. Relates to #22646
* master: (117 commits) Add missing serialization BWC for disk usage estimates Expose disk usage estimates in nodes stats S3 repository: Deprecate specifying credentials through env vars, sys props, and remove profile files (elastic#22567) Fix Eclipse project generation Fix deprecation logging for lenient booleans Remove @Header we no longer need Make lexer abstract [Docs] Remove outdated info about enabling/disabling doc_values (elastic#22694) Move lexer hacks to EnhancedPainlessLexer Fix incorrect args order passed to createAggregator Improve painless's javadocs Add TestWithDependenciesPlugin to build (elastic#22646) Add parsing from xContent to SearchProfileShardResults and nested classes (elastic#22649) Add unit tests for FiltersAggregator (elastic#22678) Don't register search response listener in transport clients unmute FieldStatsIntegrationIT.testGeoPointNotIndexed, fix already pushed Mute FieldStatsIntegrationIT.testGeoPointNotIndexed, for now Painless: Add augmentation to string for base 64 (elastic#22665) Fix NPE on FieldStats with mixed cluster on version pre/post 5.2 (elastic#22688) Add parsing methods for UpdateResponse (elastic#22586) ...
This commit adds a TestWithDependenciesPlugin to the gradle build.
The main piece of functionality that it adds is to copy plugin-metadata
from dependencies into the generated-resources for the current test source.
This is necessary to ensure that permissions for dependencies are applied
when running the tests.
A current limitation is that the permissions are applied differently
than in the distribution sources. When permissions are granted to all
depedencies for a module or plugin, the permissions are granted to all
dependencies on the classpath for tests besides a few hardcoded
exclusions: