From 2214164f35812dbe1ef81302950224835e17c355 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Ricau Date: Thu, 6 Jan 2022 11:22:02 +0100 Subject: [PATCH] Allow updating LeakCanary config without installing Fixes #2270 --- .github/workflows/main.yml | 2 +- .../api/leakcanary-android-core.api | 5 ++ leakcanary-android-core/build.gradle | 3 +- .../java/leakcanary/LeakActivityTest.kt | 46 +++------- .../java/leakcanary/NoInstallTest.kt | 15 ++++ .../leakcanary/LazyForwardingEventListener.kt | 18 ++++ .../src/main/java/leakcanary/LeakCanary.kt | 4 +- .../RemoteWorkManagerHeapAnalyzer.kt | 3 +- .../java/leakcanary/ToastEventListener.kt | 4 +- .../leakcanary/WorkManagerHeapAnalyzer.kt | 3 +- leakcanary-android/build.gradle | 7 ++ .../java/leakcanary/LeakActivityTest.kt | 86 +++++++++++++++++++ 12 files changed, 150 insertions(+), 46 deletions(-) create mode 100644 leakcanary-android-core/src/androidTest/java/leakcanary/NoInstallTest.kt create mode 100644 leakcanary-android-core/src/main/java/leakcanary/LazyForwardingEventListener.kt create mode 100644 leakcanary-android/src/androidTest/java/leakcanary/LeakActivityTest.kt diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 3d28f7d382..da62c24257 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -29,7 +29,7 @@ jobs: uses: reactivecircus/android-emulator-runner@v2.14.3 with: api-level: 16 - script: ./gradlew leakcanary-android-core:connectedCheck leakcanary-android-instrumentation:connectedCheck --stacktrace + script: ./gradlew leakcanary-android-core:connectedCheck leakcanary-android:connectedCheck leakcanary-android-instrumentation:connectedCheck --stacktrace snapshot-deployment: if: github.repository == 'square/leakcanary' && github.event_name == 'push' diff --git a/leakcanary-android-core/api/leakcanary-android-core.api b/leakcanary-android-core/api/leakcanary-android-core.api index bcfa0c399a..57c3797cdf 100644 --- a/leakcanary-android-core/api/leakcanary-android-core.api +++ b/leakcanary-android-core/api/leakcanary-android-core.api @@ -78,6 +78,11 @@ public abstract interface class leakcanary/HeapDumper { public abstract fun dumpHeap (Ljava/io/File;)V } +public final class leakcanary/LazyForwardingEventListener : leakcanary/EventListener { + public fun (Lkotlin/jvm/functions/Function0;)V + public fun onEvent (Lleakcanary/EventListener$Event;)V +} + public final class leakcanary/LeakCanary { public static final field INSTANCE Lleakcanary/LeakCanary; public final fun dumpHeap ()V diff --git a/leakcanary-android-core/build.gradle b/leakcanary-android-core/build.gradle index 5183f7555d..b853d52cbe 100644 --- a/leakcanary-android-core/build.gradle +++ b/leakcanary-android-core/build.gradle @@ -20,11 +20,10 @@ dependencies { testImplementation libs.kotlin.reflect testImplementation libs.mockito testImplementation libs.mockitoKotlin - // AppWatcher auto installer for running tests - androidTestImplementation project(':leakcanary-object-watcher-android') androidTestImplementation libs.androidX.test.espresso androidTestImplementation libs.androidX.test.rules androidTestImplementation libs.androidX.test.runner + androidTestImplementation libs.assertjCore androidTestImplementation project(':shark-hprof-test') } diff --git a/leakcanary-android-core/src/androidTest/java/leakcanary/LeakActivityTest.kt b/leakcanary-android-core/src/androidTest/java/leakcanary/LeakActivityTest.kt index 323937384c..45e97d6132 100644 --- a/leakcanary-android-core/src/androidTest/java/leakcanary/LeakActivityTest.kt +++ b/leakcanary-android-core/src/androidTest/java/leakcanary/LeakActivityTest.kt @@ -1,7 +1,5 @@ package leakcanary -import android.content.Intent -import android.net.Uri import androidx.test.espresso.Espresso.onData import androidx.test.espresso.Espresso.onView import androidx.test.espresso.action.ViewActions.click @@ -12,6 +10,7 @@ import androidx.test.espresso.matcher.ViewMatchers.withText import androidx.test.platform.app.InstrumentationRegistry import androidx.test.rule.ActivityTestRule import com.squareup.leakcanary.core.R +import java.io.File import leakcanary.internal.activity.LeakActivity import leakcanary.internal.activity.db.HeapAnalysisTable import leakcanary.internal.activity.db.LeakTable.AllLeaksProjection @@ -30,9 +29,6 @@ import shark.LeakTraceObject import shark.OnAnalysisProgressListener import shark.ValueHolder.IntHolder import shark.dump -import java.io.File -import java.util.concurrent.CountDownLatch -import java.util.concurrent.TimeUnit.SECONDS internal class LeakActivityTest { @@ -93,27 +89,6 @@ internal class LeakActivityTest { .check(matches(isDisplayed())) } - @Test - fun importHeapDumpFile() = tryAndRestoreConfig { - val latch = CountDownLatch(1) - LeakCanary.config = LeakCanary.config.copy(onHeapAnalyzedListener = { - DefaultOnHeapAnalyzedListener.create().onHeapAnalyzed(it) - latch.countDown() - }) - val hprof = writeHeapDump { - "Holder" clazz { - staticField["leak"] = "com.example.Leaking" watchedInstance {} - } - } - val intent = Intent(Intent.ACTION_VIEW, Uri.fromFile(hprof)) - activityTestRule.launchActivity(intent) - require(latch.await(5, SECONDS)) - onView(withText("1 Heap Dump")).check(matches(isDisplayed())) - onData(withItem { it.leakCount == 1 }) - .perform(click()) - onView(withText("1 Distinct Leak")).check(matches(isDisplayed())) - } - private fun writeHeapDump(block: HprofWriterHelper.() -> Unit): File { val hprofFile = testFolder.newFile("temp.hprof") hprofFile.dump { @@ -163,14 +138,13 @@ internal class LeakActivityTest { } } } - - private fun tryAndRestoreConfig(block: () -> Unit) { - val original = LeakCanary.config - try { - block() - } finally { - LeakCanary.config = original - } +} + +fun tryAndRestoreConfig(block: () -> Unit) { + val original = LeakCanary.config + try { + block() + } finally { + LeakCanary.config = original } - -} \ No newline at end of file +} diff --git a/leakcanary-android-core/src/androidTest/java/leakcanary/NoInstallTest.kt b/leakcanary-android-core/src/androidTest/java/leakcanary/NoInstallTest.kt new file mode 100644 index 0000000000..ebb4fa5306 --- /dev/null +++ b/leakcanary-android-core/src/androidTest/java/leakcanary/NoInstallTest.kt @@ -0,0 +1,15 @@ +package leakcanary + +import org.assertj.core.api.Assertions.assertThat +import org.junit.Test + +class NoInstallTest { + + @Test fun appWatcher_is_not_installed() { + assertThat(AppWatcher.isInstalled).isFalse() + } + + @Test fun can_update_LeakCanary_config_without_installing() = tryAndRestoreConfig { + LeakCanary.config = LeakCanary.config.copy(dumpHeap = false) + } +} diff --git a/leakcanary-android-core/src/main/java/leakcanary/LazyForwardingEventListener.kt b/leakcanary-android-core/src/main/java/leakcanary/LazyForwardingEventListener.kt new file mode 100644 index 0000000000..95426a42ad --- /dev/null +++ b/leakcanary-android-core/src/main/java/leakcanary/LazyForwardingEventListener.kt @@ -0,0 +1,18 @@ +package leakcanary + +import leakcanary.EventListener.Event + +/** + * Forwards events to the [EventListener] provided by lazyEventListener which + * is evaluated lazily, when the first comes in. + */ +class LazyForwardingEventListener( + lazyEventListener: () -> EventListener +) : EventListener { + + private val eventListenerDelegate by lazy(lazyEventListener) + + override fun onEvent(event: Event) { + eventListenerDelegate.onEvent(event) + } +} diff --git a/leakcanary-android-core/src/main/java/leakcanary/LeakCanary.kt b/leakcanary-android-core/src/main/java/leakcanary/LeakCanary.kt index ce4862e0af..32e23d99c9 100644 --- a/leakcanary-android-core/src/main/java/leakcanary/LeakCanary.kt +++ b/leakcanary-android-core/src/main/java/leakcanary/LeakCanary.kt @@ -197,7 +197,9 @@ object LeakCanary { val eventListeners: List = listOf( LogcatEventListener, ToastEventListener, - if (InternalLeakCanary.formFactor == TV) TvEventListener else NotificationEventListener, + LazyForwardingEventListener { + if (InternalLeakCanary.formFactor == TV) TvEventListener else NotificationEventListener + }, when { RemoteWorkManagerHeapAnalyzer.remoteLeakCanaryServiceInClasspath -> RemoteWorkManagerHeapAnalyzer diff --git a/leakcanary-android-core/src/main/java/leakcanary/RemoteWorkManagerHeapAnalyzer.kt b/leakcanary-android-core/src/main/java/leakcanary/RemoteWorkManagerHeapAnalyzer.kt index d61b3eff4a..50f534fa43 100644 --- a/leakcanary-android-core/src/main/java/leakcanary/RemoteWorkManagerHeapAnalyzer.kt +++ b/leakcanary-android-core/src/main/java/leakcanary/RemoteWorkManagerHeapAnalyzer.kt @@ -29,10 +29,9 @@ object RemoteWorkManagerHeapAnalyzer : EventListener { } } - private val application = InternalLeakCanary.application - override fun onEvent(event: Event) { if (event is HeapDump) { + val application = InternalLeakCanary.application val heapAnalysisRequest = OneTimeWorkRequest.Builder(RemoteHeapAnalyzerWorker::class.java).apply { val dataBuilder = Data.Builder() diff --git a/leakcanary-android-core/src/main/java/leakcanary/ToastEventListener.kt b/leakcanary-android-core/src/main/java/leakcanary/ToastEventListener.kt index d2dc45bc22..6ad0b5dffb 100644 --- a/leakcanary-android-core/src/main/java/leakcanary/ToastEventListener.kt +++ b/leakcanary-android-core/src/main/java/leakcanary/ToastEventListener.kt @@ -18,8 +18,6 @@ import leakcanary.internal.friendly.mainHandler object ToastEventListener : EventListener { - private val appContext = InternalLeakCanary.application - // Only accessed from the main thread private var toastCurrentlyShown: Toast? = null @@ -36,7 +34,9 @@ object ToastEventListener : EventListener { } } + @Suppress("DEPRECATION") private fun showToastBlocking() { + val appContext = InternalLeakCanary.application val waitingForToast = CountDownLatch(1) mainHandler.post(Runnable { val resumedActivity = InternalLeakCanary.resumedActivity diff --git a/leakcanary-android-core/src/main/java/leakcanary/WorkManagerHeapAnalyzer.kt b/leakcanary-android-core/src/main/java/leakcanary/WorkManagerHeapAnalyzer.kt index 8379ee0edd..d8762c92d7 100644 --- a/leakcanary-android-core/src/main/java/leakcanary/WorkManagerHeapAnalyzer.kt +++ b/leakcanary-android-core/src/main/java/leakcanary/WorkManagerHeapAnalyzer.kt @@ -24,8 +24,6 @@ object WorkManagerHeapAnalyzer : EventListener { } } - private val application = InternalLeakCanary.application - // setExpedited() requires WorkManager 2.7.0+ private val workManagerSupportsExpeditedRequests by lazy { try { @@ -49,6 +47,7 @@ object WorkManagerHeapAnalyzer : EventListener { addExpeditedFlag() }.build() SharkLog.d { "Enqueuing heap analysis for ${event.file} on WorkManager remote worker" } + val application = InternalLeakCanary.application WorkManager.getInstance(application).enqueue(heapAnalysisRequest) } } diff --git a/leakcanary-android/build.gradle b/leakcanary-android/build.gradle index 33d6c04445..e00456ba99 100644 --- a/leakcanary-android/build.gradle +++ b/leakcanary-android/build.gradle @@ -10,12 +10,19 @@ dependencies { api project(':leakcanary-object-watcher-android') // Plumber auto installer implementation project(':plumber-android') + + androidTestImplementation libs.androidX.test.espresso + androidTestImplementation libs.androidX.test.rules + androidTestImplementation libs.androidX.test.runner + androidTestImplementation libs.assertjCore + androidTestImplementation project(':shark-hprof-test') } android { compileSdk versions.compileSdk defaultConfig { minSdk versions.minSdk + testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" } buildFeatures.buildConfig = false lintOptions { diff --git a/leakcanary-android/src/androidTest/java/leakcanary/LeakActivityTest.kt b/leakcanary-android/src/androidTest/java/leakcanary/LeakActivityTest.kt new file mode 100644 index 0000000000..7606a506e6 --- /dev/null +++ b/leakcanary-android/src/androidTest/java/leakcanary/LeakActivityTest.kt @@ -0,0 +1,86 @@ +package leakcanary + +import android.app.Activity +import android.content.Intent +import android.net.Uri +import androidx.test.espresso.Espresso.onView +import androidx.test.espresso.action.ViewActions.click +import androidx.test.espresso.assertion.ViewAssertions.matches +import androidx.test.espresso.matcher.ViewMatchers.isDisplayed +import androidx.test.espresso.matcher.ViewMatchers.withText +import androidx.test.rule.ActivityTestRule +import java.io.File +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit.SECONDS +import leakcanary.EventListener.Event.HeapAnalysisDone +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import shark.HprofWriterHelper +import shark.ValueHolder.IntHolder +import shark.dump + +internal class LeakActivityTest { + + @get:Rule + var testFolder = TemporaryFolder() + + @Suppress("UNCHECKED_CAST") + // This class is internal but ActivityTestRule requires being passed in the real class. + private val leakActivityClass = Class.forName("leakcanary.internal.activity.LeakActivity") + as Class + + @get:Rule + val activityTestRule = object : ActivityTestRule(leakActivityClass, false, false) { + override fun getActivityIntent(): Intent { + return LeakCanary.newLeakDisplayActivityIntent() + } + } + + @Test + fun importHeapDumpFile() = tryAndRestoreConfig { + val latch = CountDownLatch(1) + LeakCanary.config = LeakCanary.config.run { + copy(eventListeners = eventListeners + EventListener { event -> + if (event is HeapAnalysisDone<*>) { + latch.countDown() + } + }) + } + val hprof = writeHeapDump { + "Holder" clazz { + staticField["leak"] = "com.example.Leaking" watchedInstance {} + } + } + val intent = Intent(Intent.ACTION_VIEW, Uri.fromFile(hprof)) + activityTestRule.launchActivity(intent) + require(latch.await(5, SECONDS)) + onView(withText("1 Heap Dump")).check(matches(isDisplayed())) + onView(withText("1 Distinct Leak")).perform(click()) + onView(withText("Holder.leak")).check(matches(isDisplayed())) + } + + private fun writeHeapDump(block: HprofWriterHelper.() -> Unit): File { + val hprofFile = testFolder.newFile("temp.hprof") + hprofFile.dump { + "android.os.Build" clazz { + staticField["MANUFACTURER"] = string("Samsing") + } + "android.os.Build\$VERSION" clazz { + staticField["SDK_INT"] = IntHolder(47) + } + block() + } + return hprofFile + } + + private fun tryAndRestoreConfig(block: () -> Unit) { + val original = LeakCanary.config + try { + block() + } finally { + LeakCanary.config = original + } + } + +}