fix(testing): make the E2E suite green on a fresh clone - #36
Merged
Conversation
With the native harness working (#30), the shipped test failed for real: it taps login and asserts e2e_home_content, but the sample auth flow POSTs to BASE_URL and no server exists in CI or on a fresh clone. Every fork inherited a red run. A starter whose E2E suite is red by construction teaches people to ignore it - which is how the four-month-red main in #11 happened. Each E2E file now holds two tests: - A smoke test that always runs: the app boots to a usable login screen and the form accepts input. Backend-free, but not a token test - reaching a rendered login screen exercises the native Patrol harness, app bootstrap, the Riverpod scope, the router, and localization. - The authenticated flow, gated on `skip: !hasBackend` where `hasBackend = bool.fromEnvironment('E2E_BACKEND')`. Teams with an API run `patrol test --dart-define=E2E_BACKEND=true`. Skipping keeps the flow visible: it reports as `⏩ Skipped: 1` rather than disappearing, so nobody forgets it exists. Applied to both root files and all six tool/golden/*/integration_test/ counterparts, so strip-smoke stays consistent per the CLAUDE.md rule. no_feature_flags keeps its richer auth -> tasks -> create-task flow behind the same gate. Refs #33
The first attempt still failed, and the reason was more interesting than the
symptom. The smoke test reported "Found 0 widgets with key", so I checked
whether the login button renders conditionally - it does not - and then found
the real cause:
void main() async { ... }
`main()` returns **void**, not Future<void>, so the future produced by its four
startup awaits (EnvConfig.load, storage init, migrations, saved locale) is
unreachable. `app.main()` in a test could not be awaited at all: pumpAndSettle
settled an EMPTY widget tree and the assertion ran before runApp was called.
The original test hid this behind `if ($(#e2e_login_submit).exists)`, so it
looked like an auth failure when the app had not started.
Changed to `Future<void> main() async`, which is the correct signature for an
async entrypoint and is what Flutter supports. Two benefits beyond the tests:
an error thrown by those awaits now propagates instead of becoming an
unhandled async error.
The tests now `await app.main()` and additionally poll with
`$(#e2e_login_submit).waitUntilVisible()` rather than trusting a single
pumpAndSettle on a cold emulator.
Applied to lib/main.dart and all three tool/golden/*/lib/main.dart
counterparts, plus all eight integration_test files.
Refs #33
The re-run got further and failed differently, which named the real cause: pumpAndSettle timed out at app_e2e_test.dart:29 (the await $.pumpAndSettle() after await app.main()) The smoke test ran 131s rather than 13s, so awaiting main() worked - the app boots. `pumpAndSettle` demands an idle frame, and on a real device this tree never reaches one, so it throws before any assertion runs. Ruled out by reading, not guessed: `isLoading` defaults to false so no spinner renders on login, lib/ contains no AnimationController and no Timer.periodic outside Debouncer, and the auth notifier's build() returns a const state without a network call. So the tests no longer use pumpAndSettle at all. `waitUntilVisible` polls instead of demanding quiescence, which is the primitive Patrol provides for exactly this, and each tap now waits on its next target rather than on global idleness. Applied to both root files and all six golden counterparts. Why the tree never settles is a separate question worth answering - it will bite anyone writing a widget test against the full app - and is filed separately rather than guessed at here. Refs #33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With the native harness working (#30), the shipped test failed for real:
It taps login and asserts
e2e_home_content, but the sample auth flow POSTs toBASE_URLand no server exists in CI or on a fresh clone. Every fork inheriteda red run. A starter whose E2E suite is red by construction teaches people to
ignore it - which is exactly how the four-month-red
mainin #11 went unnoticed.Each E2E file now holds two tests
1. Smoke, always runs, no backend. The app boots to a usable login screen and
the form accepts input. Not a token test - reaching a rendered login screen
exercises the native Patrol harness, app bootstrap, the Riverpod scope, the
router, and localization. If any of those break, this goes red.
2. The authenticated flow, skipped by default.
Skipping rather than deleting keeps it visible as
⏩ Skipped: 1in the summary,so the gap is legible instead of forgotten.
Golden tree kept in lockstep
All six
tool/golden/{stripped,no_tasks,no_feature_flags}/integration_test/counterparts got the same treatment, per the CLAUDE.md rule that changing a file
with a
tool/golden/*counterpart means updating the counterpart too.no_feature_flagskeeps its richer auth -> open tasks -> create task flow, behindthe same gate.
clearPackageData = true(added in #30) makes the smoke test deterministic: eachrun starts with no persisted session, so the app reliably routes to login.
Verification of criteria 1 and 2 is a dispatched E2E run, reported on the issue.
Refs #33