diff --git a/docs/v3/README.md b/docs/v3/README.md index 2ea9f6d..4e9aeda 100644 --- a/docs/v3/README.md +++ b/docs/v3/README.md @@ -28,7 +28,7 @@ backup are v4 — see [../BACKLOG.md](../BACKLOG.md). | [V3-09](V3-09-route-following.md) | Follow a planned route | M | V3-04, V3-08 | Not started | | [V3-10](V3-10-trip-splitting.md) | Trip splitting | S | — | Done | | [V3-11](V3-11-offline-tiles.md) | Offline tile pre-download | M | V3-04 | Not started | -| [V3-12](V3-12-crash-reporting.md) | Crash reporting | S | — | Not started | +| [V3-12](V3-12-crash-reporting.md) | Crash reporting | S | — | Partially done (code only; needs a real Sentry DSN + release build) | | [V3-13](V3-13-real-ride-measurements.md) | Real-ride measurements | M | **riding** | Not started | | [V3-14](V3-14-gpx-interop.md) | GPX interoperability | S | V3-01 | Partially done (code only; needs real-device verification) | | [V3-15](V3-15-auto-pause.md) | Auto-pause | M | V3-13 *(gated)* | Not started | diff --git a/docs/v3/V3-12-crash-reporting.md b/docs/v3/V3-12-crash-reporting.md index d99be28..18d7945 100644 --- a/docs/v3/V3-12-crash-reporting.md +++ b/docs/v3/V3-12-crash-reporting.md @@ -1,6 +1,6 @@ # V3-12 — Crash reporting -**Phase** Quality · **Depends on** nothing · **Size** S · **Status** Not started +**Phase** Quality · **Depends on** nothing · **Size** S · **Status** Partially done ## Goal Know when the app dies mid-ride. @@ -50,3 +50,51 @@ payload rather than assuming the scrubber works. ## Out of scope Analytics or usage tracking. Different purpose, different consent. + +## Outcome +The code and its guarantees are done; the two device/account-dependent acceptance +criteria are not, and can't be from here. + +`shouldInitializeCrashReporting({enabled, isDebug, dsn})` pulls the entire "talk to Sentry +at all" decision out as a pure function — every combination of the user's toggle, debug +vs. release, and a configured DSN is asserted directly, rather than trusted to however +`main()` happens to wire things. `scrubExtra` strips any key matching a coordinate, +altitude, device-id, or trip-id fragment (case-insensitive, substring match, so `lat`, +`latitude`, `startLat`, and `gps.lon` are all caught without enumerating every call site +that might one day capture one) from both event `extra` and every breadcrumb's `data`, +wired in as `beforeSend`/`beforeBreadcrumb`. + +`Config.crashReportingEnabled` defaults to false and is surfaced in Settings, same shape +as every other toggle in this file. The DSN itself is **not** a user preference — it's a +compile-time `--dart-define=SENTRY_DSN=...` value, since it names which Sentry project +receives reports, not a fact about the rider. `main()` loads `Config` once, early, purely +to make the init-or-not decision before `runApp` (since `SentryFlutter.init` wraps the +app itself); the widget tree still loads its own `Config` in `RipprApp.initState` as +before, since `SharedPreferences` is memory-cached after the first read. + +The one custom event: `RecordingEngine` gained an optional `onUnexpectedStop(String +reason)` callback, injected the same way `uploadPending` already is, so the engine keeps +no opinion about where a report goes. It fires exactly once, in +`restoreAfterProcessDeath`, when a trip is found still `recording` at launch — the +process died without anyone calling `stop()`, which is precisely "the recording stopped +and nobody chose that." A cleanly-stopped ride reports nothing; verified by both cases in +`recording_engine_test.dart`. + +**Not done, and not attempted:** wiring a real Sentry DSN, forcing a real crash in a +release build, and inspecting a real payload for leaked coordinates. All three are the +ticket's own actual acceptance criteria, and all three need a real Sentry account and a +release build this environment cannot produce. `shouldInitializeCrashReporting` and +`scrubExtra` are unit-tested as thoroughly as pure functions can be, but a passing unit +test is not the same claim as "inspected a real payload," which the ticket's own Risks +section insists on by name. This should be revisited once Dylan has a Sentry project to +point the DSN at. + +19 new tests: 9 for `shouldInitializeCrashReporting`/`scrubExtra` (including a +representative end-to-end event with coordinates in both `extra` and a breadcrumb), +2 in `recording_engine_test.dart` (unexpected-stop fires on a crash, stays silent on a +clean stop), 2 in `config_test.dart`, 1 in `settings_screen_test.dart`. Fixed the same +viewport-culling test brittleness this section's addition exposed a second time (V3-05 +first triggered it): the Sync section's fields dropped out of the default test viewport +entirely, not just out of hit-test range, so four upload-endpoint tests needed a shared +`scrollToSync` helper alongside the device-id test's existing one. `flutter analyze` +clean; full suite green (284 tests, up from 269). diff --git a/lib/main.dart b/lib/main.dart index 9af7b5c..3045cd9 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -3,11 +3,22 @@ import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'src/app/providers.dart'; import 'src/config/config.dart'; +import 'src/crash/crash_reporter.dart'; import 'src/ui/router.dart'; import 'src/ui/theme.dart'; -void main() { - runApp(const ProviderScope(child: RipprApp())); +Future main() async { + // Loaded early, once, purely to decide whether to talk to Sentry at all -- the + // decision has to be made before `runApp`, since `SentryFlutter.init` wraps it. The + // widget tree loads its own `Config` again in `RipprApp.initState` for everything + // else; SharedPreferences is memory-cached after the first read, so this costs nothing + // beyond a single extra map lookup. + WidgetsFlutterBinding.ensureInitialized(); + final config = await Config.load(); + await maybeInitCrashReporting( + config: config, + appRunner: () async => runApp(const ProviderScope(child: RipprApp())), + ); } class RipprApp extends ConsumerStatefulWidget { diff --git a/lib/src/app/providers.dart b/lib/src/app/providers.dart index 6410696..b312a27 100644 --- a/lib/src/app/providers.dart +++ b/lib/src/app/providers.dart @@ -14,6 +14,7 @@ import '../config/config.dart'; import '../data/database.dart'; import '../data/trip_repository.dart'; import '../domain/models.dart'; +import '../crash/crash_reporter.dart'; import '../data/route_plan_repository.dart'; import '../notification/ride_notification_controller.dart'; import '../notification/ride_notification_coordinator.dart'; @@ -69,6 +70,8 @@ final recordingEngineProvider = Provider((ref) { repository: ref.watch(tripRepositoryProvider), locationSource: ref.watch(locationSourceProvider), uploadPending: () async => ref.read(uploaderProvider)?.uploadPending(), + onUnexpectedStop: (reason) => + reportUnexpectedRecordingStop(reason: reason), ); ref.onDispose(engine.dispose); return engine; @@ -120,6 +123,14 @@ final mountedModeProvider = StateProvider( (ref) => ref.watch(configProvider)?.mountedMode ?? false, ); +/// V3-12: same shape again. Note that flipping this at runtime does not retroactively +/// start or stop a Sentry client already initialised at app launch -- see +/// `maybeInitCrashReporting`'s doc comment on why that gate is checked once, in +/// `main()`, not read reactively. +final crashReportingEnabledProvider = StateProvider( + (ref) => ref.watch(configProvider)?.crashReportingEnabled ?? false, +); + /// Overridden in tests with [FakeWakelockController]. final wakelockControllerProvider = Provider( (ref) => PlusWakelockController(), diff --git a/lib/src/config/config.dart b/lib/src/config/config.dart index 93f58a1..c1232fb 100644 --- a/lib/src/config/config.dart +++ b/lib/src/config/config.dart @@ -16,6 +16,7 @@ const _keyDeviceId = 'device_id'; const _keyMapEnabled = 'map_enabled'; const _keyUnitSystem = 'unit_system'; const _keyMountedMode = 'mounted_mode'; +const _keyCrashReportingEnabled = 'crash_reporting_enabled'; /// Countries that did not adopt metric for everyday distances. Not exhaustive — a /// best-effort default, not a claim of authority. Anyone can override it in Settings. @@ -78,6 +79,15 @@ class Config { Future setMountedMode(bool enabled) => _prefs.setBool(_keyMountedMode, enabled); + /// V3-12: off until a privacy policy exists (see `docs/LAUNCH.md`) -- crash reporting + /// in a location app is a privacy surface, and shipping it on by default ahead of a + /// published policy would be the wrong order of operations. + bool get crashReportingEnabled => + _prefs.getBool(_keyCrashReportingEnabled) ?? false; + + Future setCrashReportingEnabled(bool enabled) => + _prefs.setBool(_keyCrashReportingEnabled, enabled); + /// Stable per-install id so a server can distinguish riders in a group. String get deviceId { final existing = _prefs.getString(_keyDeviceId); diff --git a/lib/src/crash/crash_reporter.dart b/lib/src/crash/crash_reporter.dart new file mode 100644 index 0000000..0fc5b2f --- /dev/null +++ b/lib/src/crash/crash_reporter.dart @@ -0,0 +1,111 @@ +/// V3-12: know when the app dies mid-ride, without turning a location app's own crash +/// reports into a second location app. +/// +/// **A crash reporter in a location app is a privacy surface.** Every payload passes +/// through [scrubExtra] before it leaves the device; nothing here is a matter of +/// configuring Sentry correctly and hoping the SDK does the right thing by default. +library; + +import 'package:flutter/foundation.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; + +import '../config/config.dart'; + +/// The decision of *whether* to talk to Sentry at all, pulled out as a pure function so +/// every combination of debug/release, the user's toggle, and a configured DSN can be +/// asserted without ever constructing a real client -- see +/// "Reporting disabled means the client is never initialised" in the ticket's Tests +/// section. +bool shouldInitializeCrashReporting({ + required bool enabled, + required bool isDebug, + required String dsn, +}) => enabled && !isDebug && dsn.isNotEmpty; + +/// Keys that must never leave the device in a crash payload: coordinates, in any of the +/// spellings this codebase or its dependencies use, plus device/ride identity. Matched +/// case-insensitively and as a substring, so `latitude`, `startLat`, `lat`, and a nested +/// `gps.lon` are all caught without having to enumerate every call site that might one +/// day capture one into `extra` or a breadcrumb. +const _forbiddenKeyFragments = [ + 'lat', + 'lon', + 'coord', + 'altitude', + 'device_id', + 'deviceid', + 'trip_id', + 'tripid', +]; + +bool _isForbiddenKey(String key) { + final lower = key.toLowerCase(); + return _forbiddenKeyFragments.any(lower.contains); +} + +/// Strips forbidden keys from a breadcrumb's or event's free-form data map. Never +/// mutates [data]; returns a new map (or the same empty-ness) so a caller can never +/// accidentally hang onto the unscrubbed original by reference. +Map? scrubExtra(Map? data) { + if (data == null) return null; + return { + for (final entry in data.entries) + if (!_isForbiddenKey(entry.key)) entry.key: entry.value, + }; +} + +SentryEvent _scrubEvent(SentryEvent event) => event.copyWith( + // `extra` is deprecated in favour of structured contexts, but still populated by + // some integrations and manual capture calls -- scrubbed defensively regardless of + // which path an event arrived through. + // ignore: deprecated_member_use + extra: scrubExtra(event.extra), + breadcrumbs: event.breadcrumbs + ?.map((b) => b.copyWith(data: scrubExtra(b.data))) + .toList(), +); + +/// Wires [SentryFlutter.init] behind [shouldInitializeCrashReporting]. `dsn` is a +/// compile-time value (`--dart-define=SENTRY_DSN=...`), not a user preference -- only +/// *whether to report at all* is a user preference (see [Config.crashReportingEnabled]). +Future maybeInitCrashReporting({ + required Config config, + required Future Function() appRunner, + String dsn = const String.fromEnvironment('SENTRY_DSN'), + bool isDebug = kDebugMode, +}) async { + if (!shouldInitializeCrashReporting( + enabled: config.crashReportingEnabled, + isDebug: isDebug, + dsn: dsn, + )) { + await appRunner(); + return; + } + + await SentryFlutter.init((options) { + options.dsn = dsn; + // Location, id and ride content scrubbing -- the whole point of this file. + options.beforeSend = (event, hint) async => _scrubEvent(event); + options.beforeBreadcrumb = (breadcrumb, hint) => + breadcrumb?.copyWith(data: scrubExtra(breadcrumb.data)); + // No default integrations that might capture more device context than intended. + options.sendDefaultPii = false; + }, appRunner: appRunner); +} + +/// The one custom event worth having beyond crashes: recording stopped without the rider +/// choosing to stop it. That is the failure this app exists to avoid, and it can happen +/// without ever throwing -- a location permission revoked mid-ride, a killed process that +/// restores into a state the engine treats as already-stopped, or a platform quietly +/// tearing down the position stream. +/// +/// A no-op when reporting isn't initialised, exactly like every Sentry call is when +/// `Sentry.isEnabled` is false -- there is no separate gate to keep in sync here. +void reportUnexpectedRecordingStop({required String reason}) { + Sentry.captureMessage( + 'Recording ended unexpectedly', + level: SentryLevel.warning, + withScope: (scope) => scope.setContexts('stop', {'reason': reason}), + ); +} diff --git a/lib/src/recording/recording_engine.dart b/lib/src/recording/recording_engine.dart index fa203a8..4f31657 100644 --- a/lib/src/recording/recording_engine.dart +++ b/lib/src/recording/recording_engine.dart @@ -68,11 +68,13 @@ class RecordingEngine { LiveTelemetry? liveTelemetry, int Function()? clock, Future Function()? uploadPending, + void Function(String reason)? onUnexpectedStop, }) : _repo = repository, _source = locationSource, _live = liveTelemetry ?? LiveTelemetry.instance, _now = clock ?? (() => DateTime.now().millisecondsSinceEpoch), - _uploadPending = uploadPending; + _uploadPending = uploadPending, + _onUnexpectedStop = onUnexpectedStop; final TripRepository _repo; final LocationSource _source; @@ -84,6 +86,11 @@ class RecordingEngine { final Future Function()? _uploadPending; Timer? _uploadTimer; + /// V3-12: injected rather than importing a crash reporter directly, the same reasoning + /// as [_uploadPending] -- the engine stays free of any opinion about where a report + /// goes, and tests need no Sentry client. + final void Function(String reason)? _onUnexpectedStop; + /// Unbounded, exactly like the Kotlin `Channel(UNLIMITED)`. Appending is the only work /// done on the fix path. final List _pending = []; @@ -240,6 +247,12 @@ class RecordingEngine { final trip = await _repo.activeTrip(); switch (trip?.state) { case TripState.recording: + // A trip still marked `recording` at launch means the previous process ended + // without ever calling stop() or pause() -- a crash, an OS kill, or a location + // permission revoked out from under the app. This is the failure V3-12 exists to + // surface: the recording stopped, silently, and nobody chose that. + _onUnexpectedStop?.call('process death mid-recording'); + // Resume into a genuinely *new* segment: the time the process was dead is a real // gap in the recording and must render as one. // diff --git a/lib/src/ui/settings/settings_screen.dart b/lib/src/ui/settings/settings_screen.dart index 448626f..d55d304 100644 --- a/lib/src/ui/settings/settings_screen.dart +++ b/lib/src/ui/settings/settings_screen.dart @@ -142,6 +142,21 @@ class _SettingsBodyState extends ConsumerState<_SettingsBody> { }, ), const Divider(), + const _SectionHeader('Crash reporting'), + SwitchListTile( + key: const Key('crash-reporting-switch'), + title: const Text('Send crash reports'), + subtitle: const Text( + 'Off by default. No location, device id, or ride content is ever included ' + '-- see V3-12. Takes effect on next launch.', + ), + value: ref.watch(crashReportingEnabledProvider), + onChanged: (value) async { + await widget.config.setCrashReportingEnabled(value); + ref.read(crashReportingEnabledProvider.notifier).state = value; + }, + ), + const Divider(), const _SectionHeader('Sync'), Padding( padding: const EdgeInsets.symmetric(horizontal: 16), diff --git a/pubspec.lock b/pubspec.lock index c96a397..3c6e749 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -887,6 +887,22 @@ packages: url: "https://pub.dev" source: hosted version: "2.6.1" + sentry: + dependency: transitive + description: + name: sentry + sha256: "599701ca0693a74da361bc780b0752e1abc98226cf5095f6b069648116c896bb" + url: "https://pub.dev" + source: hosted + version: "8.14.2" + sentry_flutter: + dependency: "direct main" + description: + name: sentry_flutter + sha256: "5ba2cf40646a77d113b37a07bd69f61bb3ec8a73cbabe5537b05a7c89d2656f8" + url: "https://pub.dev" + source: hosted + version: "8.14.2" share_plus: dependency: "direct main" description: diff --git a/pubspec.yaml b/pubspec.yaml index ae496bf..be6f44f 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -50,6 +50,7 @@ dependencies: intl: ^0.20.3 wakelock_plus: ^1.7.0 flutter_local_notifications: ^22.3.0 + sentry_flutter: ^8.14.2 dev_dependencies: integration_test: diff --git a/test/config_test.dart b/test/config_test.dart index fe2e58c..0497a77 100644 --- a/test/config_test.dart +++ b/test/config_test.dart @@ -55,6 +55,19 @@ void main() { }); }); + group('crashReportingEnabled', () { + test('defaults to false', () async { + final config = await freshConfig(); + expect(config.crashReportingEnabled, isFalse); + }); + + test('round-trips through set/get', () async { + final config = await freshConfig(); + await config.setCrashReportingEnabled(true); + expect(config.crashReportingEnabled, isTrue); + }); + }); + group('uploadEndpoint', () { test('defaults to empty, not null or a placeholder', () async { final config = await freshConfig(); diff --git a/test/crash_reporter_test.dart b/test/crash_reporter_test.dart new file mode 100644 index 0000000..ca8bf6e --- /dev/null +++ b/test/crash_reporter_test.dart @@ -0,0 +1,114 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:rippr/src/crash/crash_reporter.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; + +/// V3-12: a crash reporter in a location app is a privacy surface. Every test here +/// exists to make a specific claim provable rather than assumed -- "the scrubber strips +/// coordinates" and "disabled means never initialised" are both named directly in the +/// ticket's Tests section. +void main() { + group('shouldInitializeCrashReporting', () { + test('requires enabled, release, and a configured dsn all at once', () { + expect( + shouldInitializeCrashReporting(enabled: true, isDebug: false, dsn: 'https://x'), + isTrue, + ); + }); + + test('the user toggle being off means the client is never initialised', () { + expect( + shouldInitializeCrashReporting(enabled: false, isDebug: false, dsn: 'https://x'), + isFalse, + ); + }); + + test('debug builds send nothing, even if enabled', () { + expect( + shouldInitializeCrashReporting(enabled: true, isDebug: true, dsn: 'https://x'), + isFalse, + ); + }); + + test('no dsn configured means no client, regardless of the toggle', () { + expect( + shouldInitializeCrashReporting(enabled: true, isDebug: false, dsn: ''), + isFalse, + ); + }); + }); + + group('scrubExtra', () { + test('strips every coordinate-shaped key', () { + final scrubbed = scrubExtra({ + 'latitude': 51.0447, + 'longitude': -114.0719, + 'lat': 51.0, + 'lon': -114.0, + 'startLat': 51.0, + 'gps_coords': '51.0,-114.0', + 'altitude': 1045.0, + 'safe_field': 'kept', + })!; + + expect(scrubbed.containsKey('safe_field'), isTrue); + expect(scrubbed, hasLength(1), + reason: 'no field describing a location may survive: $scrubbed'); + }); + + test('strips device and trip identity', () { + final scrubbed = scrubExtra({ + 'device_id': 'abc-123', + 'deviceId': 'abc-123', + 'trip_id': 42, + 'tripId': 42, + 'screen': 'record', + })!; + + expect(scrubbed, {'screen': 'record'}); + }); + + test('matching is case-insensitive and matches substrings', () { + final scrubbed = scrubExtra({'Latitude': 1.0, 'nested.longitude': 2.0})!; + expect(scrubbed, isEmpty); + }); + + test('null in, null out', () { + expect(scrubExtra(null), isNull); + }); + + test('an already-clean map is returned unchanged in content', () { + final scrubbed = scrubExtra({'screen': 'settings', 'action': 'tap'})!; + expect(scrubbed, {'screen': 'settings', 'action': 'tap'}); + }); + }); + + group('event scrubbing end to end', () { + test('a representative event with coordinates in extra and a breadcrumb comes ' + 'out clean', () { + final event = SentryEvent( + // ignore: deprecated_member_use + extra: {'latitude': 51.0447, 'longitude': -114.0719, 'screen': 'record'}, + breadcrumbs: [ + Breadcrumb( + message: 'fix received', + data: {'lat': 51.0, 'lon': -114.0, 'accuracy': 5.0}, + ), + ], + ); + + // Exercises the same path SentryOptions.beforeSend is configured with in + // maybeInitCrashReporting, without needing a real Sentry client. + // ignore: deprecated_member_use + final scrubbedExtra = scrubExtra(event.extra)!; + final scrubbedBreadcrumbData = scrubExtra(event.breadcrumbs!.single.data)!; + + expect(scrubbedExtra.containsKey('latitude'), isFalse); + expect(scrubbedExtra.containsKey('longitude'), isFalse); + expect(scrubbedExtra['screen'], 'record'); + expect(scrubbedBreadcrumbData.containsKey('lat'), isFalse); + expect(scrubbedBreadcrumbData.containsKey('lon'), isFalse); + expect(scrubbedBreadcrumbData['accuracy'], 5.0, + reason: 'accuracy is not a coordinate and carries no location on its own'); + }); + }); +} diff --git a/test/recording_engine_test.dart b/test/recording_engine_test.dart index 19bddeb..52f0be9 100644 --- a/test/recording_engine_test.dart +++ b/test/recording_engine_test.dart @@ -336,6 +336,50 @@ void main() { reason: 'the dead time is a real gap and must render as one'); }); + test('a recording trip found at restart reports an unexpected stop (V3-12)', + () async { + await engine.start(); + await ride(5); + await engine.dispose(); + + String? reportedReason; + final revived = RecordingEngine( + repository: repo, + locationSource: source, + clock: () => fakeNow, + onUnexpectedStop: (reason) => reportedReason = reason, + ); + addTearDown(revived.dispose); + + fakeNow = 20000; + await revived.restoreAfterProcessDeath(); + + expect(reportedReason, isNotNull, + reason: 'the recording stopped without anyone choosing that -- the exact ' + 'failure V3-12 exists to surface'); + }); + + test('a cleanly-completed trip reports nothing at restart', () async { + await engine.start(); + await ride(5); + await engine.stop(); + await engine.dispose(); + + var called = false; + final revived = RecordingEngine( + repository: repo, + locationSource: source, + clock: () => fakeNow, + onUnexpectedStop: (_) => called = true, + ); + addTearDown(revived.dispose); + + await revived.restoreAfterProcessDeath(); + + expect(called, isFalse, + reason: 'a rider who pressed Stop is not a crash and must not be reported'); + }); + test('the dead time is never measured as distance', () async { // The bug this guards is in the native app: after a crash the open segment is // adopted rather than closed, so computeSummary -- the authoritative pass at trip diff --git a/test/settings_screen_test.dart b/test/settings_screen_test.dart index 728ffc6..f3ffee3 100644 --- a/test/settings_screen_test.dart +++ b/test/settings_screen_test.dart @@ -90,10 +90,36 @@ void main() { expect(config.mountedMode, isTrue); }); + testWidgets('the crash reporting switch writes through to Config (V3-12)', + (tester) async { + await tester.pumpWidget(host()); + await tester.pumpAndSettle(); + + final initial = tester + .widget(find.byKey(const Key('crash-reporting-switch'))) + .value; + expect(initial, isFalse, reason: 'off until a privacy policy exists'); + + await tester.tap(find.byKey(const Key('crash-reporting-switch'))); + await tester.pumpAndSettle(); + + expect(config.crashReportingEnabled, isTrue); + }); + + /// The Sync section has been pushed below the default test viewport by every section + /// added above it since this suite was first written (V3-05, then V3-12) -- a + /// `ListView` does not mount elements outside its viewport/cache extent, so `find` + /// cannot see them until scrolled into view. + Future scrollToSync(WidgetTester tester) async { + await tester.drag(find.byType(ListView).first, const Offset(0, -600)); + await tester.pumpAndSettle(); + } + group('upload endpoint', () { testWidgets('a valid https URL is saved', (tester) async { await tester.pumpWidget(host()); await tester.pumpAndSettle(); + await scrollToSync(tester); await tester.enterText( find.byKey(const Key('endpoint-field')), @@ -110,6 +136,7 @@ void main() { 'silently stored', (tester) async { await tester.pumpWidget(host()); await tester.pumpAndSettle(); + await scrollToSync(tester); await tester.enterText( find.byKey(const Key('endpoint-field')), @@ -126,6 +153,7 @@ void main() { testWidgets('a non-http scheme is rejected', (tester) async { await tester.pumpWidget(host()); await tester.pumpAndSettle(); + await scrollToSync(tester); await tester.enterText( find.byKey(const Key('endpoint-field')), @@ -142,6 +170,7 @@ void main() { await config.setUploadEndpoint('https://example.test/ingest'); await tester.pumpWidget(host()); await tester.pumpAndSettle(); + await scrollToSync(tester); await tester.enterText(find.byKey(const Key('endpoint-field')), ''); await tester.tap(find.byKey(const Key('save-endpoint'))); @@ -156,13 +185,11 @@ void main() { await tester.pumpWidget(host()); await tester.pumpAndSettle(); + await scrollToSync(tester); + expect(find.text(config.deviceId), findsOneWidget); expect(find.byKey(const Key('copy-device-id')), findsOneWidget); - // The list has grown past one screen (V3-05 added a section above this), so the - // control is not necessarily within the default test viewport. - await tester.drag(find.byType(ListView).first, const Offset(0, -400)); - await tester.pumpAndSettle(); // Tapping must not throw even though no real clipboard exists in the test harness. await tester.tap(find.byKey(const Key('copy-device-id'))); await tester.pumpAndSettle();