ci: faster, stricter CI (instrumented gate, parallel jobs, test forks) (#1348)
This commit is contained in:
6 files changed
+58
-19
No files matched your search
@@ -97,11 +97,8 @@ jobs:
|
|||||||
- name: Add local bin to PATH
|
- name: Add local bin to PATH
|
||||||
run: echo "$HOME/.local/bin" >> $GITHUB_PATH
|
run: echo "$HOME/.local/bin" >> $GITHUB_PATH
|
||||||
|
|
||||||
- name: Run ktlintCheck
|
- name: Run ktlintCheck + checkColorLiterals (hardcoded Color guard, issue #622)
|
||||||
run: ./gradlew ktlintCheck
|
run: ./gradlew ktlintCheck checkColorLiterals --continue
|
||||||
|
|
||||||
- name: Run checkColorLiterals (hardcoded Color guard, issue #622)
|
|
||||||
run: ./gradlew checkColorLiterals
|
|
||||||
|
|
||||||
- name: Upload ktlint reports
|
- name: Upload ktlint reports
|
||||||
if: always()
|
if: always()
|
||||||
@@ -185,6 +182,25 @@ jobs:
|
|||||||
- name: Run test
|
- name: Run test
|
||||||
run: ./gradlew 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
|
- name: Upload test reports
|
||||||
if: always()
|
if: always()
|
||||||
uses: actions/upload-artifact@v7
|
uses: actions/upload-artifact@v7
|
||||||
@@ -194,11 +210,11 @@ jobs:
|
|||||||
retention-days: 7
|
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:
|
build:
|
||||||
name: Build Debug APK
|
name: Build Debug APK
|
||||||
needs: [changes, ktlint, android-lint, unit-tests]
|
needs: [changes]
|
||||||
if: needs.changes.outputs.code == 'true'
|
if: needs.changes.outputs.code == 'true'
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
timeout-minutes: 10
|
timeout-minutes: 10
|
||||||
@@ -241,7 +257,7 @@ jobs:
|
|||||||
# ─────────────────────────────────────────────────────────────────────
|
# ─────────────────────────────────────────────────────────────────────
|
||||||
release-compile:
|
release-compile:
|
||||||
name: Release Compile Check
|
name: Release Compile Check
|
||||||
needs: [changes, ktlint, android-lint, unit-tests]
|
needs: [changes]
|
||||||
if: needs.changes.outputs.code == 'true'
|
if: needs.changes.outputs.code == 'true'
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
timeout-minutes: 10
|
timeout-minutes: 10
|
||||||
@@ -339,7 +355,7 @@ jobs:
|
|||||||
# ─────────────────────────────────────────────────────────────────────
|
# ─────────────────────────────────────────────────────────────────────
|
||||||
ci-summary:
|
ci-summary:
|
||||||
name: 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
|
runs-on: ubuntu-latest
|
||||||
timeout-minutes: 2
|
timeout-minutes: 2
|
||||||
if: always()
|
if: always()
|
||||||
@@ -355,11 +371,12 @@ jobs:
|
|||||||
TEST_RESULT: ${{ needs.unit-tests.result }}
|
TEST_RESULT: ${{ needs.unit-tests.result }}
|
||||||
BUILD_RESULT: ${{ needs.build.result }}
|
BUILD_RESULT: ${{ needs.build.result }}
|
||||||
RELEASE_COMPILE_RESULT: ${{ needs.release-compile.result }}
|
RELEASE_COMPILE_RESULT: ${{ needs.release-compile.result }}
|
||||||
|
INSTRUMENTED_RESULT: ${{ needs.instrumented-tests.result }}
|
||||||
run: |
|
run: |
|
||||||
LABEL="ci-red"
|
LABEL="ci-red"
|
||||||
FAILED=0
|
FAILED=0
|
||||||
FAILED_JOBS=""
|
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%%:*}"
|
name="${entry%%:*}"
|
||||||
result="${entry#*:}"
|
result="${entry#*:}"
|
||||||
echo "::notice::$name = $result"
|
echo "::notice::$name = $result"
|
||||||
|
|||||||
@@ -309,6 +309,13 @@ tasks.named("check") {
|
|||||||
dependsOn("checkColorLiterals")
|
dependsOn("checkColorLiterals")
|
||||||
}
|
}
|
||||||
|
|
||||||
tasks.withType<Test> {
|
val includeBenchmarks = providers.gradleProperty("includeBenchmarks").isPresent
|
||||||
|
|
||||||
|
tasks.withType<Test>().configureEach {
|
||||||
useJUnitPlatform()
|
useJUnitPlatform()
|
||||||
|
maxParallelForks = 2
|
||||||
|
// Assertion-free timing probes are opt-in: ./gradlew testDebugUnitTest -PincludeBenchmarks
|
||||||
|
if (!includeBenchmarks) {
|
||||||
|
filter { excludeTestsMatching("*Benchmark") }
|
||||||
|
}
|
||||||
}
|
}
|
||||||
@@ -415,6 +415,7 @@ object HermesWsClient {
|
|||||||
|
|
||||||
@VisibleForTesting
|
@VisibleForTesting
|
||||||
internal fun probeLivenessOnTransportChange(
|
internal fun probeLivenessOnTransportChange(
|
||||||
|
timeoutMs: Long = LIVENESS_PROBE_TIMEOUT_MS,
|
||||||
onFailureAction: () -> Unit = {
|
onFailureAction: () -> Unit = {
|
||||||
synchronized(outboundLock) {
|
synchronized(outboundLock) {
|
||||||
if (connected.get()) webSocket?.cancel()
|
if (connected.get()) webSocket?.cancel()
|
||||||
@@ -426,7 +427,7 @@ object HermesWsClient {
|
|||||||
transportProbeJob =
|
transportProbeJob =
|
||||||
wsScope.launch {
|
wsScope.launch {
|
||||||
Log.d(TAG, "Network transport changed — probing WebSocket liveness")
|
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) {
|
if (alive) {
|
||||||
Log.d(TAG, "WebSocket liveness probe succeeded on new transport")
|
Log.d(TAG, "WebSocket liveness probe succeeded on new transport")
|
||||||
return@launch
|
return@launch
|
||||||
|
|||||||
@@ -25,6 +25,9 @@ open class NotificationReplyReceiver : BroadcastReceiver() {
|
|||||||
const val EXTRA_SESSION_ID = "extra_session_id"
|
const val EXTRA_SESSION_ID = "extra_session_id"
|
||||||
private const val REPLY_TIMEOUT_MS = 5_000L
|
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:
|
* Turn-boundary read budget inside the reply deadline. Deliberately tight:
|
||||||
* the probe sits on the reply path, so a slow or unreachable gateway must
|
* 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()
|
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
|
* Test-friendly wrapper for notification creation. Override in tests to
|
||||||
* avoid [NotificationCompat.Builder.build()] calling Android framework
|
* avoid [NotificationCompat.Builder.build()] calling Android framework
|
||||||
@@ -75,7 +81,7 @@ open class NotificationReplyReceiver : BroadcastReceiver() {
|
|||||||
val pendingResult = goAsyncCompat()
|
val pendingResult = goAsyncCompat()
|
||||||
replyScope.launch {
|
replyScope.launch {
|
||||||
try {
|
try {
|
||||||
withTimeout(5000L) {
|
withTimeout(persistenceTimeoutMs) {
|
||||||
withContext(Dispatchers.IO) {
|
withContext(Dispatchers.IO) {
|
||||||
val db =
|
val db =
|
||||||
com.m57.hermescontrol.data.local.HermesDatabase
|
com.m57.hermescontrol.data.local.HermesDatabase
|
||||||
|
|||||||
@@ -2188,12 +2188,13 @@ class HermesWsClientTest {
|
|||||||
|
|
||||||
val failureLatch = CountDownLatch(1)
|
val failureLatch = CountDownLatch(1)
|
||||||
HermesWsClient.probeLivenessOnTransportChange(
|
HermesWsClient.probeLivenessOnTransportChange(
|
||||||
|
timeoutMs = 50L,
|
||||||
onFailureAction = {
|
onFailureAction = {
|
||||||
failureLatch.countDown()
|
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
|
@Test
|
||||||
|
|||||||
+12
-5
@@ -412,16 +412,23 @@ class NotificationReplyReceiverTest {
|
|||||||
kotlinx.coroutines.delay(10000)
|
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
|
override fun buildReplyNotification(context: Context): Notification = mockNotification
|
||||||
Thread.sleep(6000)
|
|
||||||
|
override val persistenceTimeoutMs: Long = 50L
|
||||||
|
}
|
||||||
|
|
||||||
|
shortTimeoutReceiver.onReceive(mockContext, mockIntent)
|
||||||
|
|
||||||
// Timeout should be caught in catch block and logged
|
// Timeout should be caught in catch block and logged
|
||||||
verify { android.util.Log.e("NotificationReply", any<String>(), any()) }
|
verify(timeout = 3_000) { android.util.Log.e("NotificationReply", any<String>(), any()) }
|
||||||
|
|
||||||
// Pending result must still finish after timeout
|
// Pending result must still finish after timeout
|
||||||
verify { mockPendingResult.finish() }
|
verify(timeout = 3_000) { mockPendingResult.finish() }
|
||||||
}
|
}
|
||||||
|
|
||||||
// ── Helpers ────────────────────────────────────────────────────────────
|
// ── Helpers ────────────────────────────────────────────────────────────
|
||||||
|
|||||||
Reference in new issue
Block a user