test: [SDK-4347] add Appium E2E test support to Flutter demo app#1136
test: [SDK-4347] add Appium E2E test support to Flutter demo app#1136fadi-george merged 31 commits intomainfrom
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…ntain permissions' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…ntain permissions' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
@claude review |
|
|
||
| return SectionCard( | ||
| title: 'Send Push Notification', | ||
| sectionKey: 'send_push', | ||
| onInfoTap: onInfoTap, | ||
| child: Column( | ||
| children: [ | ||
| PrimaryButton( | ||
| label: 'SIMPLE', | ||
| semanticsLabel: 'send_simple_button', | ||
| onPressed: () => vm.sendNotification(NotificationType.simple), | ||
| ), | ||
| AppSpacing.gapBox, | ||
| PrimaryButton( | ||
| label: 'WITH IMAGE', | ||
| semanticsLabel: 'send_image_button', | ||
| onPressed: () => vm.sendNotification(NotificationType.withImage), | ||
| ), | ||
| AppSpacing.gapBox, | ||
| PrimaryButton( | ||
| label: 'WITH SOUND', | ||
| semanticsLabel: 'send_sound_button', | ||
| onPressed: () => vm.sendNotification(NotificationType.withSound), | ||
| ), | ||
| AppSpacing.gapBox, | ||
| PrimaryButton( | ||
| label: 'CUSTOM', |
There was a problem hiding this comment.
🔴 The PR migrated snackbar feedback from ViewModel-based _showSnackBar() to context.showSnackBar(), but missed send_push_section.dart and user_section.dart. All four notification buttons in SendPushSection call vm.sendNotification()/vm.sendCustomNotification() as fire-and-forget with no follow-up context.showSnackBar(), so success and failure are completely silent in the UI. Similarly, UserSection calls vm.loginUser() and vm.logoutUser() with no snackbar, dropping the 'Logged in as', 'Logged out', and 'Login failed' messages that existed before. Fix by awaiting the ViewModel calls and calling context.showSnackBar() in each button callback, mirroring the pattern already used in custom_events_section.dart and outcomes_section.dart.
Extended reasoning...
What the bug is and how it manifests
The PR's stated goal is to migrate snackbar feedback from AppViewModel._showSnackBar() to context.showSnackBar() (a BuildContext extension defined in theme.dart). The migration was applied correctly in custom_events_section.dart (calls context.showSnackBar('Event tracked: name')) and outcomes_section.dart (calls context.showSnackBar(snackbarMessage)), but was skipped entirely for send_push_section.dart and user_section.dart. Users who tap any of the four notification buttons or the login/logout buttons now receive zero UI confirmation that their action succeeded or failed.
The specific code path that triggers it
In send_push_section.dart (lines 29, 35, 41, and the custom dialog branch), every button calls vm.sendNotification(type) or vm.sendCustomNotification(...) as a fire-and-forget expression (onPressed: () => vm.sendNotification(...)). The returned Future is discarded. In app_viewmodel.dart, sendNotification() now calls only debugPrint('Notification sent: ...') or debugPrint('Failed to send notification') — no notifyListeners(), no snackbar. In user_section.dart (lines 85 and 94), vm.loginUser(result) and vm.logoutUser() are invoked but no context.showSnackBar() call follows in the section widget.
Why existing code does not prevent it
The old architecture routed feedback through the ViewModel: _showSnackBar() set _snackBarMessage, called notifyListeners(), and HomeScreen consumed the message via addPostFrameCallback. That entire mechanism was removed by this PR. The new architecture requires each section widget to explicitly call context.showSnackBar() after its action completes. custom_events_section.dart and outcomes_section.dart do this correctly; send_push_section.dart and user_section.dart do not. There is no compile-time enforcement — the omission is silently valid Dart.
What the impact would be
Users tapping SIMPLE, WITH IMAGE, WITH SOUND, CUSTOM, LOGIN USER, SWITCH USER, or LOGOUT USER receive no visual confirmation. If a notification fails (e.g., missing API key, network error), the user has no way to know — the old 'Failed to send notification' and 'Login failed' messages are gone. For E2E testing, Appium test suites that assert on snackbar text for these flows will fail with no-element-found errors, directly undermining the PR's goal of enabling automated E2E tests.
Step-by-step proof
- User taps the SIMPLE button in SendPushSection.
- onPressed: () => vm.sendNotification(NotificationType.simple) fires. The Future is discarded — no await, no then().
- AppViewModel.sendNotification() calls _repository.sendNotification(type), receives a bool, then calls debugPrint('Notification sent: ...') or debugPrint('Failed to send notification').
- Control returns to the button handler, which exits. No context.showSnackBar() is ever called.
- The Scaffold's ScaffoldMessenger is never triggered. The user sees nothing.
- Compare: tapping TRACK EVENT in CustomEventsSection awaits the dialog, calls vm.trackEvent(), then calls context.showSnackBar('Event tracked: name') — snackbar appears. This pattern was not replicated in send_push_section.dart or user_section.dart.
How to fix it
For send_push_section.dart, convert each fire-and-forget call to an async callback, await the ViewModel call, and call context.showSnackBar() with an appropriate message inside an if (context.mounted) guard. For user_section.dart, add context.showSnackBar('Logged in as: result') after vm.loginUser(result) and context.showSnackBar('Logged out') after vm.logoutUser(). The ViewModel methods may need to return a success/failure result, or the section can use a try/catch pattern.
There was a problem hiding this comment.
user_section.dart was correctly fixed — both vm.loginUser() and vm.logoutUser() are now awaited with context.showSnackBar() calls after each. However, send_push_section.dart is still unaddressed: all four notification buttons (SIMPLE, WITH IMAGE, WITH SOUND, CUSTOM) still call vm.sendNotification()/vm.sendCustomNotification() as fire-and-forget with no context.showSnackBar() follow-up. For example:
onPressed: () => vm.sendNotification(NotificationType.simple),The Future is discarded and no snackbar is shown on success or failure. To fix, convert each button to an async callback, await the vm call, capture the result (sendNotification returns bool), and call context.showSnackBar() inside an if (context.mounted) guard — mirroring the pattern already applied in user_section.dart, custom_events_section.dart, and outcomes_section.dart.
| ), | ||
| SmsSection( | ||
| onInfoTap: () => _showTooltipDialog(context, 'sms'), | ||
| ), | ||
| TagsSection( | ||
| onInfoTap: () => _showTooltipDialog(context, 'tags'), | ||
| ), | ||
| OutcomesSection( | ||
| onInfoTap: () => _showTooltipDialog(context, 'outcomes'), | ||
| ), | ||
| TriggersSection( | ||
| onInfoTap: () => _showTooltipDialog(context, 'triggers'), | ||
| ), | ||
| CustomEventsSection( |
There was a problem hiding this comment.
🟡 The 'NEXT SCREEN' ElevatedButton at the bottom of home_screen.dart's ListView has no Semantics wrapper and no accessibility identifier, unlike every other interactive element in the PR. Appium tests navigating to SecondaryScreen must fall back to fragile text-based lookup ('NEXT SCREEN') instead of a stable identifier like 'next_screen_button'; fix by wrapping the ElevatedButton in Semantics(identifier: 'next_screen_button', container: true, child: ...).
Extended reasoning...
What the bug is and how it manifests
The PR's stated goal is to add Appium accessibility identifiers to every interactive element in the Flutter demo app. The 'NEXT SCREEN' ElevatedButton at the bottom of home_screen.dart's ListView is the sole navigation control on the home screen and is critical to any E2E flow that tests SecondaryScreen features. Despite this, it was given no Semantics wrapper and no identifier.
The specific code path that triggers it
In home_screen.dart, the widget tree for this button is: Semantics(identifier: 'main_scroll_view') → ListView → ... → Padding(horizontal: 16) → ElevatedButton(child: Text('NEXT SCREEN')). There is no Semantics node with an identifier anywhere between the ListView container and the ElevatedButton itself. The parent main_scroll_view identifier scopes the entire list but provides no per-element identifier for this button.
Why existing code does not prevent it
The PR correctly implements the Semantics wrapping pattern in action_button.dart for PrimaryButton and DestructiveButton:
if (semanticsLabel \!= null) {
button = Semantics(identifier: semanticsLabel, container: true, child: button);
}However, the NEXT SCREEN button is a raw ElevatedButton inlined directly in home_screen.dart rather than using PrimaryButton, so it bypasses the wrapping logic entirely. No other reviewer or automated check flagged the omission.
What the impact would be
An Appium test calling driver.findElement(By.accessibilityId('next_screen_button')) would fail with NoSuchElementException since no such identifier exists. The test author must instead use a text-based locator (e.g., By.name('NEXT SCREEN') on iOS or By.xpath('//android.widget.Button[@text="NEXT SCREEN"]') on Android). Text-based locators are less reliable — they break on any label change, are locale-sensitive, and are slower to evaluate. This is the exact fragility the PR was designed to eliminate.
Step-by-step proof
- home_screen.dart builds a Padding > ElevatedButton widget with child Text('NEXT SCREEN') — confirmed in both the diff and the full modified file.
- Flutter traverses the widget tree to build the accessibility tree; no Semantics node with an identifier wraps this button.
- The accessibility tree contains a button element with no accessibility identifier (only the system-derived label from the button text).
- An Appium test calls driver.findElement(By.accessibilityId('next_screen_button')).
- No matching node → NoSuchElementException. The test falls back to text lookup or fails.
How to fix it
Wrap the ElevatedButton in a Semantics node matching the pattern used throughout the PR:
Semantics(
identifier: 'next_screen_button',
container: true,
child: ElevatedButton(...),
)This is consistent with how action_button.dart wraps PrimaryButton/DestructiveButton and how toggle_row.dart conditionally wraps SwitchListTile.
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
examples/demo/lib/widgets/sections/send_push_section.dart:2-7— Removingimport '../../theme.dart'fromsend_push_section.dartcauses a compile error because the file still usesAppSpacing.gapBoxon five lines (between each button).AppSpacingis defined exclusively intheme.dartand Dart imports are not transitive, so none of the remaining imports expose it. Fix by restoringimport '../../theme.dart'to the file.Extended reasoning...
What the bug is and how it manifests
The PR diff for
send_push_section.dartremoves the lineimport '../../theme.dart'from the import block. The resulting file (confirmed in the modified-files section) has these imports and nothing else:import 'package:flutter/material.dart'; import 'package:provider/provider.dart'; import '../../models/notification_type.dart'; import '../../viewmodels/app_viewmodel.dart'; import '../action_button.dart'; import '../dialogs.dart'; import '../section_card.dart';
Despite the missing import, the file uses
AppSpacing.gapBoxon five separate lines in the widget body — once between each button pair in theColumn.The specific code path that triggers it
AppSpacingis a class defined inexamples/demo/lib/theme.dart(theclass AppSpacing { static const gapBox = SizedBox(height: gap); ... }declaration). Becauseimport '../../theme.dart'was removed, the Dart compiler cannot resolve the nameAppSpacingat compile time, producing the error: "'AppSpacing' isn't defined."Why existing code doesn't prevent it
Two of the remaining imports —
action_button.dartandsection_card.dart— both importtheme.dartinternally. However, Dart imports are file-scoped and non-transitive: a file that importsaction_button.dartdoes NOT gain access totheme.dart's public declarations. Only an explicitimport '../../theme.dart'insend_push_section.dartitself would makeAppSpacingavailable there.What the impact would be
This is a hard compile-time error. The demo app will not build at all —
flutter buildandflutter runboth fail at the analysis/compilation stage with "'AppSpacing' isn't defined" reported for each of the five usages. This blocks the entire PR's stated goal of running Appium E2E tests against a built APK/IPA.Step-by-step proof
send_push_section.dartimports block (modified-files, current HEAD): noimport '../../theme.dart'.- The widget
build()method contains:AppSpacing.gapBox,× 5 (between SIMPLE, WITH IMAGE, WITH SOUND, CUSTOM, and CLEAR ALL buttons). - Dart analysis resolves
AppSpacingby scanning the file's direct imports — none of which export or re-exportAppSpacing. - Compiler emits: Error: 'AppSpacing' isn't defined. (five times, one per usage site).
flutter build apkandflutter build iosboth exit with a non-zero code; the CI artifact upload steps ine2e.ymlare never reached.
How to fix it
Add back the single missing import line:
import '../../theme.dart';
This import was present before the PR (visible as a removed line in the diff) and must be retained since
AppSpacing.gapBoxis used throughout the file.
| Text('App ID', style: Theme.of(context).textTheme.bodyMedium), | ||
| const SizedBox(width: 12), | ||
| Expanded( | ||
| child: SelectableText( | ||
| vm.appId, | ||
| style: Theme.of(context).textTheme.bodySmall?.copyWith( | ||
| fontFamily: 'monospace', | ||
| ), | ||
| textAlign: TextAlign.end, | ||
| child: Semantics( | ||
| identifier: 'app_id_value', | ||
| container: true, | ||
| child: SelectableText( | ||
| _isE2E ? '••••••••-••••-••••-••••-••••••••••••' : vm.appId, | ||
| style: Theme.of(context).textTheme.bodySmall?.copyWith( | ||
| fontFamily: 'monospace', | ||
| ), | ||
| textAlign: TextAlign.end, | ||
| ), | ||
| ), | ||
| ), | ||
| ], |
There was a problem hiding this comment.
🔴 Three ToggleRow widgets omit semanticsLabel and are therefore invisible to Appium: 'Consent Required' and 'Privacy Consent' in app_section.dart, and 'Location Shared' in location_section.dart. Since ToggleRow.build() only wraps in Semantics when semanticsLabel != null, none of these toggles emit an accessibility identifier. Fix by adding semanticsLabel: 'consent_required_toggle', 'privacy_consent_toggle', and 'location_shared_toggle' to the respective ToggleRow calls.
Extended reasoning...
What the bug is and how it manifests
app_section.dart contains two ToggleRow calls — one for 'Consent Required' and one for 'Privacy Consent' — that both omit the semanticsLabel parameter. location_section.dart contains a ToggleRow for 'Location Shared' that also omits semanticsLabel. Because ToggleRow.build() only conditionally wraps the SwitchListTile in a Semantics node (guard: if (semanticsLabel != null) { tile = Semantics(identifier: semanticsLabel, ...) }), these three toggles produce no identifier in the Flutter accessibility tree. Appium element lookup by accessibilityId will find nothing.
The specific code path that triggers it
In app_section.dart the calls are:
ToggleRow(label: 'Consent Required', description: '...', value: vm.consentRequired, onChanged: vm.setConsentRequired)
ToggleRow(label: 'Privacy Consent', description: '...', value: vm.privacyConsentGiven, onChanged: vm.setPrivacyConsent)
In location_section.dart:
ToggleRow(label: 'Location Shared', description: '...', value: vm.locationShared, onChanged: vm.setLocationShared)
None of the three passes semanticsLabel. ToggleRow.build() evaluates the guard, finds null, skips the Semantics wrapping, and returns a bare SwitchListTile.
Why existing code does not prevent it
semanticsLabel is an optional field (String? semanticsLabel) with an implicit null default, so omitting it is valid Dart with no warning. Two other callers — in_app_section.dart (semanticsLabel: 'pause_iam_toggle') and push_section.dart (semanticsLabel: 'push_enabled_toggle') — correctly pass the parameter, confirming the pattern exists and was simply missed for these three toggles.
What the impact would be
Any Appium test that tries to locate these toggles by accessibilityId (e.g., 'consent_required_toggle', 'privacy_consent_toggle', 'location_shared_toggle') receives NoSuchElementException. The PR explicitly states it 'adds semantic labels and accessibility identifiers to all interactive widgets (buttons, inputs, toggles, dialogs, checkboxes, scroll views)' — these three toggles are a direct contradiction of that goal.
Step-by-step proof
- app_section.dart constructs ToggleRow(label: 'Consent Required', value: vm.consentRequired, onChanged: vm.setConsentRequired) — no semanticsLabel argument.
- Dart assigns semanticsLabel = null (the field default).
- ToggleRow.build() creates a SwitchListTile tile, then evaluates if (semanticsLabel != null) → false; no Semantics node is created.
- The accessibility tree contains the SwitchListTile with no enclosing Semantics carrying an identifier.
- Appium queries for element with accessibilityId 'consent_required_toggle' → no match → NoSuchElementException.
- Compare with in_app_section.dart: ToggleRow(..., semanticsLabel: 'pause_iam_toggle', ...) → the guard is true → Semantics(identifier: 'pause_iam_toggle', container: true, child: tile) is returned → Appium finds it correctly.
How to fix it
Add the missing semanticsLabel arguments:
- Consent Required toggle: semanticsLabel: 'consent_required_toggle'
- Privacy Consent toggle: semanticsLabel: 'privacy_consent_toggle'
- Location Shared toggle: semanticsLabel: 'location_shared_toggle'
|
|
||
| return SectionCard( | ||
| title: 'Send In-App Message', | ||
| sectionKey: 'send_iam', | ||
| onInfoTap: onInfoTap, | ||
| child: Column( | ||
| spacing: AppSpacing.gap, |
There was a problem hiding this comment.
🔴 The four IAM-type buttons in send_iam_section.dart (TOP BANNER, BOTTOM BANNER, CENTER MODAL, FULL SCREEN) are built as raw SizedBox+ElevatedButton widgets with no Semantics wrapper, making them invisible to Appium identifier-based lookup. While the PR added sectionKey: "send_iam" to SectionCard, no accessibility identifiers were added to the buttons themselves. Fix by wrapping each SizedBox in Semantics(identifier: "send_${type.name}_button", container: true) or by switching to PrimaryButton with semanticsLabel.
Extended reasoning...
What the bug is and how it manifests
send_iam_section.dart builds all four IAM-type buttons using InAppMessageType.values.map((type) { return SizedBox(width: double.infinity, child: ElevatedButton(...)); }). There is no Semantics wrapper anywhere in this subtree — not on the SizedBox, not on the ElevatedButton, and not inserted by any parent widget. The PR only added sectionKey: "send_iam" to the SectionCard call (visible in the diff), leaving the buttons themselves identifier-free.
The specific code path that triggers it
SectionCard wraps its container in Semantics(identifier: "send_iam_section"), but that node scopes the section, not individual buttons within it. The build loop produces four SizedBox > ElevatedButton widgets. Because these are raw ElevatedButton widgets — not PrimaryButton or DestructiveButton — they bypass the conditional Semantics wrapping introduced in action_button.dart entirely. The semanticsLabel parameter does not exist on raw ElevatedButton.
Why existing code does not prevent it
Every other action section uses PrimaryButton or DestructiveButton (which conditionally wrap in Semantics when semanticsLabel != null). send_iam_section.dart is the only section that constructs interactive action buttons as bare ElevatedButton widgets. The SectionCard container identifier does not substitute for per-button identifiers. No compile-time check enforces identifier coverage.
What the impact would be
All four IAM-send buttons — TOP BANNER, BOTTOM BANNER, CENTER MODAL, FULL SCREEN — are invisible to Appium identifier-based lookup. An Appium test targeting driver.findElement(By.accessibilityId("send_top_banner_button")) will receive NoSuchElementException. This directly contradicts the PRs stated goal of adding accessibility identifiers to all interactive widgets.
Step-by-step proof
- HomeScreen renders SendIamSection.
- SendIamSection.build() calls InAppMessageType.values.map((type) => SizedBox(child: ElevatedButton(...))) with no Semantics wrapper at any level.
- Flutter builds the accessibility tree; each IAM button produces an anonymous button node with no identifier attribute.
- The send_iam_section container node exists (from SectionCard), but contains four unidentified child buttons.
- Appium queries "send_top_banner_button" → no match → NoSuchElementException.
- Compare: send_push_section.dart constructs PrimaryButton(label: "SIMPLE", semanticsLabel: "send_simple_button") → action_button.dart wraps in Semantics(identifier: "send_simple_button") → Appium finds it correctly.
How to fix it
Wrap each SizedBox in a Semantics node: Semantics(identifier: "send_${type.name}_button", container: true, child: SizedBox(...)), or refactor each button to use PrimaryButton with semanticsLabel, consistent with every other action section in the app.
| AppSpacing.gapBox, | ||
| DestructiveButton( | ||
| label: 'CLEAR ALL', | ||
| semanticsLabel: 'clear_all_button', | ||
| onPressed: vm.clearAllNotifications, | ||
| ), | ||
| ], |
There was a problem hiding this comment.
🔴 The CLEAR ALL button in send_push_section.dart passes vm.clearAllNotifications as a bare method reference, and clearAllNotifications() now only calls debugPrint after this PR removed _showSnackBar('All notifications cleared') — so tapping CLEAR ALL is completely silent. Fix by wrapping onPressed in a closure: () { vm.clearAllNotifications(); context.showSnackBar('All notifications cleared'); }, mirroring the pattern in custom_events_section.dart and outcomes_section.dart.
Extended reasoning...
What the bug is and how it manifests
In send_push_section.dart (lines 57–63), the CLEAR ALL DestructiveButton is wired as:
DestructiveButton(
label: 'CLEAR ALL',
semanticsLabel: 'clear_all_button',
onPressed: vm.clearAllNotifications,
),This is a bare method reference, not a closure. In app_viewmodel.dart, clearAllNotifications() now reads:
void clearAllNotifications() {
_repository.clearAllNotifications();
debugPrint('All notifications cleared');
}The old _showSnackBar('All notifications cleared') call was removed by this PR and replaced with debugPrint only. No context.showSnackBar() was added in the widget callback.
The specific code path that triggers it
User taps CLEAR ALL → Flutter calls vm.clearAllNotifications() directly (bare method reference) → _repository.clearAllNotifications() runs → debugPrint fires → method returns. Because the callback is a bare method reference, there is no opportunity to call context.showSnackBar() after the action — there is simply no closure to attach it to.
Why existing code does not prevent it
The old architecture routed feedback through the ViewModel: _showSnackBar() set _snackBarMessage, called notifyListeners(), and HomeScreen consumed it via addPostFrameCallback. That entire mechanism was removed by this PR. The new architecture requires each widget to call context.showSnackBar() explicitly in a closure after the ViewModel action. The button's bare method reference bypasses this entirely.
What the impact would be
A user tapping CLEAR ALL gets zero visual confirmation. The action completes silently — there is no snackbar, no toast, no indication that the clear succeeded or that anything happened at all. The previous behavior gave explicit feedback via _showSnackBar('All notifications cleared'), so this is a user-visible regression.
How to fix it
Change the onPressed from a bare method reference to a closure:
DestructiveButton(
label: 'CLEAR ALL',
semanticsLabel: 'clear_all_button',
onPressed: () {
vm.clearAllNotifications();
context.showSnackBar('All notifications cleared');
},
),This mirrors the pattern used in custom_events_section.dart (context.showSnackBar('Event tracked: $name')) and outcomes_section.dart (context.showSnackBar(snackbarMessage)).
Step-by-step proof
- send_push_section.dart constructs DestructiveButton(onPressed: vm.clearAllNotifications) — a bare method reference.
- User taps CLEAR ALL. Flutter calls vm.clearAllNotifications() directly.
- clearAllNotifications() calls _repository.clearAllNotifications() then debugPrint('All notifications cleared') and returns.
- Control returns to the button; no context.showSnackBar() is ever called — there is no surrounding closure.
- ScaffoldMessenger is never triggered. The user sees nothing.
- Compare with outcomes_section.dart: vm.sendOutcome(name) is called inside a closure that then calls context.showSnackBar(snackbarMessage) — feedback is shown correctly.
| final result = await showDialog<String>( | ||
| context: context, | ||
| builder: (_) => const LoginDialog(), | ||
| ); | ||
| if (result != null && context.mounted) { | ||
| vm.loginUser(result); | ||
| await vm.loginUser(result); | ||
| if (context.mounted) { | ||
| context.showSnackBar('Logged in as $result'); | ||
| } | ||
| } | ||
| }, | ||
| ), | ||
| if (vm.isLoggedIn) ...[ | ||
| AppSpacing.gapBox, | ||
| DestructiveButton( | ||
| label: 'LOGOUT USER', | ||
| onPressed: vm.logoutUser, | ||
| semanticsLabel: 'logout_user_button', | ||
| onPressed: () async { | ||
| await vm.logoutUser(); | ||
| if (context.mounted) { | ||
| context.showSnackBar('User logged out'); |
There was a problem hiding this comment.
🔴 When OneSignal.login() or OneSignal.logout() throws (network error, SDK error), AppViewModel.loginUser()/logoutUser() silently swallow the exception in their catch blocks and return normally — causing user_section.dart to unconditionally display a false success snackbar ('Logged in as X' / 'User logged out') even when the operation failed. Fix by having these methods return a bool success indicator or re-throw on failure, then branching the snackbar message in the widget callback.
Extended reasoning...
What the bug is and how it manifests
AppViewModel.loginUser() and logoutUser() both return Future and catch all exceptions internally without re-throwing. The catch blocks call debugPrint() and return normally. The widget in user_section.dart awaits these methods and then unconditionally shows a success snackbar — there is no error branch.
The specific code path that triggers it
If OneSignal.login() throws (e.g., network error, invalid external ID, SDK initialization failure), the catch block in AppViewModel.loginUser() runs debugPrint('Login error: ') and returns normally. From the widget's perspective, the await completes without error. The if (context.mounted) guard passes and context.showSnackBar('Logged in as $result') is called — displaying a false success message. The same applies to logoutUser() always showing 'User logged out'.
Why existing code does not prevent it
The old architecture (removed by this PR) correctly handled both outcomes via _showSnackBar() inside the ViewModel — the try block called _showSnackBar('Logged in as: X') and the catch block called _showSnackBar('Login failed'). The new architecture requires each section widget to call context.showSnackBar() directly. The PR migrated the success path but never replaced the failure path. Since loginUser()/logoutUser() always return void normally regardless of outcome, the widget has no signal to distinguish success from failure.
What the impact would be
A user whose login fails (network error, SDK error, backend rejection) sees the snackbar 'Logged in as X' while the User section status still shows 'Anonymous' — a direct contradiction in the UI. The user believes they are authenticated when they are not. The failure is completely invisible. For automated E2E tests that assert on snackbar content for failure scenarios, any test expecting 'Login failed' will instead see 'Logged in as X', potentially masking real authentication failures.
Step-by-step proof
- User enters an external user ID and taps LOGIN USER in user_section.dart.
- The widget calls: await vm.loginUser(result) where result = 'test-user'.
- Inside loginUser(), _repository.loginUser('test-user') calls OneSignal.login('test-user'), which throws a network exception.
- The catch block fires: _isLoading = false; notifyListeners(); debugPrint('Login error: ...'). The method returns normally (void). _externalUserId is never set.
- Back in user_section.dart, context.mounted is true, so context.showSnackBar('Logged in as test-user') is called.
- The snackbar displays 'Logged in as test-user'. The User section status card still shows 'Anonymous'. The UI is in a contradictory state.
- The old code path — _showSnackBar('Login failed') in the catch block — was removed by this PR and not replaced.
How to fix it
Have loginUser()/logoutUser() return bool (or re-throw on failure), then branch the snackbar in the widget. For example: have loginUser return true on success and false on failure, then in the widget: context.showSnackBar(success ? 'Logged in as result' : 'Login failed').
| Row( | ||
| children: [ | ||
| Expanded( | ||
| child: AppTextField( | ||
| controller: _keyControllers[i], | ||
| decoration: InputDecoration( | ||
| labelText: widget.keyLabel, | ||
| isDense: true, | ||
| child: Semantics( | ||
| identifier: '${widget.keyLabel}_input_$i', | ||
| container: true, | ||
| child: AppTextField( | ||
| controller: _keyControllers[i], | ||
| decoration: InputDecoration( | ||
| labelText: widget.keyLabel, | ||
| isDense: true, | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| const SizedBox(width: 8), | ||
| Expanded( | ||
| child: AppTextField( | ||
| controller: _valueControllers[i], | ||
| decoration: InputDecoration( | ||
| labelText: widget.valueLabel, | ||
| isDense: true, | ||
| child: Semantics( | ||
| identifier: '${widget.valueLabel}_input_$i', | ||
| container: true, | ||
| child: AppTextField( | ||
| controller: _valueControllers[i], | ||
| decoration: InputDecoration( | ||
| labelText: widget.valueLabel, | ||
| isDense: true, | ||
| ), | ||
| ), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
🔴 Two accessibility identifiers are missing in dialogs.dart: CustomNotificationDialog's Title and Body AppTextField widgets have no Semantics wrapper, making them invisible to Appium when testing the CUSTOM send-push flow. Additionally, MultiPairInputDialog's "Add All" TextButton has no Semantics identifier, preventing Appium from tapping confirm after filling in the multi-alias/tag/trigger rows. Fix by wrapping the CustomNotificationDialog fields in Semantics(identifier: 'custom_notification_title_input'/'custom_notification_body_input', container: true) and the "Add All" button in Semantics(identifier: 'multi_pair_confirm_button', container: true).
Extended reasoning...
What the bugs are and how they manifest
The PR's stated goal is to add Appium accessibility identifiers to all interactive widgets. Two widgets in dialogs.dart were missed: (1) CustomNotificationDialog builds its Title and Body AppTextField widgets with no Semantics wrapper at all — they are completely bare. (2) MultiPairInputDialog's "Add All" confirm TextButton in the actions block has no Semantics wrapper, unlike the per-row input fields in the same dialog which were correctly updated with identifiers ('${widget.keyLabel}_input_$i', '${widget.valueLabel}_input_$i').
The specific code path that triggers each bug
For bug 1: SendPushSection shows a "CUSTOM" button (with semanticsLabel: 'send_custom_button') which calls showDialog opening CustomNotificationDialog. Inside _CustomNotificationDialogState.build(), the content Column contains two bare AppTextField widgets — _titleController labeled "Title" and _bodyController labeled "Body" — with no enclosing Semantics node. Appium can reach the dialog (the button has an identifier) but cannot locate the input fields by accessibilityId.
For bug 2: AliasesSection ("ADD MULTIPLE ALIASES"), TagsSection ("ADD MULTIPLE TAGS"), and TriggersSection ("ADD MULTIPLE TRIGGERS") all open MultiPairInputDialog. The dialog's actions block contains a bare TextButton(child: const Text('Add All'), ...) with no wrapping Semantics node. The row-level inputs DO have identifiers — Appium can fill them — but cannot locate the confirm button.
Why existing code does not prevent these omissions
The PR correctly updated every other dialog: LoginDialog wraps its input in Semantics(identifier: 'login_user_id_input') and its confirm in Semantics(identifier: 'login_confirm_button'); TrackEventDialog wraps both inputs in Semantics; OutcomeDialog wraps its inputs and confirm button; PairInputDialog was updated with confirmSemanticsLabel (defaulting to 'confirm_button'). CustomNotificationDialog and MultiPairInputDialog's confirm button were simply not touched. Since Semantics is optional in Flutter and AppTextField is a plain widget with no built-in identifier enforcement, the omissions compile silently.
What the impact would be
For the custom notification flow: an Appium test that opens the CUSTOM dialog and tries to locate the title or body field by accessibilityId will receive NoSuchElementException. The entire custom notification E2E flow cannot be automated.
For the multi-pair confirm: an Appium test can fill in the rows (Key_input_0, Value_input_0 identifiers exist), but cannot programmatically tap "Add All" — the final step that submits the dialog. The ADD MULTIPLE ALIASES, ADD MULTIPLE TAGS, and ADD MULTIPLE TRIGGERS flows all fail at the confirmation step.
Step-by-step proof for CustomNotificationDialog
- User/Appium taps the CUSTOM button (
send_custom_buttonidentifier exists — dialog opens). _CustomNotificationDialogState.build()rendersAppTextField(controller: _titleController, decoration: InputDecoration(labelText: 'Title'), ...)— noSemanticswrapper.- Flutter builds the accessibility tree; the title field has no identifier node.
- Appium calls
driver.findElement(By.accessibilityId('custom_notification_title_input'))→ no match →NoSuchElementException. - Compare:
LoginDialogwraps identically structuredAppTextFieldinSemantics(identifier: 'login_user_id_input', container: true)— Appium finds it correctly.
Step-by-step proof for MultiPairInputDialog confirm button
- Appium fills Key_input_0 and Value_input_0 (identifiers exist).
MultiPairInputDialogactionsblock:TextButton(onPressed: _allValid ? () \{ ... Navigator.pop(context, pairs); \} : null, child: const Text('Add All'))— noSemanticswrapper.- Flutter builds accessibility tree; button has no identifier.
- Appium calls
driver.findElement(By.accessibilityId('multi_pair_confirm_button'))→ no match → test fails. - Compare:
PairInputDialogwraps its confirm inSemantics(identifier: widget.confirmSemanticsLabel ?? 'confirm_button', container: true)— updated in this exact PR.
3 participants
Description
One Line Summary
Add Appium E2E test support to the Flutter demo app with semantic identifiers, UI improvements, and a shared CI workflow.
Details
Motivation
We need automated E2E testing for the Flutter SDK demo app using Appium. The existing demo app lacked the semantic identifiers and accessibility labels required for Appium to reliably locate and interact with UI elements. This PR prepares the demo app for Appium testing and adds the CI workflow to run those tests.
Scope
sectionKeyacross section widgets so Appium can scope element lookups to specific sectionsLogManagerwithdebugPrintto simplify the demo app (removes log view UI that was not needed for E2E)SwitchListTilefor better accessibility.env.exampleupdate and build docs update.github/workflows/e2e.ymlCI workflow that calls the shared Appium workflowTesting
Manual testing
Tested the demo app on iOS and Android simulators to verify all semantic labels render correctly and do not affect the user-facing UI. Verified the E2E CI workflow triggers and calls the shared workflow correctly.
Affected code checklist
Checklist
Overview
Testing
Final pass
Made with Cursor