Merge pull request 'chore(sonar): wire JaCoCo coverage and clear actionable smells' (#26) from chore/sonar-coverage-and-cleanup into main

Reviewed-on: #26
Reviewed-by: Colby Whitlock <whitlocktech@gmail.com>

.gitea/workflows/sonarqube.yml

@@ -18,11 +18,12 @@
 # SonarQube Quality Gate, so a failing gate does not fail this job — check the
 # dashboard when you want to.
 #
-# Scope: this analyses the Kotlin source directly (the Sonar scanner reads
+# Scope: the Sonar scanner reads sonar-project.properties and analyses the Kotlin
-# sonar-project.properties). It does NOT run a Gradle build, so no Android SDK /
+# source directly. Before the scan we run the JVM unit tests + JaCoCo so SonarQube
-# JDK install is needed — the Kotlin analyzer is source-based. See the "Optional
+# receives real coverage (sonar.coverage.jacoco.xmlReportPaths) — otherwise it
-# enrichment" note in sonar-project.properties for wiring in Android Lint /
+# reports 0% and the coverage gate fails despite the test suite existing. That
-# coverage reports later.
+# Gradle step needs JDK 17 + the Android SDK (same toolchain as pr-checks.yml);
+# the runner container is bare, so base tools are apt-installed first.
 
 name: SonarQube
 
@@ -40,6 +41,16 @@ jobs:
   analysis:
     runs-on: ubuntu-latest
     steps:
+      # The bare runner container lacks git/curl/unzip (checkout + sdkmanager need
+      # them) and we install JDK 17 from the Ubuntu archive rather than
+      # actions/setup-java (this runner can't reach api.adoptium.net). Mirrors
+      # pr-checks.yml — see its header note.
+      - name: Install base tools + JDK 17
+        run: |
+          apt-get update
+          apt-get install -y git curl unzip openjdk-17-jdk-headless
+          echo "JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64" >> "$GITHUB_ENV"
+
       - name: Check out (full history for accurate new-code + blame)
         uses: actions/checkout@v4
         with:
@@ -47,6 +58,31 @@ jobs:
           # compute "new code". A shallow clone degrades both.
           fetch-depth: 0
 
+      - name: Set up Android SDK
+        uses: android-actions/setup-android@v3
+
+      - name: Install Android SDK packages
+        run: |
+          set +o pipefail
+          yes | sdkmanager "platform-tools" "platforms;android-35" "build-tools;35.0.0"
+
+      - name: Cache Gradle
+        uses: actions/cache@v4
+        with:
+          path: |
+            ~/.gradle/caches
+            ~/.gradle/wrapper
+          key: gradle-${{ runner.os }}-${{ hashFiles('**/*.gradle.kts', 'gradle/libs.versions.toml', 'gradle/wrapper/gradle-wrapper.properties') }}
+          restore-keys: |
+            gradle-${{ runner.os }}-
+
+      # Produce the JaCoCo XML the scan reports as coverage. Scoped to the debug
+      # variant (matches enableUnitTestCoverage) to keep peak memory down.
+      - name: Unit tests + JaCoCo coverage
+        run: |
+          chmod +x ./gradlew
+          ./gradlew --no-daemon testDebugUnitTest jacocoTestReport
+
       - name: Run SonarQube scan
         uses: sonarsource/sonarqube-scan-action@v4
         env:

app/build.gradle.kts

@@ -10,6 +10,11 @@ plugins {
     alias(libs.plugins.kotlin.serialization)
     alias(libs.plugins.ksp)
     alias(libs.plugins.hilt)
+    jacoco
+}
+
+jacoco {
+    toolVersion = "0.8.12"
 }
 
 // Release signing material (PLAN.md §12) is never committed. It is read from, in
@@ -78,6 +83,11 @@ android {
     }
 
     buildTypes {
+        debug {
+            // Produce a JaCoCo .exec from JVM unit tests so SonarQube receives real
+            // coverage (§12.1). Debug-only: the scan analyses the debug variant.
+            enableUnitTestCoverage = true
+        }
         release {
             // R8 full-mode minify + resource shrink (§7: no offline cache, so a lean
             // release APK). Keep rules live in proguard-rules.pro.
@@ -170,3 +180,35 @@ dependencies {
     androidTestImplementation(platform(libs.androidx.compose.bom))
     androidTestImplementation(libs.androidx.compose.ui.test.junit4)
 }
+
+// JaCoCo XML coverage from the JVM unit tests, consumed by SonarQube (§12.1). Generated,
+// DI (Hilt), and Compose-scaffold classes are excluded so they don't dilute the number;
+// pure-@Composable UI is excluded on the Sonar side (sonar.coverage.exclusions) because
+// JVM unit tests can't execute composable bodies without Robolectric.
+tasks.register<JacocoReport>("jacocoTestReport") {
+    dependsOn("testDebugUnitTest")
+    group = "verification"
+    description = "Generates JaCoCo XML/HTML coverage for the debug unit tests."
+
+    reports {
+        xml.required.set(true)
+        html.required.set(true)
+    }
+
+    val coverageExcludes = listOf(
+        "**/R.class", "**/R$*.class", "**/BuildConfig.*", "**/Manifest*.*",
+        "**/*_Hilt*.*", "**/Hilt_*.*", "**/*_Factory*.*", "**/*_MembersInjector*.*",
+        "**/*_Impl*.*", "**/di/**", "**/*Module.*", "**/*Module$*.*",
+        "**/*ComposableSingletons*.*", "**/ComposableSingletons$*.*",
+    )
+    val buildDirFile = layout.buildDirectory.get().asFile
+    classDirectories.setFrom(
+        fileTree("$buildDirFile/tmp/kotlin-classes/debug") { exclude(coverageExcludes) },
+    )
+    sourceDirectories.setFrom(files("src/main/java", "src/main/kotlin"))
+    executionData.setFrom(
+        fileTree(buildDirFile) {
+            include("outputs/unit_test_code_coverage/debugUnitTest/testDebugUnitTest.exec")
+        },
+    )
+}

app/src/main/java/com/runicgateway/app/ui/admin/AdminContentScreen.kt

@@ -30,7 +30,6 @@ import androidx.compose.runtime.Composable
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableIntStateOf
 import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.remember
 import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment

app/src/main/java/com/runicgateway/app/ui/admin/AdminSupportScreen.kt

@@ -84,7 +84,6 @@ fun AdminSupportScreen(
 
     replyTo?.let { page ->
         RespondDialog(
-            page = page,
             onDismiss = { replyTo = null },
             onSend = { message, close ->
                 viewModel.respond(page.pageId, message, close)
@@ -122,7 +121,6 @@ private fun SupportPageCard(
 
 @Composable
 private fun RespondDialog(
-    page: SupportPageDto,
     onDismiss: () -> Unit,
     onSend: (message: String, close: Boolean) -> Unit,
 ) {

app/src/main/java/com/runicgateway/app/ui/auth/LoginViewModel.kt

@@ -153,18 +153,11 @@ class LoginViewModel @Inject constructor(
     fun submit() {
         val s = _state.value
         if (s.submitting) return
-        if (s.username.isBlank() || s.password.isBlank()) {
+        val validationError = validateForSubmit(s)
-            _state.update { it.copy(error = LoginError.INVALID_CREDENTIALS) }
+        if (validationError != null) {
+            _state.update { it.copy(error = validationError) }
             return
         }
-        // If 2FA is being requested, the chosen second factor must accompany the resubmit.
-        if (s.totpRequired) {
-            val factor = if (s.useRecoveryCode) s.recoveryCode else s.code
-            if (factor.isBlank()) {
-                _state.update { it.copy(error = LoginError.BAD_CODE) }
-                return
-            }
-        }
 
         _state.update { it.copy(submitting = true, error = null) }
         viewModelScope.launch {
@@ -178,36 +171,50 @@ class LoginViewModel @Inject constructor(
                 recoveryCode = recoveryCode,
                 trustDevice = s.trustDevice,
             )
-            when (result) {
+            applyLoginResult(result)
-                is LoginResult.Success ->
-                    // The trusted-device cap (result.trustLimitReached) is an edge case:
-                    // login succeeded but the device wasn't remembered. It's surfaced +
-                    // managed on the Trusted Devices screen rather than blocking sign-in.
-                    _state.update { it.copy(submitting = false, signedIn = true) }
-
-                LoginResult.TotpRequired ->
-                    // Reveal the 2FA fields; a wrong code/recovery code re-lands here as BAD_CODE.
-                    _state.update {
-                        val hadFactor = if (it.useRecoveryCode) it.recoveryCode.isNotBlank() else it.code.isNotBlank()
-                        it.copy(
-                            submitting = false,
-                            totpRequired = true,
-                            error = if (hadFactor) LoginError.BAD_CODE else null,
-                        )
-                    }
-
-                LoginResult.InvalidCredentials ->
-                    _state.update { it.copy(submitting = false, error = LoginError.INVALID_CREDENTIALS) }
-
-                LoginResult.RateLimited ->
-                    _state.update { it.copy(submitting = false, error = LoginError.RATE_LIMITED) }
-
-                LoginResult.ServerError ->
-                    _state.update { it.copy(submitting = false, error = LoginError.SERVER) }
-
-                LoginResult.NetworkError ->
-                    _state.update { it.copy(submitting = false, error = LoginError.NETWORK) }
-            }
         }
     }
+
+    /** Pre-flight form checks for [submit]; returns the error to surface, or null if ready to send. */
+    private fun validateForSubmit(s: UiState): LoginError? {
+        if (s.username.isBlank() || s.password.isBlank()) return LoginError.INVALID_CREDENTIALS
+        // If 2FA is being requested, the chosen second factor must accompany the resubmit.
+        if (s.totpRequired) {
+            val factor = if (s.useRecoveryCode) s.recoveryCode else s.code
+            if (factor.isBlank()) return LoginError.BAD_CODE
+        }
+        return null
+    }
+
+    /** Folds a [LoginResult] back into the UI state (clears [UiState.submitting] on every path). */
+    private fun applyLoginResult(result: LoginResult) = when (result) {
+        is LoginResult.Success ->
+            // The trusted-device cap (result.trustLimitReached) is an edge case:
+            // login succeeded but the device wasn't remembered. It's surfaced +
+            // managed on the Trusted Devices screen rather than blocking sign-in.
+            _state.update { it.copy(submitting = false, signedIn = true) }
+
+        LoginResult.TotpRequired ->
+            // Reveal the 2FA fields; a wrong code/recovery code re-lands here as BAD_CODE.
+            _state.update {
+                val hadFactor = if (it.useRecoveryCode) it.recoveryCode.isNotBlank() else it.code.isNotBlank()
+                it.copy(
+                    submitting = false,
+                    totpRequired = true,
+                    error = if (hadFactor) LoginError.BAD_CODE else null,
+                )
+            }
+
+        LoginResult.InvalidCredentials ->
+            _state.update { it.copy(submitting = false, error = LoginError.INVALID_CREDENTIALS) }
+
+        LoginResult.RateLimited ->
+            _state.update { it.copy(submitting = false, error = LoginError.RATE_LIMITED) }
+
+        LoginResult.ServerError ->
+            _state.update { it.copy(submitting = false, error = LoginError.SERVER) }
+
+        LoginResult.NetworkError ->
+            _state.update { it.copy(submitting = false, error = LoginError.NETWORK) }
+    }
 }

sonar-project.properties

@@ -20,13 +20,26 @@ sonar.exclusions=**/build/**,**/.gradle/**,**/generated/**
 
 sonar.sourceEncoding=UTF-8
 
-# ── Optional enrichment (enable once the reports are produced in CI) ──
+# ── Coverage (JaCoCo) ──
-# For richer Kotlin/Android results, run the reporters in sonarqube.yml and point
+# sonarqube.yml runs `./gradlew testDebugUnitTest jacocoTestReport` before the scan;
-# SonarQube at their output:
+# that task (app/build.gradle.kts) writes this XML. Without it, Sonar reports 0%
-#   • Android Lint:  ./gradlew lintDebug   → app/build/reports/lint-results-debug.xml
+# coverage even though the JVM unit suite (app/src/test) exists.
+sonar.coverage.jacoco.xmlReportPaths=app/build/reports/jacoco/jacocoTestReport/jacocoTestReport.xml
+
+# Exclude from *coverage* (not from analysis): pure-@Composable UI can't be exercised
+# by JVM unit tests without Robolectric, so counting those lines would unfairly sink
+# new-code coverage. Logic (ViewModels, repositories, core, DTOs) stays measured.
+sonar.coverage.exclusions=\
+  app/src/main/java/**/ui/**/*Screen.kt,\
+  app/src/main/java/**/ui/**/*Screen*.kt,\
+  app/src/main/java/**/ui/theme/**,\
+  app/src/main/java/**/RunicApp.kt,\
+  app/src/main/java/**/MainActivity.kt,\
+  app/src/main/java/**/*Application.kt
+
+# ── Optional enrichment (enable once produced in CI) ──
+#   • Android Lint:  ./gradlew lintDebug → app/build/reports/lint-results-debug.xml
 #     sonar.androidLint.reportPaths=app/build/reports/lint-results-debug.xml
-#   • JaCoCo coverage (needs a coverage-enabled test run):
-#     sonar.coverage.jacoco.xmlReportPaths=app/build/reports/jacoco/.../*.xml
 # The alternative to the CLI scanner used here is the SonarQube Gradle plugin
 # (org.sonarqube), which auto-discovers these reports; the CLI + properties file
 # is used instead to keep this repo's setup identical to website/ and link/.