From d81fc39f2503ee5e2664c22bfd88bf9aaf5fa593 Mon Sep 17 00:00:00 2001 From: m57 <41645758+Hy4ri@users.noreply.github.com> Date: Mon, 28 Sep 2026 00:20:52 +0300 Subject: [PATCH] ci: faster, stricter CI (instrumented gate, parallel jobs, test forks) (#1348) --- .github/workflows/android.yml | 37 ++++++++++++++----- app/build.gradle.kts | 9 ++++- .../hermescontrol/data/ws/HermesWsClient.kt | 3 +- .../notification/NotificationReplyReceiver.kt | 8 +++- .../data/ws/HermesWsClientTest.kt | 3 +- .../NotificationReplyReceiverTest.kt | 17 ++++++--- 6 files changed, 58 insertions(+), 19 deletions(-) diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index 0697eb85..4783362d 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -97,11 +97,8 @@ jobs: - name: Add local bin to PATH run: echo "$HOME/.local/bin" >> $GITHUB_PATH - - name: Run ktlintCheck - run: ./gradlew ktlintCheck - - - name: Run checkColorLiterals (hardcoded Color guard, issue #622) - run: ./gradlew checkColorLiterals + - name: Run ktlintCheck + checkColorLiterals (hardcoded Color guard, issue #622) + run: ./gradlew ktlintCheck checkColorLiterals --continue - name: Upload ktlint reports if: always() @@ -185,6 +182,25 @@ jobs: - name: Run test run: ./gradlew test + # Telemetry only, never a gate: surfaces the slowest classes so perf work + # targets real offenders. + - name: Slowest unit-test classes + if: always() + run: | + python3 - <<'EOF' >> "$GITHUB_STEP_SUMMARY" + import glob, xml.etree.ElementTree as ET + rows = [] + for path in glob.glob("app/build/test-results/testDebugUnitTest/*.xml"): + root = ET.parse(path).getroot() + rows.append((float(root.get("time", 0)), int(root.get("tests", 0)), root.get("name", path))) + rows.sort(reverse=True) + print("### Slowest unit-test classes\n") + print(f"{len(rows)} classes, {sum(r[0] for r in rows):.0f}s summed across forks\n") + print("| Time | Tests | Class |\n|---:|---:|---|") + for secs, count, name in rows[:20]: + print(f"| {secs:.1f}s | {count} | `{name.rsplit('.', 1)[-1]}` |") + EOF + - name: Upload test reports if: always() uses: actions/upload-artifact@v7 @@ -194,11 +210,11 @@ jobs: retention-days: 7 # ───────────────────────────────────────────────────────────────────── - # Job 4: Build Debug APK — depends on all three above passing. + # Job 4: Build Debug APK — runs in parallel; CI Summary gates the result. # ───────────────────────────────────────────────────────────────────── build: name: Build Debug APK - needs: [changes, ktlint, android-lint, unit-tests] + needs: [changes] if: needs.changes.outputs.code == 'true' runs-on: ubuntu-latest timeout-minutes: 10 @@ -241,7 +257,7 @@ jobs: # ───────────────────────────────────────────────────────────────────── release-compile: name: Release Compile Check - needs: [changes, ktlint, android-lint, unit-tests] + needs: [changes] if: needs.changes.outputs.code == 'true' runs-on: ubuntu-latest timeout-minutes: 10 @@ -339,7 +355,7 @@ jobs: # ───────────────────────────────────────────────────────────────────── ci-summary: name: CI Summary - needs: [changes, ktlint, android-lint, unit-tests, build, release-compile] + needs: [changes, ktlint, android-lint, unit-tests, build, release-compile, instrumented-tests] runs-on: ubuntu-latest timeout-minutes: 2 if: always() @@ -355,11 +371,12 @@ jobs: TEST_RESULT: ${{ needs.unit-tests.result }} BUILD_RESULT: ${{ needs.build.result }} RELEASE_COMPILE_RESULT: ${{ needs.release-compile.result }} + INSTRUMENTED_RESULT: ${{ needs.instrumented-tests.result }} run: | LABEL="ci-red" FAILED=0 FAILED_JOBS="" - for entry in "ktlint:$KTLINT_RESULT" "android-lint:$LINT_RESULT" "unit-tests:$TEST_RESULT" "build:$BUILD_RESULT" "release-compile:$RELEASE_COMPILE_RESULT"; do + for entry in "ktlint:$KTLINT_RESULT" "android-lint:$LINT_RESULT" "unit-tests:$TEST_RESULT" "build:$BUILD_RESULT" "release-compile:$RELEASE_COMPILE_RESULT" "instrumented-tests:$INSTRUMENTED_RESULT"; do name="${entry%%:*}" result="${entry#*:}" echo "::notice::$name = $result" diff --git a/app/build.gradle.kts b/app/build.gradle.kts index df81b98c..a28b5da5 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -309,6 +309,13 @@ tasks.named("check") { dependsOn("checkColorLiterals") } -tasks.withType { +val includeBenchmarks = providers.gradleProperty("includeBenchmarks").isPresent + +tasks.withType().configureEach { useJUnitPlatform() + maxParallelForks = 2 + // Assertion-free timing probes are opt-in: ./gradlew testDebugUnitTest -PincludeBenchmarks + if (!includeBenchmarks) { + filter { excludeTestsMatching("*Benchmark") } + } } diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt index 1098e7be..09f3166a 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt @@ -415,6 +415,7 @@ object HermesWsClient { @VisibleForTesting internal fun probeLivenessOnTransportChange( + timeoutMs: Long = LIVENESS_PROBE_TIMEOUT_MS, onFailureAction: () -> Unit = { synchronized(outboundLock) { if (connected.get()) webSocket?.cancel() @@ -426,7 +427,7 @@ object HermesWsClient { transportProbeJob = wsScope.launch { Log.d(TAG, "Network transport changed — probing WebSocket liveness") - val alive = runCatching { ping(LIVENESS_PROBE_TIMEOUT_MS) }.isSuccess + val alive = runCatching { ping(timeoutMs) }.isSuccess if (alive) { Log.d(TAG, "WebSocket liveness probe succeeded on new transport") return@launch diff --git a/app/src/main/java/com/m57/hermescontrol/notification/NotificationReplyReceiver.kt b/app/src/main/java/com/m57/hermescontrol/notification/NotificationReplyReceiver.kt index b776b055..42867446 100644 --- a/app/src/main/java/com/m57/hermescontrol/notification/NotificationReplyReceiver.kt +++ b/app/src/main/java/com/m57/hermescontrol/notification/NotificationReplyReceiver.kt @@ -25,6 +25,9 @@ open class NotificationReplyReceiver : BroadcastReceiver() { const val EXTRA_SESSION_ID = "extra_session_id" private const val REPLY_TIMEOUT_MS = 5_000L + /** Overall deadline for persisting and submitting one notification reply. */ + private const val PERSISTENCE_TIMEOUT_MS = 5_000L + /** * Turn-boundary read budget inside the reply deadline. Deliberately tight: * the probe sits on the reply path, so a slow or unreachable gateway must @@ -44,6 +47,9 @@ open class NotificationReplyReceiver : BroadcastReceiver() { */ internal open fun goAsyncCompat(): BroadcastReceiver.PendingResult = goAsync() + /** Overridable so tests can prove the timeout path without waiting 5s. */ + internal open val persistenceTimeoutMs: Long get() = PERSISTENCE_TIMEOUT_MS + /** * Test-friendly wrapper for notification creation. Override in tests to * avoid [NotificationCompat.Builder.build()] calling Android framework @@ -75,7 +81,7 @@ open class NotificationReplyReceiver : BroadcastReceiver() { val pendingResult = goAsyncCompat() replyScope.launch { try { - withTimeout(5000L) { + withTimeout(persistenceTimeoutMs) { withContext(Dispatchers.IO) { val db = com.m57.hermescontrol.data.local.HermesDatabase diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt index e3df3221..14d5800e 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/HermesWsClientTest.kt @@ -2188,12 +2188,13 @@ class HermesWsClientTest { val failureLatch = CountDownLatch(1) HermesWsClient.probeLivenessOnTransportChange( + timeoutMs = 50L, onFailureAction = { failureLatch.countDown() }, ) - assertTrue("Expected failure action to be invoked on failed ping", failureLatch.await(6, TimeUnit.SECONDS)) + assertTrue("Expected failure action to be invoked on failed ping", failureLatch.await(3, TimeUnit.SECONDS)) } @Test diff --git a/app/src/test/java/com/m57/hermescontrol/notification/NotificationReplyReceiverTest.kt b/app/src/test/java/com/m57/hermescontrol/notification/NotificationReplyReceiverTest.kt index c287e05f..fd0953b9 100644 --- a/app/src/test/java/com/m57/hermescontrol/notification/NotificationReplyReceiverTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/notification/NotificationReplyReceiverTest.kt @@ -412,16 +412,23 @@ class NotificationReplyReceiverTest { kotlinx.coroutines.delay(10000) } - receiver.onReceive(mockContext, mockIntent) + // Same timeout path as production, with a short deadline instead of 5s. + val shortTimeoutReceiver = + object : NotificationReplyReceiver() { + override fun goAsyncCompat(): BroadcastReceiver.PendingResult = mockPendingResult - // Wait longer than the 5-second timeout, but less than the 10-second delay - Thread.sleep(6000) + override fun buildReplyNotification(context: Context): Notification = mockNotification + + override val persistenceTimeoutMs: Long = 50L + } + + shortTimeoutReceiver.onReceive(mockContext, mockIntent) // Timeout should be caught in catch block and logged - verify { android.util.Log.e("NotificationReply", any(), any()) } + verify(timeout = 3_000) { android.util.Log.e("NotificationReply", any(), any()) } // Pending result must still finish after timeout - verify { mockPendingResult.finish() } + verify(timeout = 3_000) { mockPendingResult.finish() } } // ── Helpers ────────────────────────────────────────────────────────────