diff --git a/.github/workflows/ios-tests.yml b/.github/workflows/ios-tests.yml index ef51fdf3..a2ce4ba8 100644 --- a/.github/workflows/ios-tests.yml +++ b/.github/workflows/ios-tests.yml @@ -6,9 +6,50 @@ on: - main - "reconcile/**" pull_request: + # Full regression on demand, for a release candidate or a suspicious change. + workflow_dispatch: + # Nightly, so the suite still runs against trunk every day even when no PR + # touched a surface it covers. + schedule: + - cron: "0 8 * * *" + +# A superseded run is wasted runner time; only the newest commit needs testing. +concurrency: + group: ios-tests-${{ github.head_ref || github.run_number }} + cancel-in-progress: true jobs: + # Decides whether this change can affect anything the Detox suite asserts on. + # The suite is three logged-out specs; every assertion resolves to + # js/product/ProductScreens.js or + # js/screens/HomeScreenComponents/NavigationBar.js, plus the authentication + # start they trigger. Nothing behind sign-in is reachable from it. + surface: + runs-on: ubuntu-latest + outputs: + full: ${{ steps.decide.outputs.full }} + reason: ${{ steps.decide.outputs.reason }} + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: 0 + + - id: decide + env: + EVENT: ${{ github.event_name }} + LABELS: ${{ join(github.event.pull_request.labels.*.name, ',') }} + BASE: ${{ github.event.pull_request.base.sha }} + HEAD: ${{ github.event.pull_request.head.sha }} + run: ./scripts/ci-detox-surface.sh >> "$GITHUB_OUTPUT" + + - name: Report decision + run: | + echo "full detox: ${{ steps.decide.outputs.full }}" >> "$GITHUB_STEP_SUMMARY" + echo "reason: ${{ steps.decide.outputs.reason }}" >> "$GITHUB_STEP_SUMMARY" + ios-iphone-tests: + needs: surface + if: needs.surface.outputs.full == 'true' runs-on: macos-15 steps: - name: Checkout repository @@ -66,6 +107,8 @@ jobs: path: artifacts ios-ipad-tests: + needs: surface + if: needs.surface.outputs.full == 'true' runs-on: macos-15 steps: - name: Checkout repository diff --git a/.github/workflows/native-gates.yml b/.github/workflows/native-gates.yml new file mode 100644 index 00000000..c8678550 --- /dev/null +++ b/.github/workflows/native-gates.yml @@ -0,0 +1,51 @@ +name: Native Gates + +on: + push: + branches: + - main + - "reconcile/**" + pull_request: + +concurrency: + group: native-gates-${{ format('{0}-{1}', github.head_ref || github.run_number, github.job) }} + cancel-in-progress: true + +jobs: + static-gates: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Set up Node.js + uses: actions/setup-node@v3 + with: + node-version: 24 + cache: yarn + + - name: Yarn install + run: yarn + + # These gates guard the OTA channel wiring, the runtime version, the iOS + # auth presentation contract and release readiness. They ran only on a + # developer's machine until now, which meant the cheapest and most + # safety-relevant checks were the ones CI did not enforce. + - name: OTA readiness + if: ${{ !cancelled() }} + run: yarn verify:ota + + - name: iOS auth presentation + if: ${{ !cancelled() }} + run: yarn verify:ios-auth + + - name: Release readiness + if: ${{ !cancelled() }} + run: yarn verify:release-readiness + + - name: Backend contract + if: ${{ !cancelled() }} + run: yarn verify:backend + + - name: OTA lane validation + if: ${{ !cancelled() }} + run: yarn validate:ota diff --git a/docs/NATIVE-CI-POLICY.md b/docs/NATIVE-CI-POLICY.md new file mode 100644 index 00000000..e2138f15 --- /dev/null +++ b/docs/NATIVE-CI-POLICY.md @@ -0,0 +1,67 @@ +# Native CI policy + +## What the Detox suite actually covers + +**The Detox suite exercises the logged-out shell only.** It is three specs, and +every assertion in it resolves to one of two screens: + +| Assertion | Owning source file | +| --- | --- | +| `logged-out-welcome-scroll` | `js/product/ProductScreens.js` | +| "Members only", "Member sign in", "Signing in…", "Invitation-only membership" | `js/product/ProductScreens.js` | +| `nav-plus-icon` | `js/screens/HomeScreenComponents/NavigationBar.js` | + +It signs in to nothing. It therefore provides **no coverage** of notifications, +notification routing, the OTP session bootstrap, the authenticated WebView, +member profiles, avatars, private member photos, rate limiting, chat, or +anything else behind authentication. Those surfaces are covered by Jest, by the +static native gates, and by physical-device certification. + +Do not cite a green Detox run as evidence that an authenticated surface works. +It cannot be. + +## Which checks run on which change + +Every pull request runs the fast gates: + +- ESLint and Prettier +- Jest +- `verify:ota`, `verify:ios-auth`, `verify:release-readiness`, + `verify:backend`, `validate:ota` + +The full iPhone + iPad Detox regression is roughly 100 minutes of macOS runner +time per iteration. It runs only when the change can affect something the suite +can actually observe: + +- `ios/`, `android/` +- `e2e/`, `.detoxrc*` +- `.github/workflows/` +- `package.json`, `yarn.lock`, `app.json`, `eas.json`, `Gemfile*`, and the + `*.config.*` roots +- `js/product/ProductScreens.js`, `js/screens/HomeScreen*`, + `js/Discourse.js`, `js/iosAuthSession.js`, `js/site_manager.js` +- the `full-detox` label, `workflow_dispatch`, or the nightly schedule +- any case where the changed-file list cannot be computed — this **fails + closed** to the full suite + +`scripts/ci-detox-surface.sh` makes the decision and writes its reasoning to the +job summary, so every run states why Detox ran or was skipped. + +## Overrides + +`full-detox` forces the suite on. `skip-full-detox` forces it off and is an +explicit owner override, not a routine way around classification: use it only +when the classification is demonstrably wrong for a specific change, and say why +in the pull request. + +A pull request stacked on an unmerged branch inherits its parent's changed files +and will usually run the full suite. That is the conservative outcome and is +left as-is; the durable fix is landing the lineage rather than loosening the +rule. + +## What was deliberately not changed + +Detox assertions, matchers, the 180s hook budget, `--retries 2`, and the +failure-artifact upload are untouched. A failing test still fails the job, and +failures remain fully visible. This policy changes *when* the suite runs, never +what it asserts or how loudly it fails. diff --git a/scripts/ci-detox-surface.sh b/scripts/ci-detox-surface.sh new file mode 100755 index 00000000..849be925 --- /dev/null +++ b/scripts/ci-detox-surface.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +# Decides whether a change needs the full iPhone+iPad Detox regression. +# +# The Detox suite is three logged-out specs. Every assertion in it resolves to +# js/product/ProductScreens.js or js/screens/HomeScreenComponents/NavigationBar.js, +# plus the authentication start those screens trigger. It cannot reach anything +# behind sign-in. So a JS change that cannot touch the logged-out shell gets no +# signal from ~100 minutes of runner time, and is gated instead by lint, Jest, +# the static native gates, and the physical-device certification step. +# +# Writes `full=` and `reason=` for $GITHUB_OUTPUT. Fails closed: if the changed +# file list cannot be computed, run the full regression. +set -euo pipefail + +emit() { echo "full=$1"; echo "reason=$2"; } + +case "${EVENT:-}" in + workflow_dispatch) emit true "manual full regression requested"; exit 0 ;; + schedule) emit true "nightly full regression"; exit 0 ;; +esac + +# An explicit owner decision outranks any path rule, in both directions. +case ",${LABELS:-}," in + *,skip-full-detox,*) emit false "owner label skip-full-detox"; exit 0 ;; + *,full-detox,*) emit true "owner label full-detox"; exit 0 ;; +esac + +if [ -z "${BASE:-}" ] || [ -z "${HEAD:-}" ]; then + emit true "changed-file list unavailable; failing closed to the full suite" + exit 0 +fi +# Merge-base, not a two-dot diff, so a PR is judged on what it adds. Note that +# a PR stacked on an unmerged branch still inherits its parent's files here; it +# will run the full suite, and skip-full-detox is the escape hatch. +MERGE_BASE=$(git merge-base "$BASE" "$HEAD" 2>/dev/null || echo "$BASE") +if ! CHANGED=$(git diff --name-only "$MERGE_BASE" "$HEAD" 2>/dev/null) || [ -z "$CHANGED" ]; then + emit true "changed-file list unavailable; failing closed to the full suite" + exit 0 +fi + +# Native, runtime, harness, and the specific JS files the Detox specs assert on. +TRIGGERS='^(ios|android|e2e)/ +^\.detoxrc +^\.github/workflows/ +^(package\.json|yarn\.lock|app\.json|eas\.json)$ +^(Gemfile|app\.config\.|metro\.config\.|babel\.config\.|react-native\.config\.) +^js/product/ProductScreens\.js$ +^js/screens/HomeScreen +^js/(Discourse|iosAuthSession|site_manager)\.js$' + +if MATCHED=$(echo "$CHANGED" | grep -E "$(echo "$TRIGGERS" | paste -sd'|' -)" | head -5); then + emit true "native, runtime or logged-out-shell surface changed: $(echo "$MATCHED" | paste -sd',' -)" +else + emit false "no native, runtime or logged-out-shell surface changed" +fi diff --git a/testing/native-ci-policy/CLASSIFIER-VALIDATION.md b/testing/native-ci-policy/CLASSIFIER-VALIDATION.md new file mode 100644 index 00000000..ffe1fbf4 --- /dev/null +++ b/testing/native-ci-policy/CLASSIFIER-VALIDATION.md @@ -0,0 +1,50 @@ +# Surface classifier validation — 2026-09-09 + +Run of `scripts/ci-detox-surface.sh` against representative historical diffs on +the working trunk, required before merging the policy change. + +``` +--- expected TRUE: native / runtime / harness / logged-out shell --- +PR#2 pin runtime dependencies true files=2 native, runtime or logged-out-shell surface changed: package.json,yarn.lock +close dependency findings true files=12 native, runtime or logged-out-shell surface changed: .github/workflows/ios-tests.yml,.github/workflows/jest-tests.yml,.github/workflows/linting.yml,js/site_manager.js,package.json +remove Ionicons native decl true files=2 native, runtime or logged-out-shell surface changed: ios/Discourse/Info.plist +iOS share extension host true files=2 native, runtime or logged-out-shell surface changed: ios/ShareExtension/ShareViewController.swift +forward custom URLs to RN true files=2 native, runtime or logged-out-shell surface changed: ios/Discourse/AppDelegate.swift +PR#17 CI detox hygiene true files=3 native, runtime or logged-out-shell surface changed: .github/workflows/ios-tests.yml,e2e/jest.config.js,e2e/loggedOutLaunch.js +PR#16 notification routing true files=3 native, runtime or logged-out-shell surface changed: js/Discourse.js +PR#19 authenticated WebView true files=7 native, runtime or logged-out-shell surface changed: js/Discourse.js +--- expected FALSE: authenticated-surface JS / docs only --- +PR#14 docs certification record false files=3 no native, runtime or logged-out-shell surface changed +ratelimit cooldown split false files=6 no native, runtime or logged-out-shell surface changed +PR#11 private member-photo false files=2 no native, runtime or logged-out-shell surface changed +onboarding policy + launch gates false files=8 no native, runtime or logged-out-shell surface changed +PR#7 native search queries false files=4 no native, runtime or logged-out-shell surface changed +``` + +All 13 classifications are correct. + +Native, runtime, CI-harness and logged-out-shell changes all select the full +suite. Authenticated-surface changes — rate limiting, private member photos, +onboarding, native search — and docs-only changes skip it, because the Detox +suite signs in to nothing and could not have observed them. + +Two results are worth recording explicitly: + +- `PR#2 pin runtime dependencies` was first tested with the commit pair + inverted, so the merge-base diff was empty. The script returned + `full=true, reason=changed-file list unavailable; failing closed to the full + suite`. That is the fail-closed path behaving correctly on a degenerate + input, and it is why the fallback exists. +- `onboarding policy + launch gates` classifies false. That is correct: + `js/product/AdjusterCardOnboardingScreen.js` is post-authentication + onboarding, which the logged-out suite cannot reach. + +Scenario coverage beyond the historical diffs: + +``` +EVENT=schedule -> full=true nightly full regression +EVENT=workflow_dispatch -> full=true manual full regression requested +LABELS=full-detox -> full=true owner label full-detox +LABELS=skip-full-detox -> full=false owner label skip-full-detox +BASE/HEAD unset -> full=true failing closed to the full suite +```