Skip to content

Commit

Permalink
Apply precommit checks for non productive projects (#85533) (#86027)
Browse files Browse the repository at this point in the history
Ensure projects with only yaml, java or cluster tests also apply precommit checks.
We only apply testingconventions now for projects with existing src test folder
as the TestingConventionsTask is incompatible with projects with no test sourceSet.
This is a prerequisite to port more projects away from using StandaloneRestTestPlugin
and RestTestPlugin in favor of yaml, java or cluster tests with dedicated sourceSets.

Also we fix deprecation warnings for forbiddenPattern and filePermissions tasks about
implicit non declared dependencies on resources tasks
  • Loading branch information
breskeby committed Apr 20, 2022
1 parent a8be2fa commit b174af6
Show file tree
Hide file tree
Showing 20 changed files with 161 additions and 128 deletions.
Expand Up @@ -8,6 +8,7 @@

package org.elasticsearch.gradle.internal;

import org.elasticsearch.gradle.internal.precommit.TestingConventionsPrecommitPlugin;
import org.elasticsearch.gradle.internal.precommit.TestingConventionsTasks;
import org.gradle.api.Project;

Expand All @@ -17,15 +18,22 @@ public void apply(Project project) {
project.getPluginManager().apply(BuildPlugin.class);
project.getPluginManager().apply(BaseInternalPluginBuildPlugin.class);

project.getTasks().withType(TestingConventionsTasks.class).named("testingConventions").configure(t -> {
t.getNaming().clear();
t.getNaming()
.create("Tests", testingConventionRule -> testingConventionRule.baseClass("org.apache.lucene.tests.util.LuceneTestCase"));
t.getNaming().create("IT", testingConventionRule -> {
testingConventionRule.baseClass("org.elasticsearch.test.ESIntegTestCase");
testingConventionRule.baseClass("org.elasticsearch.test.rest.ESRestTestCase");
testingConventionRule.baseClass("org.elasticsearch.test.ESSingleNodeTestCase");
});
});
project.getPlugins()
.withType(
TestingConventionsPrecommitPlugin.class,
plugin -> project.getTasks().withType(TestingConventionsTasks.class).named("testingConventions").configure(t -> {
t.getNaming().clear();
t.getNaming()
.create(
"Tests",
testingConventionRule -> testingConventionRule.baseClass("org.apache.lucene.tests.util.LuceneTestCase")
);
t.getNaming().create("IT", testingConventionRule -> {
testingConventionRule.baseClass("org.elasticsearch.test.ESIntegTestCase");
testingConventionRule.baseClass("org.elasticsearch.test.rest.ESRestTestCase");
testingConventionRule.baseClass("org.elasticsearch.test.ESSingleNodeTestCase");
});
})
);
}
}
Expand Up @@ -32,16 +32,19 @@ public FilePermissionsPrecommitPlugin(ProviderFactory providerFactory) {

@Override
public TaskProvider<? extends Task> createTask(Project project) {
return project.getTasks()
.register(
FILEPERMISSIONS_TASK_NAME,
FilePermissionsTask.class,
filePermissionsTask -> filePermissionsTask.getSources()
.addAll(
providerFactory.provider(
() -> GradleUtils.getJavaSourceSets(project).stream().map(s -> s.getAllSource()).collect(Collectors.toList())
)
return project.getTasks().register(FILEPERMISSIONS_TASK_NAME, FilePermissionsTask.class, t -> {
t.getSources()
.addAll(
providerFactory.provider(
() -> GradleUtils.getJavaSourceSets(project).stream().map(s -> s.getAllSource()).collect(Collectors.toList())
)
);
t.dependsOn(
GradleUtils.getJavaSourceSets(project)
.stream()
.map(sourceSet -> sourceSet.getProcessResourcesTaskName())
.collect(Collectors.toList())
);
});
}
}
Expand Up @@ -39,6 +39,12 @@ public TaskProvider<? extends Task> createTask(Project project) {
() -> GradleUtils.getJavaSourceSets(project).stream().map(s -> s.getAllSource()).collect(Collectors.toList())
)
);
forbiddenPatternsTask.dependsOn(
GradleUtils.getJavaSourceSets(project)
.stream()
.map(sourceSet -> sourceSet.getProcessResourcesTaskName())
.collect(Collectors.toList())
);
forbiddenPatternsTask.getRootDir().set(project.getRootDir());
});
}
Expand Down
Expand Up @@ -66,6 +66,7 @@ public abstract class ForbiddenPatternsTask extends DefaultTask {
.exclude("**/*.zip")
.exclude("**/*.jks")
.exclude("**/*.crt")
.exclude("**/*.p12")
.exclude("**/*.keystore")
.exclude("**/*.png")
// vim swap file - included here to stop the build falling over if you happen to have a file open :-|
Expand Down
Expand Up @@ -25,13 +25,22 @@ public static void create(Project project, boolean withProductiveCode) {
project.getPluginManager().apply(ForbiddenPatternsPrecommitPlugin.class);
project.getPluginManager().apply(LicenseHeadersPrecommitPlugin.class);
project.getPluginManager().apply(FilePermissionsPrecommitPlugin.class);
project.getPluginManager().apply(TestingConventionsPrecommitPlugin.class);
project.getPluginManager().apply(LoggerUsagePrecommitPlugin.class);
project.getPluginManager().apply(JarHellPrecommitPlugin.class);

// TestingConventionsPlugin is incompatible with projects without
// test source sets. We wanna remove this plugin once we moved away from
// StandaloneRestTest plugin and RestTestPlugin. For apply this plugin only
// when tests are available.
// Long Term we want remove the need for most of this plugins functionality
// and not rely on multiple test tasks against a sourceSet.
if (project.file("src/test").exists()) {
project.getPluginManager().apply(TestingConventionsPrecommitPlugin.class);
}

// tasks with just tests don't need certain tasks to run, so this flag makes adding
// the task optional
if (withProductiveCode) {
project.getPluginManager().apply(JarHellPrecommitPlugin.class);
project.getPluginManager().apply(ThirdPartyAuditPrecommitPlugin.class);
project.getPluginManager().apply(DependencyLicensesPrecommitPlugin.class);
project.getPluginManager().apply(SplitPackagesAuditPrecommitPlugin.class);
Expand Down
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.gradle.internal.ElasticsearchTestBasePlugin;
import org.elasticsearch.gradle.internal.FixtureStop;
import org.elasticsearch.gradle.internal.InternalTestClustersPlugin;
import org.elasticsearch.gradle.internal.precommit.InternalPrecommitTasks;
import org.elasticsearch.gradle.test.SystemPropertyCommandLineArgumentProvider;
import org.elasticsearch.gradle.testclusters.ElasticsearchCluster;
import org.elasticsearch.gradle.testclusters.StandaloneRestIntegTestTask;
Expand Down Expand Up @@ -48,6 +49,7 @@ public void apply(Project project) {
project.getPluginManager().apply(ElasticsearchJavaBasePlugin.class);
project.getPluginManager().apply(ElasticsearchTestBasePlugin.class);
project.getPluginManager().apply(InternalTestClustersPlugin.class);
InternalPrecommitTasks.create(project, false);
project.getTasks().withType(RestIntegTestTask.class).configureEach(restIntegTestTask -> {
@SuppressWarnings("unchecked")
NamedDomainObjectContainer<ElasticsearchCluster> testClusters = (NamedDomainObjectContainer<ElasticsearchCluster>) project
Expand Down
Expand Up @@ -185,6 +185,7 @@ public void execute(Task task) {
});
}
});

final var buildProperties = project.getTasks().register("pluginProperties", Copy.class, copy -> {
copy.dependsOn(copyPluginPropertiesTemplate);
copy.from(templateFile);
Expand Down
5 changes: 1 addition & 4 deletions modules/runtime-fields-common/build.gradle
Expand Up @@ -20,7 +20,4 @@ dependencies {
compileOnly project(':modules:lang-painless:spi')
api project(':libs:elasticsearch-grok')
api project(':libs:elasticsearch-dissect')
}

//this plugin is here only for the painless extension, there are no unit tests
tasks.named("testingConventions").configure { enabled = false }
}
1 change: 0 additions & 1 deletion rest-api-spec/build.gradle
Expand Up @@ -40,7 +40,6 @@ testClusters.configureEach {
}

tasks.named("test").configure { enabled = false }
tasks.named("jarHell").configure { enabled = false }

tasks.named("yamlRestTestV7CompatTransform").configure { task ->

Expand Down
3 changes: 0 additions & 3 deletions test/x-content/build.gradle
Expand Up @@ -32,9 +32,6 @@ tasks.named("dependencyLicenses").configure { enabled = false }
tasks.named("dependenciesInfo").configure { enabled = false }
tasks.named("dependenciesGraph").configure { enabled = false }

// no tests of the tests, yet
tasks.named("testingConventions").configure { enabled = false }

tasks.named("thirdPartyAudit").configure {
ignoreMissingClasses(
// classes are missing
Expand Down
@@ -1,3 +1,10 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.eql;

import org.elasticsearch.common.settings.Settings;
Expand Down
Expand Up @@ -94,7 +94,7 @@ public void testSearchableSnapshotAction() throws Exception {

createComposableTemplate(
client(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(Locale.ROOT),
dataStream,
new Template(Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, policy).build(), null, null)
);
Expand Down Expand Up @@ -125,7 +125,12 @@ public void testSearchableSnapshotForceMergesIndexToOneSegment() throws Exceptio
createSnapshotRepo(client(), snapshotRepo, randomBoolean());
createNewSingletonPolicy(client(), policy, "cold", new SearchableSnapshotAction(snapshotRepo, true));

createComposableTemplate(client(), randomAlphaOfLengthBetween(5, 10).toLowerCase(), dataStream, new Template(null, null, null));
createComposableTemplate(
client(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(Locale.ROOT),
dataStream,
new Template(null, null, null)
);

for (int i = 0; i < randomIntBetween(5, 10); i++) {
indexDocument(client(), dataStream, true);
Expand Down Expand Up @@ -199,7 +204,7 @@ public void testDeleteActionDeletesSearchableSnapshot() throws Exception {

createComposableTemplate(
client(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(Locale.ROOT),
dataStream,
new Template(Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, policy).build(), null, null)
);
Expand Down Expand Up @@ -289,7 +294,7 @@ public void testUpdatePolicyToAddPhasesYieldsInvalidActionsToBeSkipped() throws

createComposableTemplate(
client(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(Locale.ROOT),
dataStream,
new Template(
Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 5).put(LifecycleSettings.LIFECYCLE_NAME, policy).build(),
Expand Down Expand Up @@ -368,7 +373,7 @@ public void testRestoredIndexManagedByLocalPolicySkipsIllegalActions() throws Ex

createComposableTemplate(
client(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(Locale.ROOT),
dataStream,
new Template(
Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 5).put(LifecycleSettings.LIFECYCLE_NAME, policy).build(),
Expand Down Expand Up @@ -628,7 +633,7 @@ public void testSearchableSnapshotsInHotPhasePinnedToHotNodes() throws Exception

createComposableTemplate(
client(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(Locale.ROOT),
dataStream,
new Template(
Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1).put(LifecycleSettings.LIFECYCLE_NAME, policy).build(),
Expand Down Expand Up @@ -675,7 +680,7 @@ public void testSearchableSnapshotInvokesAsyncActionOnNewIndex() throws Exceptio

createComposableTemplate(
client(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(),
randomAlphaOfLengthBetween(5, 10).toLowerCase(Locale.ROOT),
dataStream,
new Template(Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, policy).build(), null, null)
);
Expand Down

0 comments on commit b174af6

Please sign in to comment.