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

Further cleanup of Configuration model #67

Merged
merged 1 commit into from
Jul 31, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,15 @@ package com.malinskiy.marathon.cache.test.key

import com.malinskiy.marathon.cache.CacheKey
import com.malinskiy.marathon.device.DevicePoolId
import com.malinskiy.marathon.execution.Configuration
import com.malinskiy.marathon.test.Test
import java.io.OutputStream
import java.io.Writer
import java.math.BigInteger
import java.security.DigestOutputStream
import java.security.MessageDigest

class TestCacheKeyFactory(
private val componentCacheKeyProvider: ComponentCacheKeyProvider,
private val versionNameProvider: VersionNameProvider,
private val configuration: Configuration
private val versionNameProvider: VersionNameProvider
) {

suspend fun getCacheKey(poolId: DevicePoolId, test: Test): CacheKey {
Expand All @@ -25,7 +22,6 @@ class TestCacheKeyFactory(
.use {
it.write(versionNameProvider.versionName)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can remove versionNameProvider.versionName as well? The version change shouldn't trigger test cache wipeout by itself. And if the apk will change then componentCachingKey will change as well.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea might've been for the cache key to depend on the version of Marathon itself, however versionName seems to always be unknown because there is no Implementation-Version: in the JAR manifest.

it.write(CACHE_FORMAT_VERSION)
it.writeConfiguration(configuration)
it.write(componentCachingKey)
it.write(poolId.name)
it.write(test.pkg)
Expand All @@ -42,13 +38,6 @@ class TestCacheKeyFactory(
return TestCacheKey(key, test)
}

/**
* Write parameters that may affect test execution result or collected artifacts
*/
private fun Writer.writeConfiguration(configuration: Configuration) {
write("codeCoverageEnabled=${configuration.isCodeCoverageEnabled}")
}

private fun createDigestOutputStream(): DigestOutputStream =
DigestOutputStream(
object : OutputStream() {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/kotlin/com/malinskiy/marathon/di/Modules.kt
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ val analyticsModule = module {

val cacheModule = module {
single<CacheService> { CacheServiceFactory(get()).createCacheService() }
single<TestCacheKeyFactory> { TestCacheKeyFactory(get(), get(), get()) }
single<TestCacheKeyFactory> { TestCacheKeyFactory(get(), get()) }
single<TestResultsCache> { TestResultsCache(get(), get(), get()) }
single<TestCacheLoader> { TestCacheLoader(get(), get(), get()) }
single<CacheTestReporter> { CacheTestReporter(get(), get()) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import com.malinskiy.marathon.execution.strategy.impl.sorting.NoSortingStrategy
import com.malinskiy.marathon.vendor.VendorConfiguration
import java.io.File

private const val DEFAULT_EXECUTION_TIMEOUT_MILLIS: Long = 900_000
private const val DEFAULT_NO_DEVICES_TIMEOUT_MILLIS: Long = 300_000
private const val DEFAULT_OUTPUT_TIMEOUT_MILLIS: Long = 60_000

Expand All @@ -36,8 +35,6 @@ data class Configuration constructor(

val cache: CacheConfiguration,
val ignoreFailures: Boolean,
val isCodeCoverageEnabled: Boolean,
val fallbackToScreenshots: Boolean,
val strictMode: Boolean,
val uncompletedTestRetryQuota: Int,

Expand All @@ -47,7 +44,6 @@ data class Configuration constructor(
val ignoreFailureRegexes: Collection<Regex>,
val failFastFailureRegexes: Collection<Regex>,

val testBatchTimeoutMillis: Long,
val testOutputTimeoutMillis: Long,
val noDevicesTimeoutMillis: Long,
val debug: Boolean,
Expand All @@ -71,8 +67,6 @@ data class Configuration constructor(

cache: CacheConfiguration?,
ignoreFailures: Boolean?,
isCodeCoverageEnabled: Boolean?,
fallbackToScreenshots: Boolean?,
strictMode: Boolean?,
uncompletedTestRetryQuota: Int?,

Expand All @@ -82,7 +76,6 @@ data class Configuration constructor(
ignoreFailureRegexes: Collection<Regex>?,
failFastFailureRegexes: Collection<Regex>?,

testBatchTimeoutMillis: Long?,
testOutputTimeoutMillis: Long?,
noDevicesTimeoutMillis: Long?,
debug: Boolean?,
Expand All @@ -103,8 +96,6 @@ data class Configuration constructor(
strictRunFilterConfiguration = strictRunFilterConfiguration ?: StrictRunFilterConfiguration(emptyList()),
cache = cache ?: CacheConfiguration(),
ignoreFailures = ignoreFailures ?: false,
isCodeCoverageEnabled = isCodeCoverageEnabled ?: false,
fallbackToScreenshots = fallbackToScreenshots ?: false,
strictMode = strictMode ?: false,
listener = listener,
uncompletedTestRetryQuota = uncompletedTestRetryQuota ?: Integer.MAX_VALUE,
Expand All @@ -113,7 +104,6 @@ data class Configuration constructor(
excludeSerialRegexes = excludeSerialRegexes ?: emptyList(),
ignoreFailureRegexes = ignoreFailureRegexes ?: emptyList(),
failFastFailureRegexes = failFastFailureRegexes ?: emptyList(),
testBatchTimeoutMillis = testBatchTimeoutMillis ?: DEFAULT_EXECUTION_TIMEOUT_MILLIS,
testOutputTimeoutMillis = testOutputTimeoutMillis ?: DEFAULT_OUTPUT_TIMEOUT_MILLIS,
noDevicesTimeoutMillis = noDevicesTimeoutMillis ?: DEFAULT_NO_DEVICES_TIMEOUT_MILLIS,
debug = debug ?: true,
Expand All @@ -133,13 +123,10 @@ data class Configuration constructor(
"strictRunFilter" to strictRunFilterConfiguration.toString(),
"cache" to cache.toString(),
"ignoreFailures" to ignoreFailures.toString(),
"isCodeCoverageEnabled" to isCodeCoverageEnabled.toString(),
"fallbackToScreenshots" to fallbackToScreenshots.toString(),
"strictMode" to strictMode.toString(),
"testClassRegexes" to testClassRegexes.toString(),
"includeSerialRegexes" to includeSerialRegexes.toString(),
"excludeSerialRegexes" to excludeSerialRegexes.toString(),
"testBatchTimeoutMillis" to testBatchTimeoutMillis.toString(),
"testOutputTimeoutMillis" to testOutputTimeoutMillis.toString(),
"noDevicesTimeoutMillis" to noDevicesTimeoutMillis.toString(),
"debug" to debug.toString(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ package com.malinskiy.marathon.cache.test.key
import com.malinskiy.marathon.cache.CacheKey
import com.malinskiy.marathon.device.DevicePoolId
import com.malinskiy.marathon.execution.ComponentInfo
import com.malinskiy.marathon.execution.Configuration
import com.malinskiy.marathon.test.TestComponentInfo
import com.malinskiy.marathon.test.factory.configuration
import kotlinx.coroutines.runBlocking
import org.amshove.kluent.shouldBeEqualTo
import org.amshove.kluent.shouldNotBeEqualTo
Expand Down Expand Up @@ -75,26 +73,6 @@ class TestCacheKeyFactoryTest {
}
}

@Test
fun differentCacheKeysForDifferentCodeCoverageConfigurations() {
runBlocking {
val firstKey = createCacheKey(configuration = createConfiguration(codeCoverageEnabled = true))
val secondKey = createCacheKey(configuration = createConfiguration(codeCoverageEnabled = false))

firstKey shouldNotBeEqualTo secondKey
}
}

@Test
fun sameCacheKeysForSameCodeCoverageConfigurations() {
runBlocking {
val firstKey = createCacheKey(configuration = createConfiguration(codeCoverageEnabled = true))
val secondKey = createCacheKey(configuration = createConfiguration(codeCoverageEnabled = true))

firstKey shouldBeEqualTo secondKey
}
}

@Test
fun differentCacheKeysForDifferentTestPackageNames() {
runBlocking {
Expand Down Expand Up @@ -159,7 +137,6 @@ class TestCacheKeyFactoryTest {
private fun createCacheKey(
marathonVersion: String = "123",
componentCacheKey: String = "abc",
configuration: Configuration = createConfiguration(),
devicePoolId: DevicePoolId = DevicePoolId("omni"),
test: MarathonTest = createTest()
): CacheKey = runBlocking {
Expand All @@ -169,7 +146,7 @@ private fun createCacheKey(
val versionNameProvider = mock<VersionNameProvider> {
on { this.versionName }.thenReturn(marathonVersion)
}
val cacheKeyFactory = TestCacheKeyFactory(componentCacheKeyProvider, versionNameProvider, configuration)
val cacheKeyFactory = TestCacheKeyFactory(componentCacheKeyProvider, versionNameProvider)
cacheKeyFactory.getCacheKey(devicePoolId, test)
}

Expand All @@ -184,7 +161,3 @@ private fun createTest(
componentInfo = TestComponentInfo(someInfo = "someInfo", name = "component-name"),
metaProperties = emptyList()
)

private fun createConfiguration(codeCoverageEnabled: Boolean = false) = configuration {
isCodeCoverageEnabled = codeCoverageEnabled
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import com.malinskiy.marathon.android.DEFAULT_APPLICATION_PM_CLEAR
import com.malinskiy.marathon.android.DEFAULT_AUTO_GRANT_PERMISSION
import com.malinskiy.marathon.android.DEFAULT_INSTALL_OPTIONS
import com.malinskiy.marathon.android.DEFAULT_USED_STORAGE_THRESHOLD_PERCENTS
import com.malinskiy.marathon.android.defaultInitTimeoutMillis
import com.malinskiy.marathon.android.serial.SerialStrategy
import com.malinskiy.marathon.execution.Configuration
import ddmlibModule
Expand All @@ -29,8 +28,6 @@ internal fun createCommonConfiguration(
strictRunFilterConfiguration = extensionConfig.strictRunFilterConfiguration?.toStrictRunFilterConfiguration(),
cache = extensionConfig.cache?.toCacheConfiguration(),
ignoreFailures = extensionConfig.ignoreFailures,
isCodeCoverageEnabled = extensionConfig.isCodeCoverageEnabled,
fallbackToScreenshots = extensionConfig.fallbackToScreenshots,
strictMode = extensionConfig.strictMode,
listener = extensionConfig.listener,
uncompletedTestRetryQuota = extensionConfig.uncompletedTestRetryQuota,
Expand All @@ -39,7 +36,6 @@ internal fun createCommonConfiguration(
excludeSerialRegexes = extensionConfig.excludeSerialRegexes?.map { it.toRegex() },
ignoreFailureRegexes = extensionConfig.ignoreFailureRegexes?.map { it.toRegex(RegexOption.DOT_MATCHES_ALL) },
failFastFailureRegexes = extensionConfig.failFastFailureRegexes?.map { it.toRegex(RegexOption.DOT_MATCHES_ALL) },
testBatchTimeoutMillis = extensionConfig.testBatchTimeoutMillis,
testOutputTimeoutMillis = extensionConfig.testOutputTimeoutMillis,
noDevicesTimeoutMillis = extensionConfig.noDevicesTimeoutMillis,
debug = extensionConfig.debug,
Expand All @@ -51,7 +47,6 @@ private fun createAndroidConfiguration(extension: MarathonExtension, sdkDirector
val instrumentationArgs = extension.instrumentationArgs
val applicationPmClear = extension.applicationPmClear ?: DEFAULT_APPLICATION_PM_CLEAR
val testApplicationPmClear = extension.testApplicationPmClear ?: DEFAULT_APPLICATION_PM_CLEAR
val adbInitTimeout = extension.adbInitTimeout ?: defaultInitTimeoutMillis
val installOptions = extension.installOptions ?: DEFAULT_INSTALL_OPTIONS
val preferableRecorderType = extension.preferableRecorderType
val serialStrategy = extension.serialStrategy
Expand All @@ -74,7 +69,6 @@ private fun createAndroidConfiguration(extension: MarathonExtension, sdkDirector
instrumentationArgs,
applicationPmClear,
testApplicationPmClear,
adbInitTimeout,
installOptions,
preferableRecorderType,
serialStrategy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ open class MarathonExtension {

var cache: CachePluginConfiguration? = null
var ignoreFailures: Boolean? = null
var isCodeCoverageEnabled: Boolean? = null
var fallbackToScreenshots: Boolean? = null
var strictMode: Boolean? = null
var uncompletedTestRetryQuota: Int? = null

Expand All @@ -37,14 +35,12 @@ open class MarathonExtension {
*/
var failFastFailureRegexes: Collection<String>? = null

var testBatchTimeoutMillis: Long? = null
var testOutputTimeoutMillis: Long? = null
var noDevicesTimeoutMillis: Long? = null
var debug: Boolean? = null

var applicationPmClear: Boolean? = null
var testApplicationPmClear: Boolean? = null
var adbInitTimeout: Int? = null
var installOptions: String? = null
var serialStrategy: SerialStrategyConfiguration? = null

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import com.malinskiy.marathon.vendor.VendorConfiguration
import org.koin.core.module.Module
import java.io.File

const val defaultInitTimeoutMillis = 30_000

const val DEFAULT_AUTO_GRANT_PERMISSION = false
const val DEFAULT_APPLICATION_PM_CLEAR = false
const val DEFAULT_TEST_APPLICATION_PM_CLEAR = false
Expand All @@ -22,7 +20,6 @@ data class AndroidConfiguration(
val instrumentationArgs: Map<String, String> = emptyMap(),
val applicationPmClear: Boolean = DEFAULT_APPLICATION_PM_CLEAR,
val testApplicationPmClear: Boolean = DEFAULT_TEST_APPLICATION_PM_CLEAR,
val adbInitTimeoutMillis: Int = defaultInitTimeoutMillis,
val installOptions: String = DEFAULT_INSTALL_OPTIONS,
val preferableRecorderType: DeviceFeature? = null,
val serialStrategy: SerialStrategy = SerialStrategy.AUTOMATIC,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ class AndroidDeviceTestRunnerSpek : Spek(
strictRunFilterConfiguration = null,
cache = null,
ignoreFailures = null,
isCodeCoverageEnabled = null,
fallbackToScreenshots = null,
strictMode = null,
listener = null,
uncompletedTestRetryQuota = null,
Expand All @@ -71,7 +69,6 @@ class AndroidDeviceTestRunnerSpek : Spek(
excludeSerialRegexes = null,
ignoreFailureRegexes = null,
failFastFailureRegexes = null,
testBatchTimeoutMillis = null,
testOutputTimeoutMillis = null,
noDevicesTimeoutMillis = null,
debug = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ class ConfigurationFactory {
var excludeSerialRegexes: List<Regex>? = null
var ignoreFailureRegexes: List<Regex>? = null
var failFastFailureRegexes: List<Regex>? = null
var fallbackToScreenshots: Boolean? = null
var strictMode: Boolean? = null
var uncompletedTestRetryQuota: Int? = null
var filteringConfiguration: FilteringConfiguration? = null
Expand All @@ -39,13 +38,11 @@ class ConfigurationFactory {
var cache: CacheConfiguration? = null
var ignoreFailures: Boolean? = null
var includeSerialRegexes: List<Regex>? = null
var isCodeCoverageEnabled: Boolean? = null
var poolingStrategy: PoolingStrategy? = null
var retryStrategy: RetryStrategy? = null
var shardingStrategy: ShardingStrategy? = null
var sortingStrategy: SortingStrategy? = null
var testClassRegexes: Collection<Regex>? = null
var testBatchTimeoutMillis: Long? = null
var testOutputTimeoutMillis: Long? = null
var noDevicesTimeoutMillis: Long? = null

Expand Down Expand Up @@ -73,8 +70,6 @@ class ConfigurationFactory {
strictRunFilterConfiguration = strictRunFilterConfiguration,
cache = cache,
ignoreFailures = ignoreFailures,
isCodeCoverageEnabled = isCodeCoverageEnabled,
fallbackToScreenshots = fallbackToScreenshots,
strictMode = strictMode,
listener = listener,
uncompletedTestRetryQuota = uncompletedTestRetryQuota,
Expand All @@ -83,7 +78,6 @@ class ConfigurationFactory {
excludeSerialRegexes = excludeSerialRegexes,
ignoreFailureRegexes = ignoreFailureRegexes,
failFastFailureRegexes = failFastFailureRegexes,
testBatchTimeoutMillis = testBatchTimeoutMillis,
testOutputTimeoutMillis = testOutputTimeoutMillis,
noDevicesTimeoutMillis = noDevicesTimeoutMillis,
debug = debug,
Expand Down
Loading