Skip to content

Commit

Permalink
MBS 9592 Fix NPE when no ANDROID_HOME specified (#597)
Browse files Browse the repository at this point in the history
* MBS-9592 Can disable build checks completely by property

* MBS-9592 Test no ANDROID_SDK

* MBS-9592 Declare return types for System methods

* MBS-9592 Fix tests with default ANDROID_HOME
  • Loading branch information
eugene-krivobokov authored Oct 14, 2020
1 parent 6159166 commit 0bcc82e
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,15 @@ class AndroidSdk(
}

private fun androidHome(projectRootDir: File, logger: Logger): File {
val path = System.getenv("ANDROID_HOME").ifBlank { null }
?: androidHomeFromLocalProperties(
val fromEnv: String? = System.getenv("ANDROID_HOME")
val path = if (fromEnv.isNullOrBlank()) {
androidHomeFromLocalProperties(
localPropertiesLocation = File(projectRootDir, "local.properties"),
logger = { logger.error(it) })
?: error("Can't resolve ANDROID_HOME")
?: error("Can't find ANDROID_HOME")
} else {
fromEnv
}

val dir = File(path)
require(dir.exists()) {
Expand All @@ -78,12 +82,12 @@ class AndroidSdk(
* Required in places where is no access to Project, mostly on Gradle Test Kit project creation
*/
fun androidHomeFromLocalPropertiesFallback(): String {
val env = System.getenv("ANDROID_HOME")
val env: String? = System.getenv("ANDROID_HOME")
if (!env.isNullOrBlank()) {
return env
}

val rootDir = System.getProperty("rootDir")
val rootDir: String? = System.getProperty("rootDir")

return androidHomeFromLocalProperties(File("$rootDir/local.properties"))
?: error("Can't resolve android sdk: env ANDROID_HOME and $rootDir/local.properties is not available")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ interface EnvArgs {
}

companion object {
private val userName = System.getProperty("user.name")
private val userName: String? = System.getProperty("user.name")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ open class BuildParamCheckPlugin : Plugin<Project> {
override fun apply(project: Project) {
val extension = project.extensions.create<BuildChecksExtension>(extensionName)

printBuildEnvironment(project)

check(project.isRoot()) {
"Plugin must be applied to the root project but was applied to ${project.path}"
}
if (!project.pluginIsEnabled) return

printBuildEnvironment(project)

project.afterEvaluate {
val checks = ChecksFilter(extension).checks()
val checks = ChecksFilter(extension).enabledChecks()
checks
.filterIsInstance<BuildChecksExtension.RequireParameters>()
.forEach {
Expand All @@ -57,6 +59,13 @@ open class BuildParamCheckPlugin : Plugin<Project> {
}
}

private val Project.pluginIsEnabled: Boolean
get() = providers
.gradleProperty(enabledProp)
.forUseAtConfigurationTime()
.map { it.toBoolean() }
.getOrElse(true)

private fun checkJavaVersion(check: Check.JavaVersion) {
check(JavaVersion.current() == check.version) {
"Only ${check.version} is supported for this project but was ${javaInfo()}. " +
Expand Down Expand Up @@ -144,7 +153,8 @@ open class BuildParamCheckPlugin : Plugin<Project> {
}

private fun isMac(): Boolean {
return System.getProperty("os.name", "").contains("mac", ignoreCase = true)
return System.getProperty("os.name", "").orEmpty()
.contains("mac", ignoreCase = true)
}

private val validationErrors = mutableListOf<String>()
Expand Down Expand Up @@ -267,3 +277,4 @@ javaIncrementalCompilation=$javaIncrementalCompilation
}

private const val pluginName = "BuildParamCheckPlugin"
private const val enabledProp = "avito.build-checks.enabled"
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ internal class ChecksFilter(
private val extension: BuildChecksExtension
) {

fun checks(): List<Check> {
fun enabledChecks(): List<Check> {
val changedByUser = extension.checks
val enabledByUser = changedByUser.filter { it.enabled }
return if (extension.enableByDefault) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,30 @@ package com.avito.android.plugin.build_param_check

import com.avito.test.gradle.TestProjectGenerator
import com.avito.test.gradle.TestResult
import com.avito.test.gradle.file
import com.avito.test.gradle.gradlew
import java.io.File

class BuildChecksTestProjectRunner(
private val projectDir: File,
private val androidHome: File? = null,
private val androidHome: AndroidHomeLocation = AndroidHomeLocation.Default,
private val buildChecksExtension: String
) {

sealed class AndroidHomeLocation {
object Default : AndroidHomeLocation()
object Absent : AndroidHomeLocation()
class Custom(val dir: File) : AndroidHomeLocation()
}

fun runChecks(expectFailure: Boolean = false): TestResult {
val androidHomePath = if (androidHome is AndroidHomeLocation.Custom) androidHome.dir.path else null

val environment: Map<String, String>? = when (androidHome) {
is AndroidHomeLocation.Default -> null // the build use the system environment
is AndroidHomeLocation.Absent -> mapOf("ANDROID_HOME" to "")
is AndroidHomeLocation.Custom -> mapOf("ANDROID_HOME" to androidHome.dir.path.toString())
}
TestProjectGenerator(
plugins = listOf("com.avito.android.buildchecks"),
modules = emptyList(),
Expand All @@ -20,13 +34,12 @@ class BuildChecksTestProjectRunner(
$buildChecksExtension
}
""".trimIndent(),
androidHome = androidHome?.path
androidHome = androidHomePath
).generateIn(projectDir)

val environment: Map<String, String>? = if(androidHome == null) {
null
} else {
mapOf("ANDROID_HOME" to androidHome.path.toString())
if (androidHome is AndroidHomeLocation.Absent) {
// remove TestProjectGenerator fallback
projectDir.file("local.properties").delete()
}

return gradlew(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.avito.android.plugin.build_param_check

import com.avito.android.plugin.build_param_check.BuildChecksTestProjectRunner.AndroidHomeLocation
import com.avito.android.plugin.build_param_check.BuildChecksTestProjectRunner.AndroidHomeLocation.Custom
import com.avito.test.gradle.TestResult
import com.avito.test.gradle.dir
import com.avito.test.gradle.file
Expand All @@ -11,7 +13,7 @@ import java.nio.file.Path

class CheckAndroidSdkVersionTest {

private lateinit var androidHome: File
private var androidHome: File? = null
private lateinit var projectDir: File

@BeforeEach
Expand All @@ -22,11 +24,28 @@ class CheckAndroidSdkVersionTest {
}

@Test
fun `fail - no android sdk`() {
androidHome.delete()
fun `fail - no android sdk specified`() {
androidHome?.delete()
androidHome = null

val result = runCheck(
"""
androidHomeLocation = AndroidHomeLocation.Absent,
extension = """
compileSdkVersion = 29
revision = 5
""",
expectFailure = true
)
result.assertThat()
.buildFailed("Can't find ANDROID_HOME")
}

@Test
fun `fail - no android sdk in specified path`() {
androidHome?.delete()

val result = runCheck(
extension = """
compileSdkVersion = 29
revision = 5
""",
Expand All @@ -39,7 +58,7 @@ class CheckAndroidSdkVersionTest {
@Test
fun `fail - not specified versions`() {
val result = runCheck(
"""
extension = """
// no versions
""",
expectFailure = true
Expand All @@ -53,7 +72,7 @@ class CheckAndroidSdkVersionTest {
givenAndroidSdkPlatform(version = 28, revision = 1)

val result = runCheck(
"""
extension = """
compileSdkVersion = 29
revision = 5
""",
Expand All @@ -70,7 +89,7 @@ class CheckAndroidSdkVersionTest {
givenAndroidSdkPlatform(version = 29, revision = 4)

val result = runCheck(
"""
extension = """
compileSdkVersion = 29
revision = 5
""",
Expand All @@ -88,7 +107,7 @@ class CheckAndroidSdkVersionTest {
givenAndroidSdkPlatform(version = 29, revision = 5)

val result = runCheck(
"""
extension = """
compileSdkVersion = 29
revision = 5
"""
Expand All @@ -101,7 +120,7 @@ class CheckAndroidSdkVersionTest {
givenAndroidSdkPlatform(version = 29, revision = 6)

val result = runCheck(
"""
extension = """
compileSdkVersion = 29
revision = 5
"""
Expand All @@ -113,9 +132,13 @@ class CheckAndroidSdkVersionTest {
)
}

private fun runCheck(extension: String, expectFailure: Boolean = false): TestResult {
private fun runCheck(
androidHomeLocation: AndroidHomeLocation = Custom(requireNotNull(androidHome)),
extension: String,
expectFailure: Boolean = false
): TestResult {
return BuildChecksTestProjectRunner(
projectDir, androidHome,
projectDir, androidHomeLocation,
buildChecksExtension = """
enableByDefault = false
androidSdk {
Expand All @@ -127,7 +150,7 @@ class CheckAndroidSdkVersionTest {
}

private fun givenAndroidSdkPlatform(version: Int, revision: Int) {
androidHome
requireNotNull(androidHome)
.dir("platforms")
.dir("android-$version")
.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ class ChecksFilterTest {
val extension = BuildChecksExtension().apply {
enableByDefault = false
}
val checks = ChecksFilter(extension).checks()
val checks = ChecksFilter(extension).enabledChecks()

assertThat(checks).isEmpty()
}

@Test
fun `all default checks are enabled - default config`() {
val extension = BuildChecksExtension()
val checks = ChecksFilter(extension).checks()
val checks = ChecksFilter(extension).enabledChecks()

assertHasInstance<Check.MacOSLocalhost>(checks)
assertHasInstance<Check.DynamicDependencies>(checks)
Expand All @@ -40,7 +40,7 @@ class ChecksFilterTest {
enableByDefault = false
androidSdk(Action { })
}
val checks = ChecksFilter(extension).checks()
val checks = ChecksFilter(extension).enabledChecks()

assertThat(checks).hasSize(1)
assertHasInstance<Check.AndroidSdk>(checks)
Expand All @@ -53,7 +53,7 @@ class ChecksFilterTest {
it.enabled = false
})
}
val checks = ChecksFilter(extension).checks()
val checks = ChecksFilter(extension).enabledChecks()

assertNoInstance<Check.AndroidSdk>(checks)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const val androidModuleTestDependency = "androidModuleDependency"
const val kotlinModuleTestDependency = "kotlinModuleDependency"
const val markerClass = "kotlinModule.marker.Screen"
const val markerField = "rootId"
private val artifactoryUrl = System.getProperty("artifactoryUrl")
private val artifactoryUrl: String? = System.getProperty("artifactoryUrl")

private fun generateBaseStubProject(dir: File, output: File): File {
TestProjectGenerator(
Expand Down

0 comments on commit 0bcc82e

Please sign in to comment.