Skip to content
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

Address Issue #258: Configuration predicate to control dependency graph creation #259

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ affectedModuleDetector {
"Run static analysis tool without auto-correction by Impact analysis"
)
]
configurationPredicate.set(new Predicate<Configuration>() {
boolean test(Configuration configuration) {
return !configuration.name.contains("somethingToExclude")
}
})
}
```

Expand All @@ -124,6 +129,7 @@ affectedModuleDetector {
- `includeUncommitted`: If uncommitted files should be considered affected
- `top`: The top of the git log to use. Must be used in combination with configuration `includeUncommitted = false`
- `customTasks`: set of [CustomTask](https://github.com/dropbox/AffectedModuleDetector/blob/main/affectedmoduledetector/src/main/kotlin/com/dropbox/affectedmoduledetector/AffectedModuleConfiguration.kt)
- `configurationPredicate`: A predicate to filter configurations that should be considered for the dependency graph. By default, all configurations are considered.

By default, the Detector will look for `assembleAndroidDebugTest`, `connectedAndroidDebugTest`, and `testDebug`. Modules can specify a configuration block to specify which variant tests to run:
```groovy
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package com.dropbox.affectedmoduledetector

import com.dropbox.affectedmoduledetector.util.toOsSpecificPath
import org.gradle.api.artifacts.Configuration
import org.gradle.api.model.ObjectFactory
import org.gradle.api.provider.Property
import java.io.File
import java.util.function.Predicate

class AffectedModuleConfiguration {
class AffectedModuleConfiguration(objectFactory: ObjectFactory) {

/**
* Implementation of [AffectedModuleTaskType] for easy adding of custom gradle task to
Expand Down Expand Up @@ -44,6 +48,16 @@ class AffectedModuleConfiguration {
*/
var customTasks = emptySet<CustomTask>()

/**
* Predicate to determine if a configuration should be considered or ignored. This predicate
* will be called for every configuration defined by each project module. By default,
* all configurations are considered.
*/
@Suppress("UNCHECKED_CAST") // Erasure in the API results in: Property<Predicate<*>>
val configurationPredicate: Property<Predicate<Configuration>> =
(objectFactory.property(Predicate::class.java) as Property<Predicate<Configuration>>)
.convention(AlwaysConfigurationPredicate())

/**
* Folder to place the log in
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class AffectedModuleDetectorPlugin : Plugin<Project> {
private fun registerMainConfiguration(project: Project) {
project.extensions.add(
AffectedModuleConfiguration.name,
AffectedModuleConfiguration()
AffectedModuleConfiguration(project.objects)
)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.dropbox.affectedmoduledetector

import org.gradle.api.artifacts.Configuration
import java.util.function.Predicate

/**
* Default implementation of a [Configuration] [Predicate] that always returns true, indicating
* that all configurations should be considered.
*/
internal class AlwaysConfigurationPredicate : Predicate<Configuration> {
override fun test(t: Configuration): Boolean {
return true
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,30 @@ class DependencyTracker constructor(
private val rootProject: Project,
private val logger: Logger?
) {
private val configuration: AffectedModuleConfiguration by lazy {
rootProject.extensions.getByType(AffectedModuleConfiguration::class.java)
}

private val dependentList: Map<Project, Set<Project>> by lazy {
val result = mutableMapOf<Project, MutableSet<Project>>()
rootProject.subprojects.forEach { project ->
logger?.info("checking ${project.path} for dependencies")
project.configurations.forEach { config ->
logger?.info("checking config ${project.path}/$config for dependencies")
config
.dependencies
.filterIsInstance(ProjectDependency::class.java)
.forEach {
logger?.info(
"there is a dependency from ${project.path} to " +
it.dependencyProject.path
)
result.getOrPut(it.dependencyProject) { mutableSetOf() }
.add(project)
}
}
project.configurations
.filter(configuration.configurationPredicate.get()::test)
.forEach { config ->
logger?.info("checking config ${project.path}/$config for dependencies")
config
.dependencies
.filterIsInstance(ProjectDependency::class.java)
.forEach {
logger?.info(
"there is a dependency from ${project.path} to " +
it.dependencyProject.path
)
result.getOrPut(it.dependencyProject) { mutableSetOf() }
.add(project)
}
}
}
result
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.dropbox.affectedmoduledetector

import com.dropbox.affectedmoduledetector.mocks.MockObjectFactory
import com.google.common.truth.Truth.assertThat
import org.gradle.api.artifacts.Configuration
import org.junit.Assert.fail
import org.junit.Before
import org.junit.Rule
Expand All @@ -9,6 +11,7 @@ import org.junit.rules.TemporaryFolder
import org.junit.runner.RunWith
import org.junit.runners.JUnit4
import java.io.File
import java.util.function.Predicate

@RunWith(JUnit4::class)
class AffectedModuleConfigurationTest {
Expand All @@ -26,7 +29,7 @@ class AffectedModuleConfigurationTest {

@Before
fun setup() {
config = AffectedModuleConfiguration()
config = AffectedModuleConfiguration(MockObjectFactory())
}

@Test
Expand Down Expand Up @@ -324,4 +327,28 @@ class AffectedModuleConfigurationTest {

assert(actual.first().taskDescription == "Description of fake task")
}

@Test
fun `GIVEN AffectedModuleConfiguration WHEN configuration predicate is set THEN is configuration predicate`() {
// GIVEN
val expected = Predicate<Configuration> { false }
config.configurationPredicate.set(expected)

// WHEN
val predicate = config.configurationPredicate.get()

// THEN
assertThat(predicate).isSameInstanceAs(expected)
}

@Test
fun `GIVEN AffectedModuleConfiguration WHEN configuration predicate is not set THEN is default`() {
// GIVEN default configuration

// WHEN
val predicate = config.configurationPredicate.get()

// THEN
assertThat(predicate).isInstanceOf(AlwaysConfigurationPredicate::class.java)
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.dropbox.affectedmoduledetector

import com.dropbox.affectedmoduledetector.mocks.MockObjectFactory
import com.google.common.truth.Truth
import org.gradle.api.Project
import org.gradle.api.plugins.ExtraPropertiesExtension
Expand Down Expand Up @@ -220,10 +221,13 @@ class AffectedModuleDetectorImplTest {
val p19config = p19.configurations.create("p19config")
p19config.dependencies.add(p19.dependencies.project(mutableMapOf("path" to ":p18")))

affectedModuleConfiguration = AffectedModuleConfiguration().also {
affectedModuleConfiguration = AffectedModuleConfiguration(MockObjectFactory()).also {
it.baseDir = tmpDir.absolutePath
it.pathsAffectingAllModules = pathsAffectingAllModules
}
listOf(root, root2, root3).forEach { rootProject ->
rootProject.extensions.add(AffectedModuleConfiguration.name, affectedModuleConfiguration)
}
}

@Test
Expand Down Expand Up @@ -1329,7 +1333,7 @@ class AffectedModuleDetectorImplTest {
@Test
fun `GIVEN affected module configuration WHEN invalid path THEN throw exception`() {
// GIVEN
val config = AffectedModuleConfiguration().also {
val config = AffectedModuleConfiguration(MockObjectFactory()).also {
it.baseDir = tmpFolder.root.absolutePath
}

Expand All @@ -1350,7 +1354,7 @@ class AffectedModuleDetectorImplTest {
@Test
fun `GIVEN affected module configuration WHEN valid paths THEN return paths`() {
// GIVEN
val config = AffectedModuleConfiguration().also {
val config = AffectedModuleConfiguration(MockObjectFactory()).also {
it.baseDir = tmpFolder.root.absolutePath
}

Expand Down Expand Up @@ -1522,6 +1526,55 @@ class AffectedModuleDetectorImplTest {
)
}

@Test
fun `GIVEN upward configuration reference from p2 to p6 WHEN no predicate is supplied THEN p2 is affected`() {
p2.configurations.create("p2-upward-p6") { config ->
config.dependencies.add(p2.dependencies.project(mapOf("path" to p6.path)))
}
val detector = AffectedModuleDetectorImpl(
rootProject = root,
logger = logger,
ignoreUnknownProjects = false,
projectSubset = ProjectSubset.ALL_AFFECTED_PROJECTS,
modules = null,
injectedGitClient = MockGitClient(
changedFiles = listOf(
convertToFilePath("d1/d3/d6", "foo.java")
),
tmpFolder = tmpFolder.root
),
config = affectedModuleConfiguration
)
Truth.assertThat(detector.shouldInclude(p2)).isTrue()
Truth.assertThat(detector.shouldInclude(p6)).isTrue()
}

@Test
fun `GIVEN upward configuration reference from p2 to p6 WHEN predicate filtered THEN p2 is unaffected`() {
p2.configurations.create("p2-upward-p6") { config ->
config.dependencies.add(p2.dependencies.project(mapOf("path" to p6.path)))
}
affectedModuleConfiguration.configurationPredicate.set { configuration ->
!configuration.name.contains("-upward-")
}
val detector = AffectedModuleDetectorImpl(
rootProject = root,
logger = logger,
ignoreUnknownProjects = false,
projectSubset = ProjectSubset.ALL_AFFECTED_PROJECTS,
modules = null,
injectedGitClient = MockGitClient(
changedFiles = listOf(
convertToFilePath("d1/d3/d6", "foo.java")
),
tmpFolder = tmpFolder.root
),
config = affectedModuleConfiguration
)
Truth.assertThat(detector.shouldInclude(p2)).isFalse()
Truth.assertThat(detector.shouldInclude(p6)).isTrue()
}

// For both Linux/Windows
fun convertToFilePath(vararg list: String): String {
return list.toList().joinToString(File.separator)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.dropbox.affectedmoduledetector

import com.dropbox.affectedmoduledetector.mocks.MockObjectFactory
import com.google.common.truth.Truth.assertThat
import org.gradle.api.Project
import org.gradle.api.internal.plugins.PluginApplicationException
Expand All @@ -10,7 +11,6 @@ import org.junit.Test
import org.junit.rules.TemporaryFolder
import org.junit.runner.RunWith
import org.junit.runners.JUnit4
import java.lang.IllegalStateException

@RunWith(JUnit4::class)
class AffectedModuleDetectorPluginTest {
Expand Down Expand Up @@ -102,7 +102,7 @@ class AffectedModuleDetectorPluginTest {
@Test
fun `GIVEN affected module detector plugin WHEN register_custom_task is called AND AffectedModuleConfiguration customTask is not empty THEN task is added`() {
// GIVEN
val configuration = AffectedModuleConfiguration()
val configuration = AffectedModuleConfiguration(MockObjectFactory())
configuration.customTasks = setOf(fakeTask)
rootProject.extensions.add(AffectedModuleConfiguration.name, configuration)

Expand All @@ -122,7 +122,7 @@ class AffectedModuleDetectorPluginTest {
@Test
fun `GIVEN affected module detector plugin WHEN registerCustomTasks is called AND AffectedModuleConfiguration customTask is empty THEN task isn't added`() {
// GIVEN
val configuration = AffectedModuleConfiguration()
val configuration = AffectedModuleConfiguration(MockObjectFactory())
rootProject.extensions.add(AffectedModuleConfiguration.name, configuration)
val plugin = AffectedModuleDetectorPlugin()

Expand All @@ -144,7 +144,7 @@ class AffectedModuleDetectorPluginTest {
@Test
fun `GIVEN affected module detector plugin WHEN registerTestTasks THEN task all task added`() {
// GIVEN
val configuration = AffectedModuleConfiguration()
val configuration = AffectedModuleConfiguration(MockObjectFactory())
rootProject.extensions.add(AffectedModuleConfiguration.name, configuration)
val plugin = AffectedModuleDetectorPlugin()

Expand All @@ -168,7 +168,7 @@ class AffectedModuleDetectorPluginTest {
@Test
fun `GIVEN affected module detector plugin WHEN registerTestTasks called THEN added all tasks from InternalTaskType`() {
// GIVEN
val configuration = AffectedModuleConfiguration()
val configuration = AffectedModuleConfiguration(MockObjectFactory())
rootProject.extensions.add(AffectedModuleConfiguration.name, configuration)
val plugin = AffectedModuleDetectorPlugin()
val availableTaskVariants = 3 // runAffectedAndroidTests, assembleAffectedAndroidTests and runAffectedUnitTests
Expand All @@ -187,7 +187,7 @@ class AffectedModuleDetectorPluginTest {
fun `GIVEN affected module detector plugin WHEN registerCustomTasks called THEN added all tasks from FakeTaskType`() {
// GIVEN
val givenCustomTasks = setOf(fakeTask, fakeTask.copy(commandByImpact = "otherCommand"))
val configuration = AffectedModuleConfiguration()
val configuration = AffectedModuleConfiguration(MockObjectFactory())
configuration.customTasks = givenCustomTasks
rootProject.extensions.add(AffectedModuleConfiguration.name, configuration)
val plugin = AffectedModuleDetectorPlugin()
Expand Down
Loading