drivaspreset commented on PR #43004: URL: https://github.com/apache/superset/pull/43004#issuecomment-5269091451
> Heya @drivaspreset, nice coverage here on GAQ. @sadpandajoe's review comments are still open though, mostly pointing at duplicated code that could reuse existing helpers (`apiPutChart`, `apiPostChart`, the combo box filter component, the button component) - worth folding those in, this file's big enough that the reuse would help. > > Codeant's race-condition threads look legit too: the `girl`/`boy` race assertion around line 583 doesn't actually confirm the fast request completed before checking for staleness, and the native-filter dropdown test around line 1172 arms its response listener before navigation, so it could pass without the click ever hitting GAQ. Worth tightening those up before merging. Thanks for the flag! Joe's comments are now folded in — `apiPutChart`/`apiPostChart` replace the raw calls, and the setup/response-listener duplication is extracted into helpers (`setupDashboardWithBigNumberCharts`, `trackChartAsyncSignals`/`trackMultiChartAsyncSignals`) in `dashboard-test-helpers.ts`. The combo-box and apply-button reuse now goes through the existing `DashboardFilterBar`/`Button` components instead of hand-rolled locators. Codeant's two threads were legit — fixed both: - **Girl/boy race assertion**: the chart actually keeps showing the stale value the entire time a query is loading (confirmed via trace) rather than blanking, so the old assertion could pass without girl's second request ever completing. Reworked it to anchor on a fresh network-response tracker scoped to the race window instead of display text — which also surfaced that a repeated identical filter selection can hit GAQ's cache-hit shortcut (`200`, not `202`); the fix accepts either as valid proof of completion. - **Native-filter dropdown test**: moved the GAQ-signal assertion to run before the dropdown is touched at all, so it's explicit in code (not just a comment) that the async fetch happens during filter-panel init, and opening the dropdown is a separate, decoupled render check. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
