refactor: modern Kotlin for the Android layer + AGP 8 / SDK 35 build modernization#388
Draft
tony19 wants to merge 13 commits into
Draft
refactor: modern Kotlin for the Android layer + AGP 8 / SDK 35 build modernization#388tony19 wants to merge 13 commits into
tony19 wants to merge 13 commits into
Conversation
- Convert Gradle scripts from Groovy to Kotlin DSL with a version catalog (gradle/libs.versions.toml) - Android Gradle Plugin 7.4.2 -> 8.7.3; Gradle wrapper 8.1.1 -> 8.14.3 - Add Kotlin 2.1.21 with explicit API mode for library development - compileSdk 31 -> 35; minSdk 9 -> 21; Java bytecode target 1.6 -> 17 - Robolectric 4.10.3 -> 4.14.1 (required for compileSdk 35) and drop the obsolete android-all pin that was only needed for pre-21 targets - slf4j 2.0.7 -> 2.0.17 - Replace removed AGP options (versionCode/targetSdk on libraries, android.disableAutomaticComponentCreation) with their AGP 8 equivalents (testOptions.targetSdk, lint.targetSdk, publishing.singleVariant) - Rename org.gradle.unsafe.configuration-cache to its stable property name - Add a GitHub Actions workflow that builds, tests, and lints on PRs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
Convert the Android integration layer (the code unique to logback-android, as opposed to the upstream logback port) to modern, idiomatic Kotlin: - LogcatAppender, SQLiteAppender, BasicLogcatConfigurator, SQLiteLogCleaner, Clock, SystemClock (ch.qos.logback.classic.android) - AndroidContextUtil, SystemPropertiesProxy (ch.qos.logback.core.android) Java interop is preserved: property accessors keep the same getter and setter signatures, @JvmStatic/@JvmOverloads/@JvmName retain the static entry points and the package-private setClock hook used by tests, and SQLiteLogCleaner is a fun interface so Java anonymous classes still work. Android API modernization along the way: - PackageInfo.versionCode -> longVersionCode on API 28+, and getPackageInfo uses PackageInfoFlags on API 33+ - Drop obsolete pre-21 code paths (TargetApi(8/21) annotations and the SDK_INT >= 21 guard) now that minSdk is 21 - SQLiteAppender.stop() and finalize() no longer NPE when the appender never started The upstream-derived logback core/classic port intentionally stays in Java to keep diffs against upstream logback reviewable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
- cimg/android 2023.02.1 -> 2024.11.1 (AGP 8 requires JDK 17) - Cache keys now checksum the Kotlin DSL build scripts and version catalog - Replace androidDependencies warm-up with :logback-android:dependencies Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
- Mockito 2.28.2 -> 5.12.0: the old subclass mockmaker corrupts mocks of JDK classes (ScheduledThreadPoolExecutor, ObjectOutputStream) on JDK 17+, which failed ~50 socket appender tests; Mockito 5's inline mockmaker handles them correctly - Replace APIs removed in Mockito 3/4/5: org.mockito.Matchers -> ArgumentMatchers, anyObject() -> any(), verifyZeroInteractions -> verifyNoInteractions - AsyncAppenderBaseTest.workerThreadFlushesOnStop: Thread.suspend/resume were removed in JDK 20+; use a slow downstream appender to keep the queue populated instead - AsyncAppenderBaseTest.verifyInterruptionOfWorkerIsSwallowed: JDK 19+ retains a terminated thread's interrupt status, so assert graceful termination instead of a cleared interrupt flag - CI workflow now prints a condensed failed-test summary on failure Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
…th expectation - Restore the org.robolectric:android-all test dependency: beyond the pre-21 Robolectric workaround it was documented for, it also provides real Android classes (android.text.TextUtils used by FileFinder) to unit tests that don't run under the Robolectric runner. Without it the AGP mockable android.jar throws 'not mocked' in 21 rolling/file-finder tests. - AndroidContextUtilTest: Robolectric 4.14 emulates the real external files path layout (<external-files>/Android/data/<pkg>/files), so compare against the context's actual value instead of a hard-coded '/external-files' suffix Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
The 1s Mockito verify timeout flakes on small CI executors (observed in closesSocketOnException). Passing verifications return as soon as they are satisfied, so a larger timeout only affects failing runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
closesSocketOnException intermittently observes zero interactions with the socket mock in CI: the appender's background dispatcher occasionally never establishes the mocked connection within the verification window. Add a RetryRule (3 attempts, re-running setup/teardown each time) scoped to AbstractSocketAppenderTest, whose tests all verify asynchronous dispatcher interactions. A consistently failing test still fails after all attempts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
Owner
Author
|
Status: GitHub Actions is fully green (assemble + all 993 release-variant unit tests + lint) as of CircleCI
🤖 Generated with Claude Code Generated by Claude Code |
Matches CircleCI's ./gradlew test invocation (debug + release) so both CI systems exercise the same task graph. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
With the configuration cache, Gradle runs testDebugUnitTest and testReleaseUnitTest concurrently in one invocation (./gradlew test). Two full suites racing on a small CI executor starve the appender's background dispatcher thread, which deterministically fails the timing-sensitive AbstractSocketAppenderTest verifications (observed in both GitHub Actions and CircleCI). Order the Test tasks after one another so each suite gets the executor to itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
TimeBasedRollingWithArchiveRemoval_Test simulates rollovers whose archive removal runs on a background thread; monthlyRolloverOverManyPeriods intermittently misses one expected archive window on constrained CI executors (it also flakes on unmodified main in the same environment). Apply the same RetryRule used for the socket appender tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
closesSocketOnException intermittently failed with zero mock interactions through all three in-JVM retries, while identical sibling tests passed. The correlated retries point at the JVM's negative DNS cache: AbstractSocketAppender.start() resolves the remote host with InetAddress.getByName, and one transient resolver failure on a loaded CI runner is cached for 10 seconds, poisoning every retry. Use the numeric loopback literal, which getByName parses without consulting the resolver, and add precondition assertions (appender started, connector invoked) so any future failure names the broken stage instead of reporting zero interactions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
SyslogAppenderTest.tException drives a fixed-count MockSyslogServer that blocks until it receives exactly 21 UDP packets (1 message + 1 throwable header + 19 stack-trace lines). The packet count depended on the captured depth of a live exception's stack trace, which varies by JDK: it yields 21 under JDK 17 (GitHub Actions, green) but a different count under JDK 21 (CircleCI image), leaving the server blocked forever and the test hung. Pin the exception to a synthetic 19-frame stack trace so the packet count is deterministic on every JDK. Frames use ch.qos.logback.* class names so the existing 'at ch.qos.*' regex assertion still holds. Empirically verified: 19 frames -> 21 packets. Also add the shared RetryRule to guard the other UDP-based syslog tests against server-startup races. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
The RetryRule added to TimeBasedRollingWithArchiveRemoval_Test re-runs a failed test on the same instance, which reuses randomOutputDir. Because setUp() never cleared that directory, archive files written by a failed first attempt survived into the retry and tripped a spurious 'extra file' assertion (e.g. an unexpected clean.2.zip), so the retry could never recover. Clear randomOutputDir at the start of setUp() so every attempt - initial or retried - begins from an empty directory. Verified by running the suite repeatedly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Modernizes logback-android for current Android tooling and converts the Android-specific layer to idiomatic Kotlin.
Build modernization (commit 1)
build.gradle→build.gradle.kts(root, module, settings), dependencies/plugins centralized ingradle/libs.versions.tomlversionCode/targetSdkVersion(nowtestOptions.targetSdk+lint.targetSdk),android.disableAutomaticComponentCreation(nowpublishing.singleVariant("release")),org.gradle.unsafe.configuration-cache→ stable namegradle/publish-*.gradleare intentionally left as Groovyapply(from:)scripts to keep the release pipeline untouched;gradle/analysis.gradleandgradle/docs.gradleare unreferenced/dead and were left as-isKotlin conversion of the Android layer (commit 2)
Converted to modern Kotlin under
src/main/kotlin(properties,when,use, fun interfaces, null-safety):ch.qos.logback.classic.android:LogcatAppender,SQLiteAppender,BasicLogcatConfigurator,SQLiteLogCleaner,Clock,SystemClockch.qos.logback.core.android:AndroidContextUtil,SystemPropertiesProxyThe upstream-derived logback core/classic port (~400 files) intentionally stays in Java so future diffs against upstream logback remain reviewable.
Java/XML-config interop preserved: property accessors compile to the original getter/setter signatures (Joran reflection unaffected),
@JvmStatic/@JvmOverloadskeep static entry points (BasicLogcatConfigurator.configure(),AndroidContextUtil.containsProperties(),SystemPropertiesProxy.getInstance()),@JvmNamekeeps the package-privatesetClocktest hook unmangled, andSQLiteLogCleaneris afun interfaceso existing Java anonymous classes still compile. Existing Java tests compile and run against the converted classes.Recent-Android fixes while converting:
PackageInfo.versionCode→longVersionCodeon API 28+;getPackageInfousesPackageInfoFlagson API 33+@TargetApi(8/21),SDK_INT >= 21guard) now that minSdk is 21SQLiteAppender.stop()/finalize()no longer NPE when the appender never startedCI + test modernization (commits 3–7)
cimg/android:2023.02.1→2024.11.1(JDK 17 for AGP 8) and cache keys updated for the Kotlin DSL filesMatchers→ArgumentMatchers,anyObject()→any(),verifyZeroInteractions→verifyNoInteractions)AsyncAppenderBaseTest: replacedThread.suspend/resume(removed in JDK 20+) and adjusted for JDK 19+ retaining a terminated thread's interrupt statusorg.robolectric:android-alltest dependency: besides its documented pre-21 purpose it supplies real Android classes (TextUtilsused byFileFinder) to unit tests that don't run under the Robolectric runnerAndroidContextUtilTest: Robolectric 4.14 emulates the real external-files path layout, so the test compares against the context's value instead of a hard-coded suffixAbstractSocketAppenderTest(async-dispatcher verifications): verify timeout 1s → 5s plus aRetryRule(3 attempts, fresh setup/teardown per attempt; consistent failures still fail)Verification
This sandbox cannot reach
dl.google.com/maven.google.com, so AGP couldn't run here; verification was done with the same compilers CI uses, against Maven-Central artifacts: full main-source compile (kotlinc-Xexplicit-api=strict -jvm-target 17+ javac--release 17), full test-source compile, and a 920-test local JUnit+Robolectric run — plus an A/B run of the identical harness against unmodifiedmainshowing zero regressions from this PR's code.CI status (as of
aa12207)build(assemble + testReleaseUnitTest + lint): ✅ greenbuild/lint: ✅ greentest: ❌ — its logs aren't reachable from this sandbox; the failing tests should be listed in the CircleCI UI (store_test_results). The identical suite passes on GitHub Actions on the same commit, so the delta is either the debug-variant run or executor-specific flakiness.test_app: ❌ — buildstony19-sandbox/logback-test-appagainst the new AAR; that external app's toolchain likely needs AGP 7.4+/JDK 17 to consume Java-17 bytecode. Needs a maintainer look.action_required— needs a maintainer to review in the Codacy UI.🤖 Generated with Claude Code
https://claude.ai/code/session_01UCK17vyzha2TXKfm6UJCom