Skip to content

Fix Tip Jar test and add error handling - #6168

Open
michelinewu wants to merge 2 commits into
masterfrom
mw_fix_tip_jar_test
Open

Fix Tip Jar test and add error handling#6168
michelinewu wants to merge 2 commits into
masterfrom
mw_fix_tip_jar_test

Conversation

@michelinewu

Copy link
Copy Markdown
Contributor

Fix the Flaky Tip Jar Test and Add Error Handling for Failed Widget Data Fetches

Issues

The Tip Jar test was flaky/hanging for two independent reasons that this branch fixes together:

The widget settings window never recovered from a failed data fetch. WidgetModule.load() in useWidget.tsx ran const data = await this.fetchData(); this.setData(data); this.setPrevSettings(data); this.state.setIsLoading(false); with no error handling. If fetchData() rejected — a transient API failure, exactly the kind of thing that shows up under CI load — setIsLoading(false) was never reached, so the widget window was stuck on an infinite loading spinner with no feedback to the user. In the test, this meant addSource('The Jar', ...) could hand back a child window that never finishes loading, so every subsequent selector wait against it times out.

The "active" jar image had no stable selector. ImagePickerInput.tsx marked the selected option only via a CSS-module class (cx(styles.imageOption, p.value === opt.value && styles.active)). The old test located the selected martini glass with the compound selector .active img[src="...glass-martini.png"], which is brittle — it depends on the parent's class and can race with the click handler updating p.value.

Fixes

  • app/components-react/widgets/common/useWidget.tsx — wrapped the fetch in try/catch/finally. On success, behavior is unchanged. On failure, logs a console.error with the widget name and elapsed time, then shows an alertAsync telling the user to reopen the settings window or re-add the widget (closing the window afterward via afterCloseFn: this.close). The finally block always calls this.state.setIsLoading(false), so the spinner can no longer get stuck regardless of outcome.
  • app/services/widgets/widgets-data.ts — added getWidgetName(widgetType), looking up WidgetDefinitions[widgetType].name and falling back to $t('Widget') (with a console.error if the type is unknown). Used to build the new error message.
  • app/i18n/en-US/widgets.json — added the translation string for the new alert.
  • app/components-react/shared/inputs/ImagePickerInput.tsx — added data-name (image-option-active when selected, image-option-<value> otherwise) so the active option can be selected without depending on the hashed CSS-module class, and switched the cx call to its object form for the same class list.
  • test/regular/widgets/tip-jar.ts — rewritten to use addSource('The Jar', 'The Jar', false, true) (see below), wait for the "Jar Image" tab, use 15s timeouts on the glass-image waits, select the active state via the new [data-name="image-option-active"] selector instead of the old compound one, and explicitly closeWindow('child') at the end.
  • test/helpers/modules/sources.tsaddSource's 4th parameter was previously named audioSource and unused inside the function body (dead parameter). It's renamed to waitForResponse and given real behavior: when true, sleep 1s before calling focusChild(), giving sources whose settings window depends on an async data fetch (e.g. Tip Jar) time to load before the test starts interacting with it.
  • test/helpers/modules/forms/form.tsgetInputControllers() now passes a 15s timeout to waitForDisplayed(formSelector) instead of the default, consistent with the other lengthened timeouts in this change.

Files changed: app/components-react/shared/inputs/ImagePickerInput.tsx, app/components-react/widgets/common/useWidget.tsx, app/i18n/en-US/widgets.json, app/services/widgets/widgets-data.ts, test/helpers/modules/forms/form.ts, test/helpers/modules/sources.ts, test/regular/widgets/tip-jar.ts

Performance Implications

None on the happy path. The try/catch/finally around widget data fetch adds negligible overhead, and the error branch (console log + alert) only runs when a fetch already failed. On the e2e side, tests gain fixed delays — the 1s sleep in addSource when waitForResponse is true, and the larger 15s timeouts — which slightly slows those specific tests in exchange for removing the flakiness that was causing them to hang or fail outright.

Notes

addSource's 4th argument is also passed as true for the "Audio Input Capture" cases in test/regular/filters.ts. That argument was previously unused (dead audioSource parameter), so those calls had no effect; now that it's wired to waitForResponse, those filter tests will also pick up the 1s pre-focusChild sleep because these source settings windows also have a lag when loading data from the backend.

Copilot AI lite review requested due to automatic review settings September 9, 2026 17:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The updated Tip Jar test currently waits for a generic “active” option that may already exist by default, so it may not actually verify that clicking the martini image changed the selection.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Improves robustness of widget settings loading (specifically Tip Jar) by adding error handling around widget data fetches, and updates e2e selectors/timeouts to reduce flakiness in the Tip Jar test.

Changes:

  • Add try/catch/finally around widget settings fetch to avoid infinite loading state and to notify the user on failures.
  • Add getWidgetName(widgetType) helper and a new i18n string used in the widget-load failure alert.
  • Stabilize the Tip Jar e2e flow via a new data-name selector on ImagePickerInput, longer waits, and explicit window close.
File summaries
File Description
app/components-react/shared/inputs/ImagePickerInput.tsx Adds stable data-name markers for selected/non-selected image options.
app/components-react/widgets/common/useWidget.tsx Adds error handling + user alert for failed widget settings fetch; always clears loading state.
app/i18n/en-US/widgets.json Adds translation string for the new widget-load error alert.
app/services/widgets/widgets-data.ts Adds getWidgetName helper for user-facing error messages.
test/helpers/modules/forms/form.ts Increases form visibility wait timeout to reduce test flakiness.
test/helpers/modules/sources.ts Adds optional delay before refocusing the child window when keeping properties open.
test/regular/widgets/tip-jar.ts Updates Tip Jar test to use the new stable selector and longer timeouts; closes child window explicitly.
Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/regular/widgets/tip-jar.ts
@bundlemon

bundlemon Bot commented Sep 9, 2026

Copy link
Copy Markdown

BundleMon

Files updated (1)
Status Path Size Limits
renderer.(hash).js
10.48MB (+1.29KB +0.01%) -
Unchanged files (3)
Status Path Size Limits
vendors~renderer.(hash).js
4.67MB -
updater.js
115.29KB -
guest-api.js
40.23KB -

Total files change +1.29KB +0.01%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants