Pass -PsnapshotFile for parallel tests

Author: vkryl

scripts/check-test-tools.sh

@@ -9,8 +9,10 @@
 # and device serial are replaced. Report notes when connection to the device was lost,
 # which fails tests regardless of code.
 #
-# Usage: scripts/check-test-tools.sh [-y] [checks] [benchmark] [minsdk]
-# Without steps runs all of them, -y skips confirmation.
+# Usage: scripts/check-test-tools.sh [-y] [-PsnapshotFile=<path>] [checks] [benchmark] [minsdk]
+# Without steps runs all of them, -y skips confirmation. -PsnapshotFile is the snapshot of accounts
+# of the device, see "Session snapshot" in tests/README.md: without it, tests with accounts are
+# skipped.
 
 # Keeps computer awake during long builds and tests
 if [ -z "$TGX_AWAKE" ]; then
@@ -33,9 +35,11 @@ fail() {
 
 ASSUME_YES=false
 STEPS=""
+SNAPSHOT_FILE=""
 for arg in "$@"; do
   case $arg in
     -y) ASSUME_YES=true ;;
+    -PsnapshotFile=*) SNAPSHOT_FILE=${arg#-PsnapshotFile=} ;;
     checks|benchmark|minsdk) STEPS="${STEPS:+$STEPS }$arg" ;;
     *) fail "Unknown argument: $arg. Steps: checks, benchmark, minsdk" ;;
   esac
@@ -71,7 +75,8 @@ anonymize() {
 }
 
 gradle() {
-  ./gradlew --console=plain --continue -Ptgx.testTools -Ptgx.checks=false "$@"
+  ./gradlew --console=plain --continue -Ptgx.testTools -Ptgx.checks=false \
+    ${SNAPSHOT_FILE:+"-PsnapshotFile=$SNAPSHOT_FILE"} "$@"
 }
 
 run() {
@@ -237,7 +242,7 @@ cleanup() {
 FLAVOR=Latest
 ABI=arm64
 HAS_SNAPSHOT=false
-if [ -f tests/core/src/main/assets/snapshot.zip ]; then
+if [ -n "$SNAPSHOT_FILE" ]; then
   HAS_SNAPSHOT=true
 fi
 NEEDS_DEVICE=false
@@ -316,7 +321,7 @@ fi
 echo "Steps: $STEPS"
 if $NEEDS_DEVICE; then
   echo "Device: $MODEL, Android $RELEASE (API $SDK_INT), $ABI, flavor $FLAVOR," \
-    "app.id $APP_ID, snapshot.zip: $HAS_SNAPSHOT"
+    "app.id $APP_ID, snapshot: $HAS_SNAPSHOT"
   echo -e "${STYLE_WARN}The script:
 - installs builds of $APP_ID and test apps on $MODEL, clears their data
   and removes them after tests, also when the script is interrupted;
@@ -365,7 +370,7 @@ trap 'exit 130' INT TERM
   )"
   if $NEEDS_DEVICE; then
     echo "- Device: $MODEL, Android $RELEASE (API $SDK_INT), $ABI, flavor $FLAVOR," \
-      "snapshot.zip: $HAS_SNAPSHOT"
+      "snapshot: $HAS_SNAPSHOT"
     echo "- Animation scales before tests: $(animation_scales)"
   fi
 } > "$OUT/header.md"

tests/README.md

@@ -556,13 +556,17 @@ Tests on device replace installed app and clear its data, so they must never run
 
 ## Test accounts
 
-* Tests and checks run on production servers of Telegram with real test accounts. Messages can be sent only to **Saved Messages** or to the users authorized in `snapshot.zip`, and groups and channels created by them can include only these users. Their ids are returned by `getSnapshotUserIds()`: when there is only one, messages are sent only to **Saved Messages**.
+* Tests and checks run on production servers of Telegram with real test accounts. Messages can be sent only to **Saved Messages** or to the users authorized in the [snapshot](#session-snapshot), and groups and channels created by them can include only these users. Their ids are returned by `getSnapshotUserIds()`: when there is only one, messages are sent only to **Saved Messages**.
 * Be careful with TDLib requests that load data over network: don't send them in loops or more often than a user could. Requests answered locally are not limited.
 
 ### Session snapshot
 
-Authorized tests restore sessions from `tests/core/src/main/assets/snapshot.zip`. It is ignored by git and must never be committed: it gives access to the accounts. Use dedicated test accounts. Snapshot includes all accounts added to the app, so several accounts can be authorized to test interaction between them: messages, groups, channels, notifications.
+Authorized tests restore sessions from a snapshot of accounts, a zip file outside the clone that the build takes by its full path: `-PsnapshotFile=<path>`, e.g. `./gradlew :tests:ui:connectedLatestNonMinifiedReleaseAndroidTest -PsnapshotFile=$HOME/tgx-snapshots/pixel.zip`. Without it, authorized tests fail. The snapshot gives access to the accounts: keep it out of repositories and don't make copies of it. Use dedicated test accounts. Snapshot includes all accounts added to the app, so several accounts can be authorized to test interaction between them: messages, groups, channels, notifications.
 
-Sessions from the snapshot must not be used by several apps at once, otherwise Telegram may terminate them as leaked. That's why build runs tests on device one at a time and only on one device, and fails when another Gradle build on the machine is already running tests with the same accounts. Builds with snapshots of other accounts can run tests at the same time.
+The build adds the snapshot to assets of test APKs by a link and keeps their assets out of the build cache, so that the sessions stay only in the snapshot and in the build folders of the clone. `tests/core/src/main/assets/snapshot.zip`, where the snapshot was kept before, is no longer read: a build that finds it there fails.
 
-To create the snapshot, open **Saved Messages** of the test account → attach menu → **Files** → **Session Snapshot** (available in builds with test hooks enabled or in developer mode). The snapshot is sent to the chat: download it and save as `tests/core/src/main/assets/snapshot.zip`. Then clear data of the app where the snapshot was created, so it stops using the same sessions. Don't log out instead: it terminates sessions in the snapshot too.
+Sessions from the snapshot must not be used by several apps at once, otherwise Telegram may terminate them as leaked. Each device can have its own snapshot of the same accounts: a snapshot made in another app has other sessions, so devices run tests at the same time, each with its own snapshot. A build runs tests on one device only, and waits while another Gradle build on the machine runs tests on the same device or with the same snapshot, found by its path.
+
+A script that runs builds one at a time on the machine can let others build while tests run on a device: when it holds a lock of builds, a folder, it names it by `TGX_BUILD_LOCK` and its process by `TGX_BUILD_LOCK_PID`, written into `pid` in that folder. Tests on a device build nothing, so they remove the folder when they start, if it still has that `pid`.
+
+To create the snapshot, open **Saved Messages** of the test account → attach menu → **Files** → **Session Snapshot** (available in builds with test hooks enabled or in developer mode). The snapshot is sent to the chat: download it and save it outside the clone. Then clear data of the app where the snapshot was created, so it stops using the same sessions. Don't log out instead: it terminates sessions in the snapshot too.

tests/buildSrc/kotlin/tgx/test/AppTests.kt

@@ -10,10 +10,13 @@ import com.android.build.api.variant.AndroidComponentsExtension
 import com.android.build.api.variant.ApplicationAndroidComponentsExtension
 import com.android.build.api.variant.TestAndroidComponentsExtension
 import org.gradle.api.Project
+import org.gradle.api.file.RegularFile
+import org.gradle.api.provider.Provider
 import org.gradle.kotlin.dsl.configure
 import org.gradle.kotlin.dsl.get
 import org.gradle.kotlin.dsl.of
 import org.gradle.kotlin.dsl.register
+import tgx.gradle.fatal
 import tgx.gradle.source.KeystoreSource
 import java.io.File
 import java.util.Properties
@@ -25,7 +28,21 @@ private const val TEST_MIN_SDK_VERSION = 24
 
 const val TEST_DEVICE = "emulator"
 
-internal const val SNAPSHOT_PATH = "tests/core/src/main/assets/snapshot.zip"
+// Where the snapshot of accounts was kept before -PsnapshotFile, inside the clone
+private const val OLD_SNAPSHOT_PATH = "tests/core/src/main/assets/snapshot.zip"
+
+// Snapshot of accounts that tests start with, outside the clone: -PsnapshotFile=<full path>,
+// see "Session snapshot" in tests/README.md
+internal val Project.snapshotFile: Provider<RegularFile>
+  get() = layout.file(
+    providers.gradleProperty("snapshotFile").map { path ->
+      File(path).also {
+        if (!it.isAbsolute || !it.isFile) {
+          fatal("-PsnapshotFile=$path isn't the full path of a snapshot")
+        }
+      }
+    }
+  )
 
 // -Ptgx.checks=false builds tests without checks of a change, e.g. to measure a commit before it,
 // where they don't compile
@@ -122,11 +139,20 @@ fun Project.configureAppTests(
   inAppProcess: Boolean = false,
   testedVariants: (SdkVariant) -> Boolean = { it.supportsSdk(TEST_MIN_SDK_VERSION) }
 ) {
-  val snapshot = isolated.rootProject.projectDirectory.file(SNAPSHOT_PATH)
+  if (isolated.rootProject.projectDirectory.file(OLD_SNAPSHOT_PATH).asFile.exists()) {
+    fatal(
+      "$OLD_SNAPSHOT_PATH is no longer read: move it out of the clone " +
+        "and pass its full path by -PsnapshotFile"
+    )
+  }
+  val snapshot = snapshotFile
   val snapshotUserIds = providers.fileContents(snapshot).asBytes
     .map { readSnapshotUserIds(it.inputStream()) }
     .orNull
     .orEmpty()
+  val snapshotAsset = tasks.register<SnapshotAssetTask>("linkSnapshotAsset") {
+    this.snapshot.set(snapshot)
+  }
   val appKeystore = if (inAppProcess) findAppKeystore() else null
 
   extensions.configure<TestExtension> {
@@ -202,8 +228,20 @@ fun Project.configureAppTests(
           snapshotUserIds.joinToString(",")
         )
       }
+      if (snapshot.isPresent) {
+        variant.sources.assets?.addGeneratedSourceDirectory(
+          snapshotAsset,
+          SnapshotAssetTask::outputDirectory
+        )
+      }
     }
   }
+  // The build cache would keep copies of the snapshot outside the clone
+  tasks
+    .named { (it.startsWith("merge") || it.startsWith("compress")) && it.endsWith("Assets") }
+    .configureEach {
+      outputs.doNotCacheIf("Assets of tests have the snapshot of accounts") { true }
+    }
 
   tasks.register<DescribeHeapProfilesTask>("describeHeapProfiles") {
     mustRunAfter(tasks.named { it.startsWith("connected") && it.endsWith("AndroidTest") })
@@ -244,8 +282,8 @@ private fun Project.findAppKeystore(): Keystore? {
 }
 
 /**
- * Tests of different modules install the same app,
- * and accounts from snapshot.zip must not be used by several apps at once
+ * Tests of different modules install the same app, a phone runs tests of one build at a time,
+ * and sessions of a snapshot must not be used by several apps at once
  */
 @Suppress("UnstableApiUsage")
 private fun Project.runTestsOnDeviceOneAtATime() {
@@ -254,7 +292,7 @@ private fun Project.runTestsOnDeviceOneAtATime() {
     SnapshotLockService::class.java
   ) {
     maxParallelUsages.set(1)
-    parameters.snapshot.set(isolated.rootProject.projectDirectory.file(SNAPSHOT_PATH))
+    parameters.snapshot.set(snapshotFile.orNull)
     parameters.locksDirectory.set(File(gradle.gradleUserHomeDir, "tgx-test-accounts"))
   }
   val adb = extensions.getByType(AndroidComponentsExtension::class.java).sdkComponents.adb
@@ -274,10 +312,10 @@ private fun Project.runTestsOnDeviceOneAtATime() {
   deviceTests.configureEach {
     usesService(snapshotLock)
     doFirst {
-      snapshotLock.get().lock()
-      if (name.startsWith("connected")) {
-        requireSingleDevice(adb.get().asFile)
-      }
+      // Tests on a phone build nothing, so other builds go on while they run
+      releaseBuildLock()
+      val serial = if (name.startsWith("connected")) requireSingleDevice(adb.get().asFile) else null
+      snapshotLock.get().lock(serial)
     }
   }
 }

tests/buildSrc/kotlin/tgx/test/SnapshotAssetTask.kt

@@ -0,0 +1,40 @@
+package tgx.test
+
+import org.gradle.api.DefaultTask
+import org.gradle.api.file.DirectoryProperty
+import org.gradle.api.file.RegularFileProperty
+import org.gradle.api.tasks.InputFile
+import org.gradle.api.tasks.OutputDirectory
+import org.gradle.api.tasks.PathSensitive
+import org.gradle.api.tasks.PathSensitivity
+import org.gradle.api.tasks.TaskAction
+import org.gradle.work.DisableCachingByDefault
+import java.nio.file.Files
+
+// Name of the snapshot in assets of tests, see SnapshotApplier
+private const val SNAPSHOT_ASSET = "snapshot.zip"
+
+/**
+ * Adds the snapshot of -PsnapshotFile to assets of tests by a link, not a copy, so that the
+ * sessions stay in one file outside the clone until assets are packaged
+ */
+@DisableCachingByDefault(because = "The build cache would keep the snapshot outside the clone")
+abstract class SnapshotAssetTask : DefaultTask() {
+  @get:InputFile
+  @get:PathSensitive(PathSensitivity.NONE)
+  abstract val snapshot: RegularFileProperty
+
+  @get:OutputDirectory
+  abstract val outputDirectory: DirectoryProperty
+
+  @TaskAction
+  fun link() {
+    val directory = outputDirectory.get().asFile
+    directory.deleteRecursively()
+    directory.mkdirs()
+    Files.createSymbolicLink(
+      directory.resolve(SNAPSHOT_ASSET).toPath(),
+      snapshot.get().asFile.toPath()
+    )
+  }
+}

tests/buildSrc/kotlin/tgx/test/SnapshotLockService.kt

@@ -2,6 +2,7 @@ package tgx.test
 
 import org.gradle.api.file.DirectoryProperty
 import org.gradle.api.file.RegularFileProperty
+import org.gradle.api.logging.Logging
 import org.gradle.api.services.BuildService
 import org.gradle.api.services.BuildServiceParameters
 import tgx.gradle.fatal
@@ -10,13 +11,18 @@ import java.io.File
 import java.io.InputStream
 import java.nio.channels.FileChannel
 import java.nio.file.StandardOpenOption
+import java.security.MessageDigest
 import java.util.zip.ZipInputStream
 
 private const val SNAPSHOT_REUSE_ERROR =
-  "Tests use accounts from snapshot.zip, which must not be used by several apps at once, " +
-  "otherwise Telegram may terminate their sessions as leaked."
+  "Tests use sessions of the snapshot of -PsnapshotFile, which must not be used by several apps " +
+  "at once, otherwise Telegram may terminate them as leaked."
 
-/** Builds keep locks of accounts until they finish */
+/**
+ * Builds keep locks of their phone and of the sessions of their snapshot until they finish.
+ * A build waits while another one runs tests on the same phone or with the same snapshot:
+ * snapshots of one account in other sessions run at the same time
+ */
 abstract class SnapshotLockService : BuildService<SnapshotLockService.Parameters>, AutoCloseable {
   interface Parameters : BuildServiceParameters {
     val snapshot: RegularFileProperty
@@ -27,34 +33,38 @@ abstract class SnapshotLockService : BuildService<SnapshotLockService.Parameters
   private var isLocked = false
 
   @Synchronized
-  fun lock() {
+  fun lock(serial: String?) {
+    // serial is the phone of connected tests, null for a managed device
     if (isLocked) {
       return
     }
-    val snapshot = parameters.snapshot.get().asFile
-    val userIds = if (snapshot.exists()) {
-      snapshot.inputStream().use(::readSnapshotUserIds)
-    } else {
-      emptyList()
-    }
     val directory = parameters.locksDirectory.get().asFile
     directory.mkdirs()
-    for (userId in userIds) {
-      val channel = FileChannel.open(
-        File(directory, "$userId.lock").toPath(),
-        StandardOpenOption.CREATE,
-        StandardOpenOption.WRITE
-      )
-      if (channel.tryLock() == null) {
-        channel.close()
-        close()
-        fatal("Another Gradle build is running tests with account $userId. $SNAPSHOT_REUSE_ERROR")
-      }
-      channels += channel
+    // The phone first, so that a build waiting for a snapshot keeps its phone, not another one
+    if (serial != null) {
+      hold(File(directory, "phone-$serial.lock"), "phone $serial")
+    }
+    parameters.snapshot.orNull?.asFile?.let { snapshot ->
+      // A session is never copied, so the path of its snapshot tells it
+      val digest =
+        MessageDigest.getInstance("SHA-256").digest(snapshot.canonicalPath.toByteArray())
+      val name = digest.take(8).joinToString("") { "%02x".format(it) }
+      hold(File(directory, "session-$name.lock"), "sessions of ${snapshot.path}")
     }
     isLocked = true
   }
 
+  private fun hold(file: File, what: String) {
+    val channel =
+      FileChannel.open(file.toPath(), StandardOpenOption.CREATE, StandardOpenOption.WRITE)
+    if (channel.tryLock() == null) {
+      Logging.getLogger(SnapshotLockService::class.java)
+        .lifecycle("Waiting for another Gradle build that runs tests with $what")
+      channel.lock()
+    }
+    channels += channel
+  }
+
   override fun close() {
     // Closing channels releases their locks
     channels.forEach {
@@ -64,6 +74,17 @@ abstract class SnapshotLockService : BuildService<SnapshotLockService.Parameters
   }
 }
 
+internal fun releaseBuildLock() {
+  // Gives back the lock of builds of a script that runs builds one at a time, see TGX_BUILD_LOCK in
+  // tests/README.md, unless another script took it again
+  val directory = System.getenv("TGX_BUILD_LOCK")?.let(::File) ?: return
+  val pid = System.getenv("TGX_BUILD_LOCK_PID") ?: return
+  val owner = File(directory, "pid").takeIf { it.isFile }?.readText()?.trim()
+  if (owner == pid) {
+    directory.deleteRecursively()
+  }
+}
+
 // See TdlibAccount.FLAG_UNAUTHORIZED
 private const val ACCOUNT_FLAG_UNAUTHORIZED = 1
 
@@ -79,7 +100,7 @@ internal fun readSnapshotUserIds(snapshot: InputStream): List<Long> {
     it.name == "tdlib_accounts.bin"
   }
   if (!hasAccounts) {
-    fatal("tdlib_accounts.bin is missing in snapshot.zip")
+    fatal("tdlib_accounts.bin is missing in the snapshot")
   }
   val input = DataInputStream(zip)
   val accountCount = input.readInt()
@@ -95,7 +116,8 @@ internal fun readSnapshotUserIds(snapshot: InputStream): List<Long> {
   }
 }
 
-internal fun requireSingleDevice(adb: File) {
+internal fun requireSingleDevice(adb: File): String? {
+  // Returns the serial of the device that connected tests run on, null when none is connected
   val selectedDevices = System.getenv("ANDROID_SERIAL")
     ?.split(',')
     ?.filter { it.isNotBlank() }
@@ -114,4 +136,5 @@ internal fun requireSingleDevice(adb: File) {
         "$SNAPSHOT_REUSE_ERROR Select one device with ANDROID_SERIAL."
     )
   }
+  return devices.singleOrNull()
 }