Skip to content

Commit 264ff67

Browse files
author
Sergey Chelombitko
committed
Refactor logging
1 parent 497d2a3 commit 264ff67

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+333
-551
lines changed

core/build.gradle.kts

-2
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,11 @@ dependencies {
2525
implementation(libs.apache.commons.io)
2626
implementation(libs.apache.commons.text)
2727
implementation(libs.gson)
28-
implementation(libs.kotlin.logging)
2928
implementation(libs.kotlinx.coroutines.core)
3029
implementation(libs.ktor.client.core)
3130
implementation(libs.ktor.client.auth)
3231
implementation(libs.ktor.client.apache)
3332
implementation(libs.jackson.annotations)
34-
implementation(libs.logback.classic)
3533
implementation(libs.slf4j.api)
3634
api(libs.koin.core)
3735

core/src/main/kotlin/com/malinskiy/marathon/Marathon.kt

+13-20
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import com.malinskiy.marathon.execution.StrictRunProcessor
1818
import com.malinskiy.marathon.execution.TestParser
1919
import com.malinskiy.marathon.execution.TestShard
2020
import com.malinskiy.marathon.execution.progress.ProgressReporter
21-
import com.malinskiy.marathon.log.MarathonLogConfigurator
2221
import com.malinskiy.marathon.log.MarathonLogging
2322
import com.malinskiy.marathon.report.logs.LogsProvider
2423
import com.malinskiy.marathon.test.Test
@@ -38,13 +37,12 @@ class Marathon(
3837
private val cachedTestsReporter: CacheTestReporter,
3938
private val progressReporter: ProgressReporter,
4039
private val strictRunChecker: StrictRunChecker,
41-
private val logConfigurator: MarathonLogConfigurator,
4240
private val logsProvider: LogsProvider,
4341
private val track: Track,
4442
private val timer: Timer
4543
) : MarathonRunner {
4644

47-
private val logger = MarathonLogging.logger("Marathon")
45+
private val logger = MarathonLogging.getLogger(Marathon::class.java)
4846

4947
private val configurationValidator = LogicalConfigurationValidator()
5048
private val strictRunProcessor = StrictRunProcessor(configuration.strictRunConfiguration)
@@ -54,14 +52,18 @@ class Marathon(
5452
override suspend fun start() {
5553
logger.debug("Starting Marathon")
5654

57-
MarathonLogging.debug = configuration.debug
58-
logConfigurator.configure()
55+
configurationValidator.validate(configuration)
5956

57+
logger.debug("Initializing device provider")
6058
deviceProvider.initialize()
61-
logger.debug("Finished loading device provider")
6259

63-
configurationValidator.validate(configuration)
60+
if (configuration.outputDir.exists()) {
61+
logger.info("Cleaning output directory ${configuration.outputDir}")
62+
configuration.outputDir.deleteRecursively()
63+
}
64+
configuration.outputDir.mkdirs()
6465

66+
logger.debug("Initializing scheduler")
6567
val currentCoroutineContext = coroutineContext
6668
scheduler = Scheduler(
6769
deviceProvider,
@@ -77,38 +79,29 @@ class Marathon(
7779
timer,
7880
currentCoroutineContext
7981
)
80-
81-
logger.debug("Created scheduler")
82-
83-
if (configuration.outputDir.exists()) {
84-
logger.info("Output directory ${configuration.outputDir} already exists")
85-
configuration.outputDir.deleteRecursively()
86-
}
87-
configuration.outputDir.mkdirs()
88-
8982
scheduler.initialize()
9083
}
9184

9285
override suspend fun scheduleTests(componentInfo: ComponentInfo) {
9386
val parsedTests = testParser.extract(componentInfo)
9487
val tests = applyTestFilters(parsedTests)
9588

96-
logger.info("Scheduling ${tests.size} tests for $componentInfo")
89+
logger.info("Scheduling {} tests for {}", tests.size, componentInfo)
9790
logger.debug(tests.joinToString(", ") { it.toTestName() })
9891

9992
val shard = prepareTestShard(tests, analytics)
10093
scheduler.addTests(shard)
10194
}
10295

10396
override suspend fun stopAndWaitForCompletion(): Boolean {
104-
logger.debug("Waiting for completion")
97+
logger.debug("Waiting for test run to complete")
10598

10699
try {
107100
scheduler.stopAndWaitForCompletion()
108101
onFinish()
109102
} catch (@Suppress("TooGenericExceptionCaught") e: Throwable) {
110103
// We don't want to catch these. If an exception was thrown, we should fail the execution
111-
logger.error("Error occurred while finishing tests run", e)
104+
logger.error("An error occurred while finishing test run", e)
112105
throw e
113106
}
114107
return progressReporter.aggregateResult()
@@ -125,7 +118,7 @@ class Marathon(
125118
try {
126119
tracker.finish()
127120
} catch (@Suppress("TooGenericExceptionCaught") e: Throwable) {
128-
throw ReportGenerationException("Failed to generate test run report with exception", e)
121+
throw ReportGenerationException("Failed to generate test run report", e)
129122
}
130123
}
131124

core/src/main/kotlin/com/malinskiy/marathon/cache/gradle/GradleHttpCacheService.kt

+5-5
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,23 @@ class GradleHttpCacheService(private val configuration: RemoteCacheConfiguration
2929

3030
private val httpClient = createClient()
3131

32-
private val logger = MarathonLogging.logger("GradleHttpCacheService")
32+
private val logger = MarathonLogging.getLogger(GradleHttpCacheService::class.java)
3333

3434
override suspend fun load(key: CacheKey, reader: CacheEntryReader): Boolean =
3535
withContext(Dispatchers.IO) {
3636
try {
3737
val response = httpClient.get(key.key)
3838
if (response.status != HttpStatusCode.OK) {
3939
if (response.status != HttpStatusCode.NotFound) {
40-
logger.warn("Got response status when loading cache entry for ${key.key} : ${response.status}")
40+
logger.warn("Got response status {} when loading cache entry for {}", response.status, key.key)
4141
}
4242
false
4343
} else {
4444
reader.readFrom(response.bodyAsChannel())
4545
true
4646
}
4747
} catch (exception: IOException) {
48-
logger.warn("Error during loading cache entry for ${key.key}", exception)
48+
logger.warn("Error loading cache entry for {}", key.key, exception)
4949
false
5050
}
5151
}
@@ -59,10 +59,10 @@ class GradleHttpCacheService(private val configuration: RemoteCacheConfiguration
5959
setBody(ByteArrayContent(stream.toByteArray()))
6060
}
6161
if (!response.status.isSuccess()) {
62-
logger.warn("Got response status when storing cache entry for ${key.key} : ${response.status}")
62+
logger.warn("Got response status {} when storing cache entry for {}", response.status, key.key)
6363
}
6464
} catch (exception: IOException) {
65-
logger.warn("Error during storing cache entry for ${key.key}", exception)
65+
logger.warn("Error storing cache entry for {}", key.key, exception)
6666
} finally {
6767
stream.close()
6868
}

core/src/main/kotlin/com/malinskiy/marathon/cache/test/TestCacheLoader.kt

+9-11
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class TestCacheLoader(
2424
private val cacheKeyFactory: TestCacheKeyFactory
2525
) {
2626

27-
private val logger = MarathonLogging.logger("TestCacheLoader")
27+
private val logger = MarathonLogging.getLogger(TestCacheLoader::class.java)
2828

2929
private val _results: Channel<CacheResult> = unboundedChannel()
3030
val results: ReceiveChannel<CacheResult>
@@ -49,13 +49,11 @@ class TestCacheLoader(
4949
_results.send(result!!)
5050
}
5151

52-
logger.debug {
53-
val hitOrMiss = when (result!!) {
54-
is Hit -> "hit"
55-
is Miss -> "miss"
56-
}
57-
"Cache $hitOrMiss for ${test.test.toSimpleSafeTestName()}, took $timeMillis milliseconds"
52+
val hitOrMiss = when (result!!) {
53+
is Hit -> "hit"
54+
is Miss -> "miss"
5855
}
56+
logger.debug("Cache {} for {} took {} milliseconds", hitOrMiss, test.test.toSimpleSafeTestName(), timeMillis)
5957
}
6058
}
6159
}
@@ -67,12 +65,12 @@ class TestCacheLoader(
6765
if (configuration.strictRunConfiguration.filter.matches(test)) {
6866
testCacheBlackList.add(test)
6967
} else {
70-
testsToCheck.send(TestToCheck(poolId, test, isStrictRun = false))
68+
testsToCheck.send(TestToCheck(poolId, test))
7169
}
7270
}
7371

7472
if (testCacheBlackList.isNotEmpty()) {
75-
logger.debug { "Cache miss for test in blacklist: ${testCacheBlackList.map { it.toSimpleSafeTestName() }} " }
73+
logger.debug("Cache miss for tests in blacklist: {}", testCacheBlackList.map { it.toSimpleSafeTestName() })
7674
_results.send(Miss(poolId, TestShard(testCacheBlackList)))
7775
}
7876
} else {
@@ -84,8 +82,8 @@ class TestCacheLoader(
8482
testsToCheck.close()
8583
cacheCheckCompleted.await()
8684
_results.close()
87-
logger.debug { "Cache loader is terminated" }
85+
logger.debug("Cache loader is terminated")
8886
}
8987

90-
private class TestToCheck(val poolId: DevicePoolId, val test: Test, val isStrictRun: Boolean)
88+
private class TestToCheck(val poolId: DevicePoolId, val test: Test)
9189
}

core/src/main/kotlin/com/malinskiy/marathon/cache/test/TestCacheSaver.kt

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class TestCacheSaver(
1616
private val testCacheKeyProvider: TestCacheKeyFactory
1717
) {
1818

19-
private val logger = MarathonLogging.logger("TestCacheSaver")
19+
private val logger = MarathonLogging.getLogger(TestCacheSaver::class.java)
2020

2121
private val tasks: Channel<SaveTask> = unboundedChannel()
2222
private lateinit var completableDeferred: Deferred<Unit>
@@ -38,7 +38,7 @@ class TestCacheSaver(
3838
suspend fun terminate() {
3939
tasks.close()
4040
completableDeferred.await()
41-
logger.debug { "Cache saver is terminated" }
41+
logger.debug("Terminated test cache saver")
4242
}
4343

4444
private class SaveTask(val poolId: DevicePoolId, val result: TestResult)

core/src/main/kotlin/com/malinskiy/marathon/cache/test/TestResultsCache.kt

+5-5
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class TestResultsCache(
1818
private val track: Track
1919
) {
2020

21-
private val logger = MarathonLogging.logger("TestResultsCache")
21+
private val logger = MarathonLogging.getLogger(TestResultsCache::class.java)
2222

2323
suspend fun load(key: CacheKey, test: Test): TestResult? {
2424
val start = Instant.now()
@@ -28,8 +28,8 @@ class TestResultsCache(
2828
return null
2929
}
3030
return reader.testResult
31-
} catch (exception: Throwable) {
32-
logger.warn("Error during loading cache entry for ${test.toSimpleSafeTestName()}", exception)
31+
} catch (e: Throwable) {
32+
logger.warn("Error during loading cache entry for {}", test.toSimpleSafeTestName(), e)
3333
return null
3434
} finally {
3535
val finish = Instant.now()
@@ -42,8 +42,8 @@ class TestResultsCache(
4242
try {
4343
val writer = TestResultEntryWriter(testResult)
4444
cacheService.store(key, writer)
45-
} catch (exception: Throwable) {
46-
logger.warn("Error during storing cache entry for ${testResult.test.toSimpleSafeTestName()}", exception)
45+
} catch (e: Throwable) {
46+
logger.warn("Error during storing cache entry for {}", testResult.test.toSimpleSafeTestName(), e)
4747
} finally {
4848
val finish = Instant.now()
4949
track.cacheStore(start, finish, testResult.test)

core/src/main/kotlin/com/malinskiy/marathon/di/Modules.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ fun coreModule(timer: Timer?) = module {
6868
single<Timer> { timer ?: SystemTimer(get()) }
6969
single<ProgressReporter> { ProgressReporter(get()) }
7070
single<StrictRunChecker> { ConfigurationStrictRunChecker(get()) }
71-
single<Marathon> { Marathon(get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get()) }
71+
single<Marathon> { Marathon(get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get(), get()) }
7272
}
7373

7474
fun KoinApplication.marathonConfiguration(configuration: Configuration): KoinApplication {
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
package com.malinskiy.marathon.exceptions
22

3-
class NoDevicesException(message: String) : RuntimeException(message)
3+
class NoDevicesException(message: String = "No devices found") : RuntimeException(message)

core/src/main/kotlin/com/malinskiy/marathon/execution/Configuration.kt

-4
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ data class Configuration(
3232
val filteringConfiguration: FilteringConfiguration,
3333
val strictRunConfiguration: StrictRunConfiguration,
3434

35-
val debug: Boolean,
3635
val ignoreFailures: Boolean,
3736
val strictMode: Boolean,
3837
val uncompletedTestRetryQuota: Int,
@@ -64,7 +63,6 @@ data class Configuration(
6463
filteringConfiguration: FilteringConfiguration?,
6564
strictRunConfiguration: StrictRunConfiguration?,
6665

67-
debug: Boolean?,
6866
ignoreFailures: Boolean?,
6967
strictMode: Boolean?,
7068
uncompletedTestRetryQuota: Int?,
@@ -94,7 +92,6 @@ data class Configuration(
9492
retryStrategy = retryStrategy ?: NoRetryStrategy(),
9593
filteringConfiguration = filteringConfiguration ?: FilteringConfiguration(),
9694
strictRunConfiguration = strictRunConfiguration ?: StrictRunConfiguration(),
97-
debug = debug ?: true,
9895
ignoreFailures = ignoreFailures ?: false,
9996
strictMode = strictMode ?: false,
10097
uncompletedTestRetryQuota = uncompletedTestRetryQuota ?: Integer.MAX_VALUE,
@@ -122,7 +119,6 @@ data class Configuration(
122119
"retry" to retryStrategy.toString(),
123120
"filtering" to filteringConfiguration.toString(),
124121
"strictRun" to strictRunConfiguration.toString(),
125-
"debug" to debug.toString(),
126122
"ignoreFailures" to ignoreFailures.toString(),
127123
"strictMode" to strictMode.toString(),
128124
"includeSerialRegexes" to includeSerialRegexes.toString(),

core/src/main/kotlin/com/malinskiy/marathon/execution/DevicePoolActor.kt

+8-8
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class DevicePoolActor(
4141
) :
4242
Actor<DevicePoolMessage>(parent = parent, context = context) {
4343

44-
private val logger = MarathonLogging.logger("DevicePoolActor[${poolId.name}]")
44+
private val logger = MarathonLogging.getLogger("DevicePoolActor[${poolId.name}]")
4545

4646
override suspend fun receive(msg: DevicePoolMessage) {
4747
when (msg) {
@@ -82,7 +82,7 @@ class DevicePoolActor(
8282
private var noDevicesTimeoutDeferred: Deferred<Unit>? = null
8383

8484
private suspend fun notifyDevices() {
85-
logger.debug { "Notify devices" }
85+
logger.debug("Notify devices")
8686
devices.values.forEach {
8787
it.safeSend(DeviceEvent.WakeUp)
8888
}
@@ -146,19 +146,19 @@ class DevicePoolActor(
146146
}
147147

148148
private suspend fun removeDevice(device: Device) {
149-
logger.debug { "remove device ${device.serialNumber}" }
149+
logger.debug("Removing device {}", device.serialNumber)
150150
val actor = devices.remove(device.serialNumber)
151151
actor?.safeSend(DeviceEvent.Terminate)
152-
logger.debug { "devices.size = ${devices.size}" }
152+
logger.debug("devices.size = {}", devices.size)
153153
if (noActiveDevices()) {
154154
if (!queue.stopRequested) return // we may receive new tests in the future
155155

156156
noDevicesTimeoutDeferred?.cancel()
157157

158-
logger.debug { "scheduling terminating of device pool actor as no devices found" }
158+
logger.debug("Scheduling termination of device pool actor as no devices found")
159159
noDevicesTimeoutDeferred = async(poolJob) {
160160
delay(TimeUnit.MINUTES.toMillis(NO_DEVICES_IN_POOL_TIMEOUT_MINUTES))
161-
logger.debug { "terminating device pool actor as no devices found after timeout" }
161+
logger.debug("Terminating device pool actor as no devices found after timeout")
162162
terminate()
163163
}
164164
}
@@ -170,11 +170,11 @@ class DevicePoolActor(
170170

171171
private suspend fun addDevice(device: Device) {
172172
if (devices.containsKey(device.serialNumber)) {
173-
logger.warn { "device ${device.serialNumber} already present in pool ${poolId.name}" }
173+
logger.warn("Device {} is already present in pool {}", device.serialNumber, poolId.name)
174174
return
175175
}
176176

177-
logger.debug { "add device ${device.serialNumber}" }
177+
logger.debug("Adding device {}", device.serialNumber)
178178

179179
noDevicesTimeoutDeferred?.cancel()
180180

0 commit comments

Comments
 (0)