tests for Android profiling (#1949)

Author: stefanosiano

CHANGELOG.md

@@ -2,7 +2,7 @@
 
 ## Unreleased
 
-* Feat: Add Android profiling traces #1897
+* Feat: Add Android profiling traces #1897 and its tests #1949
 
 ## 5.6.2
 

Sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt

@@ -0,0 +1,16 @@
+package io.sentry
+
+import kotlin.test.Test
+import kotlin.test.assertNull
+
+class NoOpTransactionProfilerTest {
+    private var profiler = NoOpTransactionProfiler.getInstance()
+
+    @Test
+    fun `onTransactionStart does not throw`() =
+        profiler.onTransactionStart(NoOpTransaction.getInstance())
+
+    @Test
+    fun `onTransactionFinish returns null`() =
+        assertNull(profiler.onTransactionFinish(NoOpTransaction.getInstance()))
+}

Sentry/src/test/java/io/sentry/util/FileUtilsTest.kt

@@ -0,0 +1,77 @@
+package io.sentry.util
+
+import java.io.File
+import java.nio.file.Files
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertNull
+import kotlin.test.assertTrue
+
+class FileUtilsTest {
+
+    @Test
+    fun `deleteRecursively returns true on null file`() {
+        assertTrue(FileUtils.deleteRecursively(null))
+    }
+
+    @Test
+    fun `deleteRecursively returns true on non-existing file`() {
+        assertTrue(FileUtils.deleteRecursively(File("")))
+    }
+
+    @Test
+    fun `deleteRecursively deletes a simple file`() {
+        val f = Files.createTempFile("here", "test").toFile()
+        assertTrue(f.exists())
+        assertTrue(FileUtils.deleteRecursively(f))
+        assertFalse(f.exists())
+    }
+
+    @Test
+    fun `deleteRecursively deletes a folder`() {
+        // Setup vars
+        val rootDir = Files.createTempDirectory("here").toFile()
+        val rootChild = File(rootDir, "test")
+        val innerDir = File(rootDir, "dir2")
+        val innerChild = File(innerDir, "test")
+
+        // Create directories and files
+        rootChild.createNewFile()
+        innerDir.mkdir()
+        innerChild.createNewFile()
+
+        // Assert dirs and files exist
+        assertTrue(rootDir.exists() && rootDir.isDirectory)
+        assertTrue(rootChild.exists())
+        assertTrue(innerDir.exists() && innerDir.isDirectory)
+        assertTrue(innerChild.exists())
+
+        // Assert deletion returns true
+        assertTrue(FileUtils.deleteRecursively(rootDir))
+
+        // Assert dirs and files no longer exist
+        assertFalse(rootChild.exists())
+        assertFalse(rootDir.exists())
+        assertFalse(innerChild.exists())
+        assertFalse(innerDir.exists())
+    }
+
+    @Test
+    fun `readText returns null on null, non existing or unreadable file`() {
+        val f = File("here", "test")
+        val unreadableFile = Files.createTempFile("here", "test").toFile()
+        unreadableFile.setReadable(false)
+        assertNull(FileUtils.readText(null))
+        assertNull(FileUtils.readText(f))
+        assertNull(FileUtils.readText(unreadableFile))
+    }
+
+    @Test
+    fun `readText returns the content of a file`() {
+        val f = Files.createTempFile("here", "test").toFile()
+        val text = "Lorem ipsum dolor sit amet\nLorem ipsum dolor sit amet"
+        f.writeText(text)
+        assertEquals(text, FileUtils.readText(f))
+    }
+}

sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java

@@ -21,6 +21,7 @@
 import java.util.concurrent.Future;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.annotations.TestOnly;
 
 final class AndroidTransactionProfiler implements ITransactionProfiler {
 
@@ -59,6 +60,10 @@ public AndroidTransactionProfiler(
         Objects.requireNonNull(buildInfoProvider, "The BuildInfoProvider is required.");
     this.packageInfo = ContextUtils.getPackageInfo(context, options.getLogger());
     final String tracesFilesDirPath = options.getProfilingTracesDirPath();
+    if (!options.isProfilingEnabled()) {
+      options.getLogger().log(SentryLevel.INFO, "Profiling is disabled in options.");
+      return;
+    }
     if (tracesFilesDirPath == null || tracesFilesDirPath.isEmpty()) {
       options
           .getLogger()
@@ -90,7 +95,7 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
 
     // traceFilesDir is null or intervalUs is 0 only if there was a problem in the constructor, but
     // we already logged that
-    if (traceFilesDir == null || intervalUs == 0) {
+    if (traceFilesDir == null || intervalUs == 0 || !traceFilesDir.exists()) {
       return;
     }
 
@@ -225,7 +230,7 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
         buildInfoProvider.getModel(),
         buildInfoProvider.getVersionRelease(),
         buildInfoProvider.isEmulator(),
-        CpuInfoUtils.readMaxFrequencies(),
+        CpuInfoUtils.getInstance().readMaxFrequencies(),
         totalMem,
         options.getProguardUuid(),
         versionName,
@@ -253,4 +258,9 @@ public synchronized void onTransactionStart(@NotNull ITransaction transaction) {
       return null;
     }
   }
+
+  @TestOnly
+  void setTimedOutProfilingData(@Nullable ProfilingTraceData data) {
+    this.timedOutProfilingData = data;
+  }
 }

sentry-android-core/src/main/java/io/sentry/android/core/internal/util/CpuInfoUtils.java

@@ -7,26 +7,38 @@
 import java.util.List;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.TestOnly;
+import org.jetbrains.annotations.VisibleForTesting;
 
 @ApiStatus.Internal
 public final class CpuInfoUtils {
 
-  private static final @NotNull String SYSTEM_CPU_PATH = "/sys/devices/system/cpu/";
-  private static final @NotNull String CPUINFO_MAX_FREQ_PATH = "cpufreq/cpuinfo_max_freq";
+  private static final CpuInfoUtils instance = new CpuInfoUtils();
+
+  public static CpuInfoUtils getInstance() {
+    return instance;
+  }
+
+  private CpuInfoUtils() {}
+
+  private static final @NotNull String SYSTEM_CPU_PATH = "/sys/devices/system/cpu";
+
+  @VisibleForTesting
+  static final @NotNull String CPUINFO_MAX_FREQ_PATH = "cpufreq/cpuinfo_max_freq";
 
   /** Cached max frequencies to avoid reading files multiple times */
-  private static @NotNull List<String> cpuMaxFrequenciesMhz = new ArrayList<>();
+  private final @NotNull List<Integer> cpuMaxFrequenciesMhz = new ArrayList<>();
 
   /**
    * Read the max frequency of each core of the cpu and returns it in Mhz
    *
    * @return A list with the frequency of each core of the cpu in Mhz
    */
-  public static @NotNull List<String> readMaxFrequencies() {
+  public @NotNull List<Integer> readMaxFrequencies() {
     if (!cpuMaxFrequenciesMhz.isEmpty()) {
       return cpuMaxFrequenciesMhz;
     }
-    File[] cpuDirs = new File(SYSTEM_CPU_PATH).listFiles();
+    File[] cpuDirs = new File(getSystemCpuPath()).listFiles();
     if (cpuDirs == null) {
       return new ArrayList<>();
     }
@@ -37,7 +49,7 @@ public final class CpuInfoUtils {
 
       if (!cpuMaxFreqFile.exists() || !cpuMaxFreqFile.canRead()) continue;
 
-      long khz = 0;
+      long khz;
       try {
         String content = FileUtils.readText(cpuMaxFreqFile);
         if (content == null) continue;
@@ -47,8 +59,19 @@ public final class CpuInfoUtils {
       } catch (IOException e) {
         continue;
       }
-      cpuMaxFrequenciesMhz.add(Long.toString(khz / 1000));
+      cpuMaxFrequenciesMhz.add((int) (khz / 1000));
     }
     return cpuMaxFrequenciesMhz;
   }
+
+  @VisibleForTesting
+  @NotNull
+  String getSystemCpuPath() {
+    return SYSTEM_CPU_PATH;
+  }
+
+  @TestOnly
+  final void clear() {
+    cpuMaxFrequenciesMhz.clear();
+  }
 }

sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt

@@ -143,6 +143,29 @@ class AndroidOptionsInitializerTest {
         )
     }
 
+    @Test
+    fun `profilingTracesDirPath should be set at initialization`() {
+        val sentryOptions = SentryAndroidOptions()
+        val mockContext = createMockContext()
+
+        AndroidOptionsInitializer.init(sentryOptions, mockContext)
+
+        assertTrue(sentryOptions.profilingTracesDirPath?.endsWith("${File.separator}cache${File.separator}sentry${File.separator}profiling_traces")!!)
+        assertFalse(File(sentryOptions.profilingTracesDirPath!!).exists())
+    }
+
+    @Test
+    fun `profilingTracesDirPath should be created and cleared when profiling is enabled`() {
+        val sentryOptions = SentryAndroidOptions()
+        val mockContext = createMockContext()
+        sentryOptions.isProfilingEnabled = true
+
+        AndroidOptionsInitializer.init(sentryOptions, mockContext)
+
+        assertTrue(File(sentryOptions.profilingTracesDirPath!!).exists())
+        assertTrue(File(sentryOptions.profilingTracesDirPath!!).list()!!.isEmpty())
+    }
+
     @Test
     fun `outboxDir should be set at initialization`() {
         fixture.initSut()
@@ -218,6 +241,17 @@ class AndroidOptionsInitializerTest {
         assertTrue(fixture.sentryOptions.transportGate is AndroidTransportGate)
     }
 
+    @Test
+    fun `init should set Android transaction profiler`() {
+        val sentryOptions = SentryAndroidOptions()
+        val mockContext = createMockContext()
+
+        AndroidOptionsInitializer.init(sentryOptions, mockContext)
+
+        assertNotNull(sentryOptions.transactionProfiler)
+        assertTrue(sentryOptions.transactionProfiler is AndroidTransactionProfiler)
+    }
+
     @Test
     fun `NdkIntegration will load SentryNdk class and add to the integration list`() {
         fixture.initSutWithClassLoader(classToLoad = SentryNdk::class.java)