rename and improve zip approach in success case

Author: pyricau

leakcanary/leakcanary-jvm-test/src/test/java/leakcanary/JvmLiveObjectGrowthDetectorTest.kt

@@ -249,12 +249,12 @@ class JvmLiveObjectGrowthDetectorTest {
   }
 
   @Test
-  fun `DeleteOnObjectsNotGrowing does invokes file deletion in between each scenario`() {
+  fun `KeepHeapDumpsOnObjectsGrowing does invokes file deletion in between each scenario`() {
     var didDeleteFile = false
     val heapDumpDirectory = tempFolder.newFolder()
     val detector = HeapDiff.repeatingJvmInProcessScenario(
       objectGrowthDetector = ObjectGrowthDetector.forJvmHeapNoSyntheticRefs(),
-      heapDumpDeletionStrategy = HeapDumpDeletionStrategy.DeleteOnObjectsNotGrowing {
+      heapDumpDeletionStrategy = HeapDumpDeletionStrategy.KeepHeapDumpsOnObjectsGrowing {
         didDeleteFile = true
         it.delete()
       },
@@ -271,12 +271,12 @@ class JvmLiveObjectGrowthDetectorTest {
   }
 
   @Test
-  fun `DeleteOnObjectsNotGrowing does not delete any heap dump if objects growing`() {
+  fun `KeepHeapDumpsOnObjectsGrowing does not delete any heap dump if objects growing`() {
     val maxHeapDump = 5
     val heapDumpDirectory = tempFolder.newFolder()
     val detector = HeapDiff.repeatingJvmInProcessScenario(
       objectGrowthDetector = ObjectGrowthDetector.forJvmHeapNoSyntheticRefs(),
-      heapDumpDeletionStrategy = HeapDumpDeletionStrategy.DeleteOnObjectsNotGrowing(),
+      heapDumpDeletionStrategy = HeapDumpDeletionStrategy.KeepHeapDumpsOnObjectsGrowing(),
       heapDumpDirectoryProvider = { heapDumpDirectory }
     )
 
@@ -292,12 +292,12 @@ class JvmLiveObjectGrowthDetectorTest {
   }
 
   @Test
-  fun `DeleteOnObjectsNotGrowing invokes file deletion on completion if objects not growing`() {
+  fun `KeepHeapDumpsOnObjectsGrowing invokes file deletion on completion if objects not growing`() {
     var filesDeleted = 0
     val heapDumpDirectory = tempFolder.newFolder()
     val detector = HeapDiff.repeatingJvmInProcessScenario(
       objectGrowthDetector = ObjectGrowthDetector.forJvmHeapNoSyntheticRefs(),
-      heapDumpDeletionStrategy = HeapDumpDeletionStrategy.DeleteOnObjectsNotGrowing {
+      heapDumpDeletionStrategy = HeapDumpDeletionStrategy.KeepHeapDumpsOnObjectsGrowing {
         filesDeleted++
         it.delete()
       },
@@ -321,12 +321,12 @@ class JvmLiveObjectGrowthDetectorTest {
   }
 
   @Test
-  fun `ZipAndDeleteOnObjectsNotGrowing leaves zipped heap dumps if objects growing`() {
+  fun `KeepZippedHeapDumpsOnObjectsGrowing leaves zipped heap dumps if objects growing`() {
     val maxHeapDump = 5
     val heapDumpDirectory = tempFolder.newFolder()
     val detector = HeapDiff.repeatingJvmInProcessScenario(
       objectGrowthDetector = ObjectGrowthDetector.forJvmHeapNoSyntheticRefs(),
-      heapDumpDeletionStrategy = HeapDumpDeletionStrategy.ZipAndDeleteOnObjectsNotGrowing(),
+      heapDumpDeletionStrategy = HeapDumpDeletionStrategy.KeepZippedHeapDumpsOnObjectsGrowing(),
       heapDumpDirectoryProvider = { heapDumpDirectory }
     )
 
@@ -343,11 +343,11 @@ class JvmLiveObjectGrowthDetectorTest {
   }
 
   @Test
-  fun `ZipAndDeleteOnObjectsNotGrowing deletes all files if objects not growing`() {
+  fun `KeepZippedHeapDumpsOnObjectsGrowing deletes all files if objects not growing`() {
     val heapDumpDirectory = tempFolder.newFolder()
     val detector = HeapDiff.repeatingJvmInProcessScenario(
       objectGrowthDetector = ObjectGrowthDetector.forJvmHeapNoSyntheticRefs(),
-      heapDumpDeletionStrategy = HeapDumpDeletionStrategy.ZipAndDeleteOnObjectsNotGrowing(),
+      heapDumpDeletionStrategy = HeapDumpDeletionStrategy.KeepZippedHeapDumpsOnObjectsGrowing(),
       heapDumpDirectoryProvider = { heapDumpDirectory }
     )
     val leakyScenarioRuns = 3
@@ -366,35 +366,6 @@ class JvmLiveObjectGrowthDetectorTest {
     assertThat(heapDumpDirectory.listFiles()).isEmpty()
   }
 
-  @Test
-  fun `ZipAndDeleteOnObjectsNotGrowing zips heap dumps in between each scenario`() {
-    val heapDumpDirectory = tempFolder.newFolder()
-    val maxHeapDump = 5
-
-    val detector = HeapDiff.repeatingJvmInProcessScenario(
-      objectGrowthDetector = ObjectGrowthDetector.forJvmHeapNoSyntheticRefs(),
-      heapDumpDeletionStrategy = HeapDumpDeletionStrategy.ZipAndDeleteOnObjectsNotGrowing(),
-      heapDumpDirectoryProvider = { heapDumpDirectory }
-    )
-    var i = 1
-    detector.findRepeatedlyGrowingObjects(
-      maxHeapDumps = maxHeapDump,
-      scenarioLoopsPerDump = 1
-    ) {
-      val heapDumpDirectoryFileExtensions = heapDumpDirectory.listFiles()!!.map { it.extension }
-      assertThat(heapDumpDirectoryFileExtensions).hasSize(i - 1)
-      if (i > 1) {
-        assertThat(heapDumpDirectoryFileExtensions).containsOnly("zip")
-      }
-      leakies += Any()
-      i++
-    }
-
-    val heapDumpDirectoryFileExtensions = heapDumpDirectory.listFiles()!!.map { it.extension }
-    assertThat(heapDumpDirectoryFileExtensions).hasSize(maxHeapDump)
-    assertThat(heapDumpDirectoryFileExtensions).containsOnly("zip")
-  }
-
   private fun ObjectGrowthDetector.Companion.forJvmHeapNoSyntheticRefs(): ObjectGrowthDetector {
     val referenceMatchers = JvmObjectGrowthReferenceMatchers.defaults
     return ObjectGrowthDetector(

leakcanary/leakcanary-test-core/api/leakcanary-test-core.api

@@ -16,21 +16,21 @@ public final class leakcanary/HeapDumpDeletionStrategy$DeleteOnHeapDumpClose : l
 	public fun onObjectGrowthDetectionComplete (Lshark/HeapDiff;)V
 }
 
-public final class leakcanary/HeapDumpDeletionStrategy$DeleteOnObjectsNotGrowing : leakcanary/HeapDumpDeletionStrategy {
-	public fun <init> ()V
-	public fun <init> (Lkotlin/jvm/functions/Function1;)V
-	public synthetic fun <init> (Lkotlin/jvm/functions/Function1;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
+public final class leakcanary/HeapDumpDeletionStrategy$KeepHeapDumps : leakcanary/HeapDumpDeletionStrategy {
+	public static final field INSTANCE Lleakcanary/HeapDumpDeletionStrategy$KeepHeapDumps;
 	public fun onHeapDumpClosed (Ljava/io/File;)V
 	public fun onObjectGrowthDetectionComplete (Lshark/HeapDiff;)V
 }
 
-public final class leakcanary/HeapDumpDeletionStrategy$KeepHeapDumps : leakcanary/HeapDumpDeletionStrategy {
-	public static final field INSTANCE Lleakcanary/HeapDumpDeletionStrategy$KeepHeapDumps;
+public final class leakcanary/HeapDumpDeletionStrategy$KeepHeapDumpsOnObjectsGrowing : leakcanary/HeapDumpDeletionStrategy {
+	public fun <init> ()V
+	public fun <init> (Lkotlin/jvm/functions/Function1;)V
+	public synthetic fun <init> (Lkotlin/jvm/functions/Function1;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public fun onHeapDumpClosed (Ljava/io/File;)V
 	public fun onObjectGrowthDetectionComplete (Lshark/HeapDiff;)V
 }
 
-public final class leakcanary/HeapDumpDeletionStrategy$ZipAndDeleteOnObjectsNotGrowing : leakcanary/HeapDumpDeletionStrategy {
+public final class leakcanary/HeapDumpDeletionStrategy$KeepZippedHeapDumpsOnObjectsGrowing : leakcanary/HeapDumpDeletionStrategy {
 	public fun <init> ()V
 	public fun <init> (Lkotlin/jvm/functions/Function1;)V
 	public synthetic fun <init> (Lkotlin/jvm/functions/Function1;ILkotlin/jvm/internal/DefaultConstructorMarker;)V

leakcanary/leakcanary-test-core/src/main/java/leakcanary/HeapDumpDeletionStrategy.kt

@@ -32,7 +32,7 @@ interface HeapDumpDeletionStrategy : DumpingHeapGraphProvider.HeapDumpClosedList
    * objects. This is useful if you intend to open up the heap dumps directly or re run
    * the analysis on failure.
    */
-  class DeleteOnObjectsNotGrowing(
+  class KeepHeapDumpsOnObjectsGrowing(
     private val deleteFile: (File) -> Unit = { it.delete() }
   ) : HeapDumpDeletionStrategy {
     // This assumes the detector instance is always used from the same thread, which seems like a
@@ -54,31 +54,33 @@ interface HeapDumpDeletionStrategy : DumpingHeapGraphProvider.HeapDumpClosedList
   }
 
   /**
-   * Zips the heap dumps until we're done diffing, then delete them only if there are no growing
-   * objects. This is useful if you intend to upload the heap dumps on failure in CI and you
+   * Keeps the heap dumps until we're done diffing, then on completion creates a zip for each heap
+   * dump if there are growing object, and delete all the source heap dumps.
+   * This is useful if you intend to upload the heap dumps on failure in CI and you
    * want to keep disk space, network usage and cloud storage low. Zipped heap dumps are typically
    * 4x smaller so this is worth it, although the trade off is that zipping can add a few seconds
    * per heap dump to the runtime duration of a test.
    */
-  class ZipAndDeleteOnObjectsNotGrowing(
+  class KeepZippedHeapDumpsOnObjectsGrowing(
     private val deleteFile: (File) -> Unit = { it.delete() }
   ) : HeapDumpDeletionStrategy {
     // This assumes the detector instance is always used from the same thread, which seems like a
     // safe enough assumption for tests.
     private val closedHeapDumpFiles = mutableListOf<File>()
 
     override fun onHeapDumpClosed(heapDumpFile: File) {
-      val zippedHeapDumpFile = heapDumpFile.zipFile()
-      deleteFile(heapDumpFile)
-      closedHeapDumpFiles += zippedHeapDumpFile
+      closedHeapDumpFiles += heapDumpFile
     }
 
     override fun onObjectGrowthDetectionComplete(result: HeapDiff) {
-      if (!result.isGrowing) {
+      if (result.isGrowing) {
         closedHeapDumpFiles.forEach {
-          deleteFile(it)
+          it.zipFile()
         }
       }
+      closedHeapDumpFiles.forEach {
+        deleteFile(it)
+      }
       closedHeapDumpFiles.clear()
     }