From 43215b49a030935f4341f7deb3c1418e8373efb8 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Wed, 22 Jul 2026 13:58:52 -0500 Subject: [PATCH] chore(sonar): wire JaCoCo coverage and clear actionable smells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the SonarQube coverage gate (0% on new code) — a reporting gap, not a testing gap: the JVM unit suite already exists but the source-only scan never received a coverage report. - app/build.gradle.kts: apply jacoco, enable debug unit-test coverage, add a jacocoTestReport task (excludes generated/Hilt/Compose-singleton classes) - sonar-project.properties: consume the JaCoCo XML; exclude pure-@Composable UI from coverage (JVM unit tests can't execute composable bodies) - .gitea/workflows/sonarqube.yml: run JDK 17 + Android SDK + `testDebugUnitTest jacocoTestReport` before the scan Also clear the three actionable code smells: remove an unused import (AdminContentScreen), remove an unused parameter (AdminSupportScreen. RespondDialog), and decompose LoginViewModel.submit() (cognitive complexity 20 -> under 15). The remaining 12 smells (snake_case DTO fields that mirror the JSON wire contract; Compose/nav complexity) are marked Won't Fix in SonarQube with rationale. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01NgyHnrNa8WwG3doxvxjuCr --- .gitea/workflows/sonarqube.yml | 46 ++++++++-- app/build.gradle.kts | 42 +++++++++ .../app/ui/admin/AdminContentScreen.kt | 1 - .../app/ui/admin/AdminSupportScreen.kt | 2 - .../app/ui/auth/LoginViewModel.kt | 87 ++++++++++--------- sonar-project.properties | 25 ++++-- 6 files changed, 149 insertions(+), 54 deletions(-) diff --git a/.gitea/workflows/sonarqube.yml b/.gitea/workflows/sonarqube.yml index 756f2b4..c7f9b6b 100644 --- a/.gitea/workflows/sonarqube.yml +++ b/.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 -# sonar-project.properties). It does NOT run a Gradle build, so no Android SDK / -# JDK install is needed — the Kotlin analyzer is source-based. See the "Optional -# enrichment" note in sonar-project.properties for wiring in Android Lint / -# coverage reports later. +# Scope: the Sonar scanner reads sonar-project.properties and analyses the Kotlin +# source directly. Before the scan we run the JVM unit tests + JaCoCo so SonarQube +# receives real coverage (sonar.coverage.jacoco.xmlReportPaths) — otherwise it +# reports 0% and the coverage gate fails despite the test suite existing. That +# 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: diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 873409a..0df18df 100644 --- a/app/build.gradle.kts +++ b/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("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") + }, + ) +} diff --git a/app/src/main/java/com/runicgateway/app/ui/admin/AdminContentScreen.kt b/app/src/main/java/com/runicgateway/app/ui/admin/AdminContentScreen.kt index e230e1d..01aab33 100644 --- a/app/src/main/java/com/runicgateway/app/ui/admin/AdminContentScreen.kt +++ b/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 diff --git a/app/src/main/java/com/runicgateway/app/ui/admin/AdminSupportScreen.kt b/app/src/main/java/com/runicgateway/app/ui/admin/AdminSupportScreen.kt index 9f2d369..5a7eb91 100644 --- a/app/src/main/java/com/runicgateway/app/ui/admin/AdminSupportScreen.kt +++ b/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, ) { diff --git a/app/src/main/java/com/runicgateway/app/ui/auth/LoginViewModel.kt b/app/src/main/java/com/runicgateway/app/ui/auth/LoginViewModel.kt index f71865f..1059c2e 100644 --- a/app/src/main/java/com/runicgateway/app/ui/auth/LoginViewModel.kt +++ b/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()) { - _state.update { it.copy(error = LoginError.INVALID_CREDENTIALS) } + val validationError = validateForSubmit(s) + 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) { - 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) } - } + applyLoginResult(result) } } + + /** 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) } + } } diff --git a/sonar-project.properties b/sonar-project.properties index 8b91d73..5078f11 100644 --- a/sonar-project.properties +++ b/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) ── -# For richer Kotlin/Android results, run the reporters in sonarqube.yml and point -# SonarQube at their output: -# • Android Lint: ./gradlew lintDebug → app/build/reports/lint-results-debug.xml +# ── Coverage (JaCoCo) ── +# sonarqube.yml runs `./gradlew testDebugUnitTest jacocoTestReport` before the scan; +# that task (app/build.gradle.kts) writes this XML. Without it, Sonar reports 0% +# 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/.