diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index de3ba2f5b..1a98334ba 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -46,6 +46,7 @@ This will: - Copy the necessary `google-services.json` files - Download all dependencies - Build all modules +- Run the R8/ProGuard gate - Run checkstyle - Run unit tests @@ -145,6 +146,7 @@ commands can be run locally to highlight any issues before committing your code: This script runs: - `./gradlew clean` - `./gradlew assembleDebug` - Build all modules +- `./gradlew proguard-tests:build` - Check the libraries' consumer ProGuard rules against R8 - `./gradlew checkstyle` - Run code style checks - `./gradlew testDebugUnitTest -x :e2eTest:testDebugUnitTest` - Run unit tests diff --git a/okf-bundle/ci-workflows/android.md b/okf-bundle/ci-workflows/android.md index 092f220cf..7463b44e7 100644 --- a/okf-bundle/ci-workflows/android.md +++ b/okf-bundle/ci-workflows/android.md @@ -35,10 +35,23 @@ Canonical owner for the CI unit-path step list. Script: [scripts/build.sh](../.. 1. Copy `library/google-services.json` → `app/` and `proguard-tests/` 2. `./gradlew --max-workers=2 clean` 3. `./gradlew --max-workers=2 assembleDebug` -4. `./gradlew --max-workers=2 checkstyle` -5. `./gradlew --max-workers=2 testDebugUnitTest -x :e2eTest:testDebugUnitTest` +4. `./gradlew --max-workers=2 proguard-tests:build` — the R8 gate, [detail below](#proguard-step) +5. `./gradlew --max-workers=2 checkstyle` +6. `./gradlew --max-workers=2 testDebugUnitTest -x :e2eTest:testDebugUnitTest` -Step 4 is **Java-only** (`include("**/*.java")`), so it inspects zero files in the Kotlin modules — [Kotlin blind spot](../testing/agent-command-policy.md#checkstyle-kotlin-blind-spot). Android Lint covers that gap and runs in its own workflow, **not** in `build.sh` — see below. +Step 5 is **Java-only** (`include("**/*.java")`), so it inspects zero files in the Kotlin modules — [Kotlin blind spot](../testing/agent-command-policy.md#checkstyle-kotlin-blind-spot). Android Lint covers that gap and runs in its own workflow, **not** in `build.sh` — see below. + + + +### Step 4: the R8 gate + +`:proguard-tests` is a source-less application module that depends on `:auth`, `:firestore`, `:database` and `:storage` and builds its release variant with `isMinifyEnabled = true`, so `minifyReleaseWithR8` is the only place the libraries' consumer ProGuard rules are ever applied. Each library's own release build sets `isMinifyEnabled = false`. `consumerProguardFiles` is declared by `:auth` (`auth-proguard.pro`), `:firestore` and `:database` (`proguard-rules.pro` each); `:storage` and `:common` declare none, though AGP also merges rules generated by annotation processors. The gate is sensitive: removing `-dontwarn com.facebook.**` from `auth-proguard.pro` fails `minifyReleaseWithR8`, because `:auth` takes `facebook-login` as `compileOnly`. It proves the rules shrink and package, not that the shrunk app behaves correctly at runtime. + +The module's `beforeVariants` filter disables its **debug** variant when `CI=true`, so CI builds the release variant only; a local run of the script builds both. + + + +**Lint footprint.** `build` depends on `check`, which depends on `lint`, so this step runs Android Lint too. On CI that is `:proguard-tests:lintRelease`; locally, with `CI` unset, it is `:proguard-tests:lintDebug` — the ungated debug variant [`lintAll` does not cover](../testing/agent-command-policy.md#canonical-registry), so a debug-only finding can redden a local `build.sh` while CI stays green. Either way lint **analysis** fans out to the library dependencies (`:auth`, `:common`, `:database`, `:firestore`, `:storage`, plus `:internal:lintchecks` locally, since `:auth` takes it as `debugImplementation`), because AGP builds a lint model for each; only the **report** is scoped to `:proguard-tests`. Two consequences: it duplicates analysis [`lintAll`](#lint-workflow) already does, and since the step sits at position 4 of 6 under `set -e`, a finding here aborts the run before `checkstyle` and before every unit-test result — the same masking that kept lint out of this script in the first place (below). Drop to `:proguard-tests:assembleRelease` if that trade stops being worth it. @@ -52,8 +65,6 @@ It is a separate workflow rather than a step in `build.sh` for two reasons. Lint The workflow copies `library/google-services.json` into `app/` and `proguard-tests/` before running, because `lintAll` gates `:proguard-tests:lintRelease` and that module applies the `google-services` plugin. -`proguard-tests:build` is currently commented out (re-enable before release). Green Android CI does **not** prove ProGuard/R8 packaging. - ## Agent notes - Match this path locally with `./scripts/build.sh` — [agent command policy](../testing/agent-command-policy.md). diff --git a/okf-bundle/modules/index.md b/okf-bundle/modules/index.md index 7c5f36864..92a620845 100644 --- a/okf-bundle/modules/index.md +++ b/okf-bundle/modules/index.md @@ -21,7 +21,7 @@ Version and SDK floors: `buildSrc/.../Config.kt` — [repo tooling](../repo-tool | `:library` | Umbrella / publish aggregation (`prepareArtifacts`) | | `:app` | Demo app (Auth Compose sample) | | `:e2eTest` | Auth emulator e2e (Robolectric + Compose UI test) | -| `:proguard-tests` | R8/ProGuard packaging checks (disabled in CI unit path — [Android CI](../ci-workflows/android.md)) | +| `:proguard-tests` | R8/ProGuard packaging gate for the libraries' consumer rules; run by `build.sh` — [Android CI](../ci-workflows/android.md#proguard-step) | | `:lint`, `:internal:lint`, `:internal:lintchecks` | Custom lint detectors — [what each is for](#custom-lint-modules) | | `buildSrc` | Shared `Config` (version, SDK levels, submodule list) | diff --git a/okf-bundle/testing/agent-command-policy.md b/okf-bundle/testing/agent-command-policy.md index 1f7fc5ce1..eded88173 100644 --- a/okf-bundle/testing/agent-command-policy.md +++ b/okf-bundle/testing/agent-command-policy.md @@ -26,7 +26,7 @@ Single source for **which shell commands agents may run** in this repo. E2e is a | Intent | Command | Never use instead | |--------|---------|-------------------| -| Full CI unit path (assemble + checkstyle + unit tests) | `./scripts/build.sh` | Ad-hoc `./gradlew clean assembleDebug test` without checkstyle; inventing a different exclusion set | +| Full CI unit path (assemble + R8 + checkstyle + unit tests) | `./scripts/build.sh` | Ad-hoc `./gradlew clean assembleDebug test` without checkstyle; inventing a different exclusion set | | Unit tests (all library modules; exclude e2eTest) | `./gradlew testDebugUnitTest -x :e2eTest:testDebugUnitTest` | Bare `./gradlew test` (pulls wrong tasks / e2e); IDE-only as the agent gate | | Unit tests (one module with a real `src/test` suite) | `./gradlew ::testDebugUnitTest` (e.g. `:auth:testDebugUnitTest`, `:firestore:…`, `:storage:…`) | `:common:testDebugUnitTest` / `:database:testDebugUnitTest` as “green” evidence (empty suites — [empty unit-suite trap](#empty-unit-suite-trap)); full suite when only one module changed *as a substitute for* the CI path at handoff | | Assemble one module (when no JVM unit suite) | `./gradlew ::assembleDebug` (e.g. `:database`, `:common`) | Treating empty `testDebugUnitTest` as validation | @@ -122,9 +122,10 @@ Single source for **which shell commands agents may run** in this repo. E2e is a - [.github/PULL_REQUEST_TEMPLATE.md](../../.github/PULL_REQUEST_TEMPLATE.md) mentions `./gradlew check` (stale vs current CI). - **Agents:** treat **`./scripts/build.sh`** as the CI-matching unit path — [Android CI](../ci-workflows/android.md). Full handoff (including e2e when Auth UI touched): [validation checklist](validation-checklist.md). -### ProGuard tests disabled in build.sh +### `proguard-tests:build` is the only R8 gate -- See [Android CI § `build.sh`](../ci-workflows/android.md#what-buildsh-runs) — `proguard-tests:build` is commented out; green unit CI does not prove ProGuard/R8. +- `build.sh` runs it, and it is the only task that applies the libraries' consumer ProGuard rules — [Android CI § the ProGuard step](../ci-workflows/android.md#proguard-step). +- Do **not** comment it out to clear a red build. A failure there is a real defect in the consumer rules or in code they must keep; fix the rules. ### Emulator foreground vs CI diff --git a/okf-bundle/testing/validation-checklist.md b/okf-bundle/testing/validation-checklist.md index 0233df3c1..f04eca27b 100644 --- a/okf-bundle/testing/validation-checklist.md +++ b/okf-bundle/testing/validation-checklist.md @@ -26,7 +26,7 @@ Work types and tiers: [change authoring workflow](change-authoring-workflow.md). ## Build and unit tests -Repo root. Full CI unit path (what `build.sh` runs — `assembleDebug`, `checkstyle`, unit tests): [Android CI](../ci-workflows/android.md). Lint and e2e are **separate** workflows; `build.sh` does not run them. +Repo root. Full CI unit path (what `build.sh` runs — `assembleDebug`, `proguard-tests:build`, `checkstyle`, unit tests): [Android CI](../ci-workflows/android.md). Lint and e2e are **separate** workflows; `build.sh` does not run `lintAll` or `e2eTest`. It is not lint-free, though: `proguard-tests:build` pulls that module's lint through `check`, and lint **analysis** with it for the library dependencies (`:auth`, `:common`, `:database`, `:firestore`, `:storage`, and `:internal:lintchecks` on a local run) — [lint footprint](../ci-workflows/android.md#proguard-step-lint). Findings are reported for `:proguard-tests` only, so a lint failure during `build.sh` is still that module's gate, not `lintAll`'s. ```bash ./scripts/build.sh @@ -64,7 +64,7 @@ Instrumented `androidTest` (database/firestore) is **not** in CI or the agent al `checkstyle` is scoped `include("**/*.java")` from the root `build.gradle.kts`, so on a Kotlin-only diff it inspects **zero files and exits 0**. A green checkstyle is not evidence for a change in `:auth`, `:app` or `:e2eTest`; `lintAll` is what covers those — [Kotlin blind spot](agent-command-policy.md#checkstyle-kotlin-blind-spot). -`lintAll` runs Android Lint for all 10 Android modules at `checkAllWarnings = true`, `warningsAsErrors = true` and `abortOnError = true` — so any new finding fails the build. It runs in its own workflow ([lint.yml](../ci-workflows/android.md#lint-workflow)), **not** in `build.sh`, so you must run it separately — a green `build.sh` says nothing about lint. Config: the shared policy in the root `build.gradle.kts` sets those flags and the common `disable` set, and a module's own `lint { }` block adds only its module-specific disables; `library/quality/checkstyle.xml` for checkstyle. +`lintAll` runs Android Lint for all 10 Android modules at `checkAllWarnings = true`, `warningsAsErrors = true` and `abortOnError = true` — so any new finding fails the build. It runs in its own workflow ([lint.yml](../ci-workflows/android.md#lint-workflow)), **not** in `build.sh`, so you must run it separately — a green `build.sh` covers only `:proguard-tests`' own findings (see above), never the other nine modules'. Config: the shared policy in the root `build.gradle.kts` sets those flags and the common `disable` set, and a module's own `lint { }` block adds only its module-specific disables; `library/quality/checkstyle.xml` for checkstyle. `auth/lint-baseline.xml` suppresses 180 pre-existing findings. **Never** run `updateLintBaseline` to clear a failure your change caused — [baseline trap](agent-command-policy.md#lint-baseline-trap). @@ -114,7 +114,7 @@ Before closing **`implementation_gate`**, **`review_gate`**, **`commit_gate`**, ## Handoff checklist -- [ ] `./scripts/build.sh` (or equivalent assemble + checkstyle + unit exclusion path) exit 0 +- [ ] `./scripts/build.sh` (or equivalent assemble + R8 + checkstyle + unit exclusion path) exit 0 - [ ] Module evidence per [module validation matrix](#module-validation-matrix) - [ ] `./gradlew checkstyle` when **Java** sources changed - [ ] `./gradlew lintAll` when Kotlin or resources changed in a gated module diff --git a/scripts/build.sh b/scripts/build.sh index b216f04b7..6e406a724 100755 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -8,8 +8,7 @@ cp library/google-services.json proguard-tests/google-services.json ./gradlew $GRADLE_ARGS clean ./gradlew $GRADLE_ARGS assembleDebug -# TODO(thatfiredev): re-enable before release -# ./gradlew $GRADLE_ARGS proguard-tests:build +./gradlew $GRADLE_ARGS proguard-tests:build ./gradlew $GRADLE_ARGS checkstyle # Android Lint is the Kotlin-capable gate, but it runs in its own workflow # (.github/workflows/lint.yml) so it runs in parallel with this path rather than