-
Notifications
You must be signed in to change notification settings - Fork 327
Cover muzzle plugin by tests #10643
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
Cover muzzle plugin by tests #10643
Changes from all commits
3c05975
beaf4a0
dce345c
2695763
dbd7215
4f9bd6c
79620c3
128e4b2
63f4d30
482d91c
bcc0531
34056db
0d99dcc
d1f50d3
a5f3454
67cc62d
2d35a7a
e55b3be
d019e9e
f3036a6
f68955d
4e9a595
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,6 +78,7 @@ dependencies { | |
|
|
||
| implementation("org.eclipse.aether", "aether-connector-basic", "1.1.0") | ||
| implementation("org.eclipse.aether", "aether-transport-http", "1.1.0") | ||
| implementation("org.eclipse.aether", "aether-transport-file", "1.1.0") | ||
| implementation("org.apache.maven", "maven-aether-provider", "3.3.9") | ||
|
|
||
| implementation("com.github.zafarkhaja:java-semver:0.10.2") | ||
|
|
@@ -103,9 +104,12 @@ testing { | |
| @Suppress("UnstableApiUsage") | ||
| suites { | ||
| val test by getting(JvmTestSuite::class) { | ||
| dependencies { | ||
| implementation(libs.assertj.core) | ||
| } | ||
| targets.configureEach { | ||
| testTask.configure { | ||
| enabled = project.hasProperty("runBuildSrcTests") | ||
| enabled = providers.systemProperty("runBuildSrcTests").isPresent or providers.systemProperty("idea.active").isPresent | ||
|
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. note: Allow running tests from IntelliJ in |
||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import org.eclipse.aether.resolution.VersionRangeRequest | |
| import org.eclipse.aether.resolution.VersionRangeResult | ||
| import org.eclipse.aether.spi.connector.RepositoryConnectorFactory | ||
| import org.eclipse.aether.spi.connector.transport.TransporterFactory | ||
| import org.eclipse.aether.transport.file.FileTransporterFactory | ||
| import org.eclipse.aether.transport.http.HttpTransporterFactory | ||
| import org.eclipse.aether.version.Version | ||
| import org.gradle.api.GradleException | ||
|
|
@@ -34,13 +35,15 @@ internal object MuzzleMavenRepoUtils { | |
| } | ||
|
|
||
| /** | ||
| * Create new RepositorySystem for muzzle's Maven/Aether resoltions. | ||
| * Create new RepositorySystem for muzzle's Maven/Aether resolutions. | ||
| * Supports both HTTP/HTTPS and file:// repositories. | ||
| */ | ||
| @JvmStatic | ||
| fun newRepositorySystem(): RepositorySystem { | ||
| val locator = MavenRepositorySystemUtils.newServiceLocator().apply { | ||
| addService(RepositoryConnectorFactory::class.java, BasicRepositoryConnectorFactory::class.java) | ||
| addService(TransporterFactory::class.java, HttpTransporterFactory::class.java) | ||
| addService(TransporterFactory::class.java, FileTransporterFactory::class.java) | ||
|
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. note: Necessary to support a fake maven repository (file based) |
||
| } | ||
| return locator.getService(RepositorySystem::class.java) | ||
| } | ||
|
|
@@ -66,15 +69,16 @@ internal object MuzzleMavenRepoUtils { | |
| fun inverseOf( | ||
| muzzleDirective: MuzzleDirective, | ||
| system: RepositorySystem, | ||
| session: RepositorySystemSession | ||
| session: RepositorySystemSession, | ||
| defaultRepos: List<RemoteRepository> = MUZZLE_REPOS | ||
| ): Set<MuzzleDirective> { | ||
| val allVersionsArtifact = DefaultArtifact( | ||
| muzzleDirective.group, | ||
| muzzleDirective.module, | ||
| "jar", | ||
| "[,)" | ||
| ) | ||
| val repos = muzzleDirective.getRepositories(MUZZLE_REPOS) | ||
| val repos = muzzleDirective.getRepositories(defaultRepos) | ||
| val allRangeRequest = VersionRangeRequest().apply { | ||
| repositories = repos | ||
| artifact = allVersionsArtifact | ||
|
|
@@ -119,7 +123,8 @@ internal object MuzzleMavenRepoUtils { | |
| fun resolveVersionRange( | ||
| muzzleDirective: MuzzleDirective, | ||
| system: RepositorySystem, | ||
| session: RepositorySystemSession | ||
| session: RepositorySystemSession, | ||
| defaultRepos: List<RemoteRepository> = MUZZLE_REPOS | ||
| ): VersionRangeResult { | ||
| val directiveArtifact: Artifact = DefaultArtifact( | ||
| muzzleDirective.group, | ||
|
|
@@ -129,7 +134,7 @@ internal object MuzzleMavenRepoUtils { | |
| muzzleDirective.versions | ||
| ) | ||
| val rangeRequest = VersionRangeRequest().apply { | ||
| repositories = muzzleDirective.getRepositories(MUZZLE_REPOS) | ||
| repositories = muzzleDirective.getRepositories(defaultRepos) | ||
| artifact = directiveArtifact | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,11 @@ | ||
| package datadog.gradle.plugin.muzzle | ||
|
|
||
| import datadog.gradle.plugin.muzzle.MuzzleMavenRepoUtils.inverseOf | ||
| import datadog.gradle.plugin.muzzle.MuzzleMavenRepoUtils.muzzleDirectiveToArtifacts | ||
| import datadog.gradle.plugin.muzzle.MuzzleMavenRepoUtils.resolveVersionRange | ||
| import datadog.gradle.plugin.muzzle.tasks.MuzzleEndTask | ||
| import datadog.gradle.plugin.muzzle.tasks.MuzzleGenerateReportTask | ||
| import datadog.gradle.plugin.muzzle.tasks.MuzzleGetReferencesTask | ||
| import datadog.gradle.plugin.muzzle.tasks.MuzzleMergeReportsTask | ||
| import datadog.gradle.plugin.muzzle.tasks.MuzzleTask | ||
| import datadog.gradle.plugin.muzzle.planner.MuzzleTaskPlanner | ||
| import org.eclipse.aether.artifact.Artifact | ||
| import org.gradle.api.NamedDomainObjectProvider | ||
| import org.gradle.api.Plugin | ||
|
|
@@ -101,14 +99,15 @@ class MuzzlePlugin : Plugin<Project> { | |
| project.tasks.register<MuzzleMergeReportsTask>("mergeMuzzleReports") | ||
|
|
||
| val hasRelevantTask = project.gradle.startParameter.taskNames.any { taskName -> | ||
| // removing leading ':' if present | ||
| val muzzleTaskName = taskName.removePrefix(":") | ||
| val projectPath = project.path.removePrefix(":") | ||
| muzzleTaskName == "muzzle" || "$projectPath:muzzle" == muzzleTaskName || | ||
| muzzleTaskName == "runMuzzle" | ||
| val taskProjectPath = taskName.substringBeforeLast(":", "") | ||
| val taskNameOnly = taskName.substringAfterLast(":") | ||
| val isRelevantForProject = taskProjectPath.isEmpty() || taskProjectPath == project.path | ||
|
|
||
| isRelevantForProject && taskNameOnly.endsWith("muzzle", ignoreCase = true) | ||
|
Comment on lines
-104
to
+106
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. note: Small change for better readability and avoiding useless and confusing prefix removal. Change performed after unit tests. |
||
| } | ||
| if (!hasRelevantTask) { | ||
| // Adding muzzle dependencies has a large config overhead. Stop unless muzzle is explicitly run. | ||
| project.logger.info("No muzzle tasks invoked for ${project.path}, skipping muzzle task planification") | ||
| return | ||
| } | ||
|
|
||
|
|
@@ -117,40 +116,19 @@ class MuzzlePlugin : Plugin<Project> { | |
|
|
||
| val system = MuzzleMavenRepoUtils.newRepositorySystem() | ||
| val session = MuzzleMavenRepoUtils.newRepositorySystemSession(system) | ||
| val taskPlanner = MuzzleTaskPlanner.from(system, session) | ||
| project.afterEvaluate { | ||
| // use runAfter to set up task finalizers in version order | ||
| var runAfter: TaskProvider<MuzzleTask> = muzzleTask | ||
| val muzzleReportTasks = mutableListOf<TaskProvider<MuzzleTask>>() | ||
|
|
||
| project.extensions.getByType<MuzzleExtension>().directives.forEach { directive -> | ||
| project.logger.debug("configuring {}", directive) | ||
|
|
||
| if (directive.isCoreJdk) { | ||
| runAfter = addMuzzleTask(directive, null, project, runAfter, muzzleBootstrap, muzzleTooling) | ||
| muzzleReportTasks.add(runAfter) | ||
| } else { | ||
| val range = resolveVersionRange(directive, system, session) | ||
|
|
||
| muzzleDirectiveToArtifacts(directive, range).forEach { | ||
| runAfter = addMuzzleTask(directive, it, project, runAfter, muzzleBootstrap, muzzleTooling) | ||
| muzzleReportTasks.add(runAfter) | ||
| } | ||
|
|
||
| if (directive.assertInverse) { | ||
| inverseOf(directive, system, session).forEach { inverseDirective -> | ||
| val inverseRange = resolveVersionRange(inverseDirective, system, session) | ||
|
|
||
| muzzleDirectiveToArtifacts(inverseDirective, inverseRange).forEach { | ||
| runAfter = addMuzzleTask(inverseDirective, it, project, runAfter, muzzleBootstrap, muzzleTooling) | ||
| muzzleReportTasks.add(runAfter) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| project.logger.info("configured $directive") | ||
| val directives = project.extensions.getByType<MuzzleExtension>().directives | ||
| taskPlanner.plan(directives).forEach { plan -> | ||
| runAfter = registerMuzzleTask(plan.directive, plan.artifact, project, runAfter, muzzleBootstrap, muzzleTooling) | ||
| muzzleReportTasks.add(runAfter) | ||
| project.logger.info("configured ${plan.directive}") | ||
|
Comment on lines
-125
to
+128
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. note: This is extracted in the new |
||
| } | ||
|
|
||
| if (muzzleReportTasks.isEmpty() && !project.extensions.getByType<MuzzleExtension>().directives.any { it.assertPass }) { | ||
| if (muzzleReportTasks.isEmpty() && !directives.any { it.assertPass }) { | ||
| muzzleReportTasks.add(muzzleTask) | ||
| } | ||
|
|
||
|
|
@@ -180,7 +158,7 @@ class MuzzlePlugin : Plugin<Project> { | |
| * @param muzzleTooling The configuration provider for agent tooling dependencies. | ||
| * @return The muzzle task provider. | ||
| */ | ||
| private fun addMuzzleTask( | ||
| private fun registerMuzzleTask( | ||
| muzzleDirective: MuzzleDirective, | ||
| versionArtifact: Artifact?, | ||
| instrumentationProject: Project, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package datadog.gradle.plugin.muzzle.planner | ||
|
|
||
| import datadog.gradle.plugin.muzzle.MuzzleDirective | ||
| import datadog.gradle.plugin.muzzle.MuzzleMavenRepoUtils | ||
| import org.eclipse.aether.RepositorySystem | ||
| import org.eclipse.aether.RepositorySystemSession | ||
| import org.eclipse.aether.artifact.Artifact | ||
|
|
||
| /** | ||
| * Default [MuzzleResolutionService] implementation backed by Maven/Aether resolution. | ||
| */ | ||
| internal class MavenMuzzleResolutionService( | ||
| private val system: RepositorySystem, | ||
| private val session: RepositorySystemSession, | ||
| ) : MuzzleResolutionService { | ||
| override fun resolveArtifacts(directive: MuzzleDirective): Set<Artifact> { | ||
| val range = MuzzleMavenRepoUtils.resolveVersionRange(directive, system, session) | ||
| return MuzzleMavenRepoUtils.muzzleDirectiveToArtifacts(directive, range) | ||
| } | ||
|
|
||
| override fun inverseOf(directive: MuzzleDirective): Set<MuzzleDirective> = | ||
| MuzzleMavenRepoUtils.inverseOf(directive, system, session) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package datadog.gradle.plugin.muzzle.planner | ||
|
|
||
| import datadog.gradle.plugin.muzzle.MuzzleDirective | ||
| import org.eclipse.aether.artifact.Artifact | ||
|
|
||
| /** | ||
| * Resolves muzzle directives into concrete artifacts and inverse directives. | ||
| */ | ||
| internal interface MuzzleResolutionService { | ||
| /** | ||
| * Resolves all dependency artifacts to test for the given directive. | ||
| */ | ||
| fun resolveArtifacts(directive: MuzzleDirective): Set<Artifact> | ||
|
|
||
| /** | ||
| * Computes directives representing the inverse of the given directive. | ||
| */ | ||
| fun inverseOf(directive: MuzzleDirective): Set<MuzzleDirective> | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package datadog.gradle.plugin.muzzle.planner | ||
|
|
||
| import datadog.gradle.plugin.muzzle.MuzzleDirective | ||
| import org.eclipse.aether.artifact.Artifact | ||
|
|
||
| /** | ||
| * Planned unit of muzzle work for task creation. | ||
| * | ||
| * For `coreJdk()` directives, [artifact] is `null`. | ||
| */ | ||
| internal data class MuzzleTaskPlan( | ||
| val directive: MuzzleDirective, | ||
| val artifact: Artifact?, | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| package datadog.gradle.plugin.muzzle.planner | ||
|
|
||
| import datadog.gradle.plugin.muzzle.MuzzleDirective | ||
| import org.eclipse.aether.RepositorySystem | ||
| import org.eclipse.aether.RepositorySystemSession | ||
|
|
||
| /** | ||
| * Expands configured directives into ordered task plans. | ||
| */ | ||
| internal class MuzzleTaskPlanner( | ||
| private val resolutionService: MuzzleResolutionService, | ||
| ) { | ||
| companion object { | ||
| fun from(system: RepositorySystem, session: RepositorySystemSession): MuzzleTaskPlanner = | ||
| MuzzleTaskPlanner(MavenMuzzleResolutionService(system, session)) | ||
| } | ||
|
|
||
| /** | ||
| * Expands declared muzzle directives into executable task plans. | ||
| * | ||
| * Planning rules: | ||
| * - Core-JDK directives (`coreJdk()`) create exactly one [MuzzleTaskPlan] with `artifact = null`. | ||
| * - Non-core directives are resolved with [MuzzleResolutionService.resolveArtifacts], creating one | ||
| * plan per resolved artifact. | ||
| * - If a non-core directive has `assertInverse = true`, inverse directives are obtained from | ||
| * [MuzzleResolutionService.inverseOf], then each inverse directive is resolved and expanded with | ||
| * the same one-plan-per-artifact rule. | ||
| * | ||
| * Ordering: | ||
| * - The input [directives] order is preserved. | ||
| * - Direct plans for a directive are emitted before its inverse plans. | ||
| * - Artifact plan order follows the iteration order returned by the resolution service. | ||
| * | ||
| * No de-duplication is performed here. If needed, de-duplication must be handled by callers or by | ||
| * the resolution service implementation. | ||
| */ | ||
| fun plan(directives: List<MuzzleDirective>): List<MuzzleTaskPlan> = buildList { | ||
| directives.forEach { directive -> | ||
| if (directive.isCoreJdk) { | ||
| add(MuzzleTaskPlan(directive, null)) | ||
| } else { | ||
| resolutionService.resolveArtifacts(directive).forEach { artifact -> | ||
| add(MuzzleTaskPlan(directive, artifact)) | ||
| } | ||
| if (directive.assertInverse) { | ||
| resolutionService.inverseOf(directive).forEach { inverseDirective -> | ||
| resolutionService.resolveArtifacts(inverseDirective).forEach { artifact -> | ||
| add(MuzzleTaskPlan(inverseDirective, artifact)) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,13 +74,17 @@ abstract class MuzzleTask @Inject constructor( | |
| @TaskAction | ||
| fun muzzle() { | ||
| when { | ||
| // Version-specific task: created by MuzzlePlugin for each resolved artifact. | ||
| muzzleDirective.isPresent -> { | ||
| assertMuzzle(muzzleDirective.get()) | ||
| } | ||
| // Fallback for the root "muzzle" lifecycle task when no pass{} directives are | ||
| // declared. In that case there are no version-specific pass tasks, so we assert | ||
| // the instrumentation against its own compile-time classpath as a basic sanity check. | ||
| !project.extensions.getByType<MuzzleExtension>().directives.any { it.assertPass } -> { | ||
| project.logger.info("No muzzle pass directives configured. Asserting pass against instrumentation compile-time dependencies") | ||
| assertMuzzle() | ||
| } | ||
| muzzleDirective.isPresent -> { | ||
| assertMuzzle(muzzleDirective.get()) | ||
| } | ||
|
Comment on lines
+77
to
-83
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. note: This was actually a bug, detected by a unit test. If there were no pass directive at all, the wrong branch was used. |
||
| } | ||
| } | ||
|
|
||
|
|
||
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.
note:
check_build_srcdo|should not need to wait on thebuildjob.