ci: run the Dart test suite on pull_request - #8930
Conversation
Nothing ran `flutter test test/` on a PR. The only two references to `flutter test` live in build-linux.yml and build-windows.yml, both workflow_call-only and both scoped to integration_test/ — so the unit suite never executed in CI at all. That is part of why the user_failures deserialization break (#8929) reached three releases: a unit test would have caught it, and a unit test now pins it, but on `pull_request` nothing would have run it. flutter test needs no Go toolchain, Android SDK, or Java, so this gate is just checkout + flutter + pubget/gen + test. Deletes test/widget_test.dart, which had to go first: it is the stock `flutter create` counter template with `pumpWidget` commented out, so it asserts against an empty widget tree and fails unconditionally. It was never adapted to this app and tests nothing. Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a path-filtered GitHub Actions workflow for Flutter tests, including dependency caching, Flutter setup, source generation, environment preparation, and test execution. The existing counter widget smoke test is removed. ChangesFlutter test CI
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a PR-triggered GitHub Actions workflow to run the Flutter/Dart unit + widget test suite under test/, closing a CI gap where only integration_test/ was exercised (and not on pull_request). It also removes the default flutter create template widget test that fails unconditionally so the new CI gate can be green.
Changes:
- Add
.github/workflows/flutter-test.ymlto runmake pubget,make gen, thenflutter teston relevant PR path changes. - Create a placeholder
app.envin CI to satisfy Flutter asset bundling during tests. - Delete
test/widget_test.dart(the unused template test that fails against an empty widget tree).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
test/widget_test.dart |
Removes a default template widget test that fails unconditionally, preventing CI from going green. |
.github/workflows/flutter-test.yml |
Introduces a PR-triggered workflow that runs the Dart/Flutter test/ suite with minimal setup (Flutter + pubget + codegen + tests). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/flutter-test.yml (1)
42-58: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid caching
~/.pub-cachetwice.The explicit cache step and
subosito/flutter-action@v2.22.0withcache: trueboth cache pub dependencies, causing redundant restore/save work. Keep the Flutter action’s cache and remove the explicit cache step, or intentionally disable one layer. (raw.githubusercontent.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/flutter-test.yml around lines 42 - 58, Remove the explicit “Cache Flutter dependencies” actions/cache step from the workflow and retain cache: true on the “Install Flutter” subosito/flutter-action step, ensuring ~/.pub-cache is managed by only one caching mechanism.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/flutter-test.yml:
- Around line 39-40: Update the actions/checkout@v4 step in the workflow to
disable credential persistence by setting persist-credentials to false before
the PR-controlled make pubget, make gen, and flutter test steps; do not add
authenticated Git access unless explicitly required.
---
Nitpick comments:
In @.github/workflows/flutter-test.yml:
- Around line 42-58: Remove the explicit “Cache Flutter dependencies”
actions/cache step from the workflow and retain cache: true on the “Install
Flutter” subosito/flutter-action step, ensuring ~/.pub-cache is managed by only
one caching mechanism.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 87e28a25-4d8a-4388-b523-5578aa7f5a45
📒 Files selected for processing (2)
.github/workflows/flutter-test.ymltest/widget_test.dart
💤 Files with no reviewable changes (1)
- test/widget_test.dart
| - name: Checkout code | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflow excerpt =="
if [ -f .github/workflows/flutter-test.yml ]; then
nl -ba .github/workflows/flutter-test.yml | sed -n '1,120p'
else
echo "missing .github/workflows/flutter-test.yml"
fi
echo
echo "== actions checkout usages with persist-credentials =="
rg -n "actions/checkout|persist-credentials" .github/workflows || true
echo
echo "== read commands after checkout that are likely PR-controlled =="
python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/flutter-test.yml')
if not p.exists():
raise SystemExit
lines = p.read_text().splitlines()
for i,l in enumerate(lines,1):
if i <= 45: continue
if any(s in l for s in ['shell:', 'run:', '- ', '&&', ';']):
print(f"{i}: {l}")
PYRepository: getlantern/lantern
Length of output: 219
🏁 Script executed:
#!/bin/sh
set -euo pipefail
echo "== workflow excerpt =="
if [ -f .github/workflows/flutter-test.yml ]; then
awk '{printf "%6d\t%s\n", NR, $0}' .github/workflows/flutter-test.yml | sed -n '1,140p'
else
echo "missing .github/workflows/flutter-test.yml"
fi
echo
echo "== actions checkout usages with persist-credentials =="
grep -Rn "actions/checkout\|persist-credentials" .github/workflows || true
echo
echo "== commands after checkout that are likely PR-controlled =="
python3 - <<'PY'
from pathlib import Path
p = Path('.github/workflows/flutter-test.yml')
if not p.exists():
raise SystemExit
lines = p.read_text().splitlines()
in_run = False
for i,l in enumerate(lines, 1):
stripped = l.strip()
if stripped.startswith('run:'):
in_run = True
elif l.startswith(' - ') or l.startswith(' - '):
in_run = False
if i <= 45:
continue
if in_run or stripped.startswith('make ') or stripped.startswith('flutter ') or stripped.startswith('//'):
print(f"{i}: {l}")
PYRepository: getlantern/lantern
Length of output: 6010
🌐 Web query:
actions/checkout v4 persist-credentials default true README
💡 Result:
In actions/checkout version 4, the persist-credentials input defaults to true [1][2]. When this setting is enabled (the default), the authentication token or SSH key used by the action is persisted in the local git configuration [3][4]. This allows subsequent steps in your workflow to execute authenticated git commands [3][4]. The action automatically removes this token during the post-job cleanup phase [3][4]. If you wish to prevent the action from configuring your credentials in the local git config, you can explicitly set persist-credentials to false in your workflow file [3][4].
Citations:
- 1: https://github.com/actions/checkout/blob/v4/action.yml
- 2: https://github.com/actions/checkout/blob/v4.1.1/action.yml
- 3: https://github.com/actions/checkout/blob/v4/README.md
- 4: https://github.com/actions/checkout
Disable checkout credential persistence before running PR-controlled code.
actions/checkout@v4 persists GITHUB_TOKEN credentials by default, and this pull_request job later runs PR-controlled make pubget, make gen, and flutter test. Set persist-credentials: false unless this workflow needs authenticated Git access.
Proposed fix
- name: Checkout code
uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| - name: Checkout code | |
| uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 39-40: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/flutter-test.yml around lines 39 - 40, Update the
actions/checkout@v4 step in the workflow to disable credential persistence by
setting persist-credentials to false before the PR-controlled make pubget, make
gen, and flutter test steps; do not add authenticated Git access unless
explicitly required.
Source: Linters/SAST tools
The gap
Nothing in CI ran
flutter test test/. Every pre-existingflutter testcall in the repo, in full:All five target
integration_test/, nevertest/.build-linux.ymlandbuild-windows.ymlareworkflow_call-only; the script is reached from those same jobs.And the workflows that do trigger on
pull_requestare onlygo.yml,android-compile-check.yml, andupdate-issue-status.yml— Go tests, a Kotlin compile gate, and issue-board bookkeeping. No Dart tests. (app-smoke-tests.ymlisworkflow_dispatch-only.)So the 114-test unit suite has never executed on a PR. Tests could rot, or pin a regression that CI would happily let through.
Why now
This is the second half of #8929. lantern-box v0.0.101 changed the JSON shape of
user_failures, which brokeAvailableServers.fromJsonfor every user carrying a recorded failure — and cost them their entire server list. It shipped in v9.1.16-beta and v9.1.17-beta.#8929 added seven tests, five of which fail without the fix. Without this workflow those five are inert: a future change that reintroduced the break would go green.
What this runs
flutter testruns on the Dart VM and builds no platform artifact, so this needs no Go toolchain, no Android SDK, no Java — just checkout, Flutter,make pubget && make gen, and the suite. Six steps.Two details worth calling out:
touch app.env—app.envis a gitignored declared pubspec asset, normally decoded from a secret by the release workflow. Asset bundling runs before any test does and hard-fails without it (No file or variants found for asset: app.env→Failed to build asset bundle). Same empty-placeholder treatment, and same comment, asandroid-compile-check.yml.make gen— generated sources are committed, so this isn't strictly required to get a green run. It's here so the suite tests the PR's own source rather than whatever.g.darthappened to be checked in; a model change with a forgottenmake genwould otherwise pass against stale generated code. It costs ~20s.No
--reporterflag: the Dart test runner already defaults to itsgithubreporter under Actions, which annotates failures inline on the PR.Path filter is
lib/**,test/**,pubspec.{yaml,lock},.github/flutter-version.yaml, plus the usual self-trigger — so this PR exercises its own workflow.Deleting
test/widget_test.dartThis had to go first, or the new job could never be green. It is the stock
flutter createcounter template, and the load-bearing line is commented out:It asserts against an empty widget tree, so it fails unconditionally — verified on
origin/mainat444c1397, which reported+107 -1with this as the only failure (widget_test.dart:17). It was never adapted to this app and there is noMyAppto pump; this repo's root widget isLanternApp. Nothing else references it.Verification
Locally, at the exact commands the workflow runs:
444c13971(main before #8929)+107 -1— onlywidget_test.dartfails6fc4c9705(main after #8929) + this PR+114All tests passed!114 = 107 baseline + the 7 new
user_failures deserializationtests from #8929. Confirmed those seven ran, rather than being silently skipped.Noticed, not fixed here
make genon a cold cache rewriteslib/features/home/provider/home_notifier.g.dart— the committed riverpod content hash is stale relative tohome_notifier.dart:Harmless — it's a hot-reload guard, and the suite passes either way. But it means a
git diff --exit-code"generated files are current" assertion would fail onmaintoday. That'd be a reasonable follow-up gate; it's a separate policy call and out of scope here.🤖 Generated with Claude Code
Summary by CodeRabbit