From 7ae4c134074db222bbadb98503f77b90b5235051 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sat, 26 Sep 2026 19:36:32 +0200 Subject: [PATCH] CI under 60 s: uv, xdist, split pre-commit, sharded Flutter coverage Tests (~214 s, gated by flutter-coverage): - Flutter suite runs as 8 parallel buckets of whole test files (check_flutter_coverage.sh --shard I/N); flutter-coverage merges their lcov files (--merged N) and applies the unchanged 100% bar and every-lib-file-present guard. `flutter test --total-shards` splits test cases inside each file, so every shard would still compile all 77 files. Proved locally: the merged lcov matches an unsharded run line for line (3312/3312), and a missing shard fails. flutter-action caches the SDK. - pytest: uv install, -n auto -q -p no:sugar (local: 25.9 s -> 10.6 s, 100% coverage both ways). - lock-popup-safety: uv, and no apt step -- xvfb-run and setup-python's tkinter are already on the runner (screens-fit relies on both already). pre-commit (~122 s): matrix of mypy / pylint / flutter-analyze / rest, each installing only its own toolchain, with a per-group hook cache. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0188kZdx3bb56QM8JFUnbHuM --- .github/workflows/pre-commit.yml | 88 ++++++++++++++--- .github/workflows/python-tests.yml | 75 +++++++++++++-- scripts/check_flutter_coverage.sh | 147 +++++++++++++++++++++++++---- 3 files changed, 270 insertions(+), 40 deletions(-) diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index 03bfe30..fda0511 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -7,8 +7,31 @@ on: branches: [main] jobs: + # One ~120 s job used to install Flutter, Node and all of requirements.txt + # and then run every hook in series; mypy (~20 s), flutter analyze (~13 s + # plus the SDK) and pylint (~12 s) dominated it. Each now gets its own runner + # with only the toolchain it needs, and the other hooks share `rest`; the + # workflow takes as long as the slowest group rather than the sum. pre-commit: runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - group: mypy + hooks: mypy + skip: "" + # Both pylint hooks (source and tests) share the id `pylint`. + - group: pylint + hooks: pylint + skip: "" + - group: flutter + hooks: flutter-analyze + skip: "" + - group: rest + hooks: "" + skip: mypy,pylint,flutter-analyze + name: pre-commit (${{ matrix.group }}) steps: - uses: actions/checkout@v4 @@ -26,32 +49,69 @@ jobs: - uses: actions/setup-python@v5 with: python-version: "3.x" - # The flutter-analyze pre-commit hook shells out to `flutter analyze` in + - uses: astral-sh/setup-uv@v10.2.0 + with: + enable-cache: true + cache-dependency-glob: requirements.txt + + # The flutter-analyze hook shells out to `flutter analyze` in # stronglift_replacement/workout_app. Pin to the version the app is # developed against (Dart SDK ^3.12.2) so CI's analyzer matches local. - uses: subosito/flutter-action@v2 + if: matrix.group == 'flutter' with: flutter-version: 3.47.5 channel: stable cache: true - # The web-lint / web-typecheck / web-coverage hooks run npm inside web/. - # Without its installed packages `tsc --build` cannot find the - # 'vite/client' and 'node' type libraries and fails with TS2688 -- which - # is what this workflow had been doing on every run since the status UI - # landed in 8f91158. + # analyze needs resolved packages (.dart_tool); the hook runs analyze + # only, so fetch deps here first. + - name: Resolve Flutter packages + if: matrix.group == 'flutter' + run: flutter pub get + working-directory: stronglift_replacement/workout_app + + # The web-lint / web-typecheck hooks run npm inside web/. Without its + # installed packages `tsc --build` cannot find the 'vite/client' and + # 'node' type libraries and fails with TS2688 -- which is what this + # workflow had been doing on every run since the status UI landed in + # 8f91158. - uses: actions/setup-node@v4 + if: matrix.group == 'rest' with: node-version: "24.21.0" cache: npm cache-dependency-path: web/package-lock.json - name: Install status-UI packages + if: matrix.group == 'rest' run: npm ci working-directory: web - - name: Install dependencies - run: pip install -r requirements.txt - # analyze needs resolved packages (.dart_tool); the hook runs analyze - # only, so fetch deps here first. - - name: Resolve Flutter packages - run: flutter pub get - working-directory: stronglift_replacement/workout_app - - uses: pre-commit/action@v3.0.1 + + # Every hook brings its own environment except the language: system + # ones, and those all run in `rest`: screens-fit imports screen_locker + # (and so its runtime deps) and dependency-freshness needs pyyaml and + # packaging. The other groups need pre-commit alone. uv with a cached + # wheel store replaces a ~27 s pip install. + - name: Install requirements for the system hooks + if: matrix.group == 'rest' + run: uv pip install --system -r requirements.txt pyyaml packaging + - name: Install pre-commit + if: matrix.group != 'rest' + run: uv pip install --system pre-commit + + # One cache per group: a shared key would hold whichever group saved + # first, and the others would rebuild their hook envs on every run. The + # restore key falls back to the old single pre-commit/action cache, + # which already holds every hook env, so a group's first run does not + # rebuild pylint's git dependencies from scratch. + - uses: actions/cache@v4 + with: + path: ~/.cache/pre-commit + key: pre-commit-${{ matrix.group }}-${{ env.pythonLocation }}-${{ hashFiles('.pre-commit-config.yaml') }} + restore-keys: | + pre-commit-${{ matrix.group }}-${{ env.pythonLocation }}- + pre-commit-3|${{ env.pythonLocation }}| + + - name: pre-commit run (${{ matrix.group }}) + env: + SKIP: ${{ matrix.skip }} + run: pre-commit run ${{ matrix.hooks }} --all-files --show-diff-on-failure --color=always diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index f758ba3..6e5d00c 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -6,6 +6,10 @@ on: pull_request: branches: [main] +env: + # Bucket count for the Flutter suite; the matrix below lists 0..N-1. + FLUTTER_SHARDS: 8 + jobs: test: runs-on: ubuntu-latest @@ -17,11 +21,22 @@ jobs: with: python-version: "3.x" - - name: Install dependencies - run: pip install -r requirements.txt + # uv with a cached wheel store installs requirements.txt in seconds; + # plain pip took ~26 s of a ~70 s job. + - uses: astral-sh/setup-uv@v10.2.0 + with: + enable-cache: true + cache-dependency-glob: requirements.txt + - name: Install dependencies + run: uv pip install --system -r requirements.txt + + # -n auto spreads the suite over every runner core (pytest-xdist is in + # requirements.txt); pytest-cov merges the workers' coverage, and every + # worker loads conftest.py, so the state-file redirects hold in each. + # -q cancels pyproject's -v: 1368 test names were most of the log. - name: Run tests with coverage - run: python -m pytest -v + run: python -m pytest -n auto -p no:sugar -q # The pytest suite replaces the whole `tk` module so it can never reach a # display, which means the lock-screen widget behaviour it guards cannot be @@ -35,15 +50,23 @@ jobs: - uses: actions/setup-python@v5 with: python-version: "3.x" + - uses: astral-sh/setup-uv@v10.2.0 + with: + enable-cache: true + cache-dependency-glob: requirements.txt + # No apt step: the runner image already ships xvfb-run, and + # setup-python's interpreter has its own tkinter (python3-tk is for the + # system python, which this job never runs). The apt-get update + + # install cost ~35 s. If either ever goes missing the check does not + # skip -- _xvfb_reexec returns 1 without xvfb-run, and the import fails + # without tkinter. - name: Install dependencies run: | - sudo apt-get update - sudo apt-get install -y xvfb python3-tk - pip install -r requirements.txt + uv pip install --system -r requirements.txt # The script imports screen_locker directly rather than through # pytest, which would put the rootdir on sys.path for it. - pip install -e . --no-deps + uv pip install --system -e . --no-deps - name: Sport selector survives gatelock's recovery tick # Run as a module (like the screen-fits gate) so scripts/ resolves as a @@ -54,18 +77,52 @@ jobs: # which became a 1-second stamp check on 2026-09-20 (pre-commit's manual # stage holds them now). Nothing else in CI runs the Flutter or web suites, # so these jobs are the only place the 100% bars are enforced on every push. - flutter-coverage: + # + # The Flutter suite took ~150 s on one runner, nearly all of it compiling 77 + # test files, so it runs as FLUTTER_SHARDS buckets of whole files in + # parallel and flutter-coverage applies the unchanged 100% bar and + # every-lib-file-present guard to their union. + flutter-shard: runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + shard: [0, 1, 2, 3, 4, 5, 6, 7] + name: flutter test (shard ${{ matrix.shard }}) steps: - uses: actions/checkout@v4 + # cache: true keeps the SDK and ~/.pub-cache between runs; without it + # the SDK download alone was ~57 s. - uses: subosito/flutter-action@v2 with: flutter-version: 3.47.5 channel: stable + cache: true + + - name: flutter test --coverage, bucket ${{ matrix.shard }} + run: bash scripts/check_flutter_coverage.sh --shard "${{ matrix.shard }}/$FLUTTER_SHARDS" + + - uses: actions/upload-artifact@v4 + with: + name: flutter-lcov-${{ matrix.shard }} + path: stronglift_replacement/workout_app/coverage/lcov.info + if-no-files-found: error + retention-days: 1 + + flutter-coverage: + needs: flutter-shard + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/download-artifact@v4 + with: + pattern: flutter-lcov-* + path: lcov-shards - name: flutter test --coverage, 100% of lib/ (workout app) - run: bash scripts/check_flutter_coverage.sh + run: bash scripts/check_flutter_coverage.sh --merged "$FLUTTER_SHARDS" lcov-shards/*/lcov.info web-coverage: runs-on: ubuntu-latest diff --git a/scripts/check_flutter_coverage.sh b/scripts/check_flutter_coverage.sh index 4402404..06411e4 100755 --- a/scripts/check_flutter_coverage.sh +++ b/scripts/check_flutter_coverage.sh @@ -6,30 +6,143 @@ # file — the 100% would be fake without this check). # 2. Line coverage across all lib/ files is below 100%. # -# Usage: scripts/check_flutter_coverage.sh +# Usage: +# scripts/check_flutter_coverage.sh whole suite, then both checks +# scripts/check_flutter_coverage.sh --shard I/N test bucket I of N (0-based) +# only; leaves its lcov.info, +# checks nothing +# scripts/check_flutter_coverage.sh --merged N LCOV... +# union N shards' lcov files +# into coverage/lcov.info, +# then both checks +# +# Why shards: the suite took ~150 s on one 4-core runner, almost all of it +# compiling 77 test files. `flutter test --total-shards` cannot help -- it +# splits the test CASES inside every file, so each shard still compiles all +# 77 -- hence buckets of whole files, one per CI runner, and a fan-in job that +# runs the same two checks on their union. set -euo pipefail APP_DIR="stronglift_replacement/workout_app" LCOV_INFO="$APP_DIR/coverage/lcov.info" -cd "$APP_DIR" +die() { + echo "ERROR: $*" >&2 + exit 1 +} -echo "Running flutter test --coverage ..." -# env -u GIT_DIR: git exports it to hooks, and the flutter tool shells out to -# git to identify its own SDK. With it set, flutter reads THIS repository as -# the SDK -- reporting our HEAD as the framework revision -- and pub then -# resolves every version constraint against "0.0.0-unknown" and fails. -# FLUTTER_TEST_CONCURRENCY: one test isolate per core is the default and peaks -# well over 4 GiB on this suite, which the 4 GiB resource cap (capped.sh) -# OOM-kills. Set it to 1-2 when running under the cap; unset keeps the default. -concurrency=() -if [[ -n "${FLUTTER_TEST_CONCURRENCY:-}" ]]; then - concurrency=(--concurrency "$FLUTTER_TEST_CONCURRENCY") -fi -env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE \ - flutter test --coverage "${concurrency[@]}" +# Runs `flutter test --coverage` in the app, on the given test files (none = +# the whole suite). +run_flutter_tests() { + echo "Running flutter test --coverage ..." + # env -u GIT_DIR: git exports it to hooks, and the flutter tool shells out to + # git to identify its own SDK. With it set, flutter reads THIS repository as + # the SDK -- reporting our HEAD as the framework revision -- and pub then + # resolves every version constraint against "0.0.0-unknown" and fails. + # FLUTTER_TEST_CONCURRENCY: one test isolate per core is the default and + # peaks well over 4 GiB on this suite, which the 4 GiB resource cap + # (capped.sh) OOM-kills. Set it to 1-2 under the cap; unset keeps the default. + local concurrency=() + if [[ -n "${FLUTTER_TEST_CONCURRENCY:-}" ]]; then + concurrency=(--concurrency "$FLUTTER_TEST_CONCURRENCY") + fi + (cd "$APP_DIR" && env -u GIT_DIR -u GIT_WORK_TREE -u GIT_INDEX_FILE \ + flutter test --coverage "${concurrency[@]}" "$@") +} -cd - > /dev/null +# Prints bucket INDEX of TOTAL: every TOTAL-th test file, in a fixed sort +# order, so the N buckets partition the suite exactly. +shard_files() { + local index=$1 total=$2 i=0 file + while IFS= read -r file; do + if (( i % total == index )); then + printf '%s\n' "$file" + fi + i=$(( i + 1 )) + done < <(cd "$APP_DIR" && find test -name '*_test.dart' | LC_ALL=C sort) +} + +# Writes to OUT the union of the given lcov files: per source file, every +# line any shard instrumented, hit if any shard hit it -- which is what one +# unsharded run reports, since flutter merges its test isolates the same way. +# LF/LH are recomputed from that union rather than summed, because a lib file +# loaded by two shards would otherwise count its lines twice. +merge_lcov() { + local out=$1 file line sf="" rest ln hits key lf lh + shift + local -A hit=() lines_of=() seen=() + local -a order=() + for file in "$@"; do + while IFS= read -r line; do + case $line in + SF:*) + sf=${line#SF:} + if [[ -z ${seen[$sf]:-} ]]; then + seen[$sf]=1 + order+=("$sf") + fi + ;; + DA:*) + rest=${line#DA:} + ln=${rest%%,*} + rest=${rest#*,} + hits=${rest%%,*} + key="$sf|$ln" + if [[ -z ${hit[$key]:-} ]]; then + hit[$key]=$hits + lines_of[$sf]+="$ln " + else + hit[$key]=$(( hit[$key] + hits )) + fi + ;; + esac + done < "$file" + done + mkdir -p "$(dirname "$out")" + for sf in "${order[@]}"; do + echo "SF:$sf" + lf=0 + lh=0 + for ln in ${lines_of[$sf]:-}; do + echo "DA:$ln,${hit[$sf|$ln]}" + lf=$(( lf + 1 )) + if (( hit[$sf|$ln] > 0 )); then + lh=$(( lh + 1 )) + fi + done + echo "LF:$lf" + echo "LH:$lh" + echo "end_of_record" + done > "$out" +} + +case "${1:-}" in + "") + run_flutter_tests + ;; + --shard) + [[ ${2:-} =~ ^([0-9]+)/([1-9][0-9]*)$ ]] || die "--shard wants I/N, got '${2:-}'" + index=${BASH_REMATCH[1]} + total=${BASH_REMATCH[2]} + (( index < total )) || die "shard index $index is not below $total" + mapfile -t files < <(shard_files "$index" "$total") + (( ${#files[@]} > 0 )) || die "shard $index/$total has no test files" + echo "Shard $index/$total: ${#files[@]} test files" + run_flutter_tests "${files[@]}" + exit 0 + ;; + --merged) + [[ ${2:-} =~ ^[1-9][0-9]*$ ]] || die "--merged wants a shard count first" + expected=$2 + shift 2 + # A shard whose artifact went missing would quietly shrink the union. + (( $# == expected )) || die "expected $expected shard lcov files, got $#" + merge_lcov "$LCOV_INFO" "$@" + ;; + *) + die "unknown argument '$1' (see the usage at the top of $0)" + ;; +esac if [[ ! -f "$LCOV_INFO" ]]; then echo "ERROR: $LCOV_INFO not found — did flutter test run?" >&2