drivaspreset commented on code in PR #43004:
URL: https://github.com/apache/superset/pull/43004#discussion_r4135002598
##########
superset-frontend/playwright/tests/dashboard/dashboard-test-helpers.ts:
##########
@@ -337,3 +388,253 @@ export async function createDashboardWithCharts(
return { dashboardId, charts };
}
+
+/** The rendered value of a big-number chart. */
+export function bigNumberValueLocator(
+ dashboard: DashboardPage,
+ chartId: number,
+): Locator {
+ return dashboard
+ .getChart(chartId)
+ .locator('.superset-legacy-chart-big-number .header-line');
+}
+
+interface SetupDashboardWithChartsResult {
+ dashboardId: number;
+ charts: DashboardLayoutChart[];
+ dashboard: DashboardPage;
+ /** Big-number value locator per chart, in the same order as `charts`. */
+ valueLocators: Locator[];
+}
+
+/**
+ * Combines {@link createDashboardWithCharts} with navigating to the result and
+ * waiting for it to load -- the setup every GAQ test case that renders a plain
+ * big-number dashboard needs before it starts recording its own signals or
+ * assertions. Callers still assert on `valueLocators` themselves (a happy-path
+ * test wants them visible; a broken-chart test wants an error alert instead),
+ * so this only removes the identical creation/navigation boilerplate, not the
+ * per-test assertions layered on top of it.
+ *
+ * @example
+ * const { charts, dashboard, valueLocators } =
+ * await setupDashboardWithBigNumberCharts(page, testAssets, testInfo, {
+ * datasetName: 'birth_names',
+ * chartNamePrefix: 'gaq_tc1_cold_cache',
+ * chartSpecs: [BIG_NUMBER_COUNT_SPEC],
+ * });
+ * const [chart] = charts;
+ * const [value] = valueLocators;
+ * await expect(value).toBeVisible({ timeout: TIMEOUT.CHART_RENDER });
+ */
+export async function setupDashboardWithBigNumberCharts(
+ page: Page,
+ testAssets: TestAssets,
+ testInfo: TestInfo,
+ options: CreateDashboardWithChartsOptions,
+ navigateOptions?: { timeout?: number },
+): Promise<SetupDashboardWithChartsResult> {
+ const { dashboardId, charts } = await createDashboardWithCharts(
+ page,
+ testAssets,
+ testInfo,
+ options,
+ );
+ const dashboard = new DashboardPage(page);
+ const valueLocators = charts.map(chart =>
+ bigNumberValueLocator(dashboard, chart.id),
+ );
+
+ await dashboard.gotoById(dashboardId);
+ await dashboard.waitForLoad(navigateOptions);
+
+ return { dashboardId, charts, dashboard, valueLocators };
+}
+
+export interface GaqSignals {
+ /**
+ * Every chart-data response status seen for a slice, in order. Under the
+ * Global Task Framework an async chart-data request produces *two* responses
+ * for the same URL: the 202 that hands the work to GTF, then the 200 the
+ * client gets when it re-issues the request and is served from the cache the
+ * tasks populated. A single value per slice would hide one of them.
+ *
+ * A native filter's value fetch hits the same endpoint without a `slice_id`,
+ * so it is keyed under `undefined` (see {@link sliceIdFromChartDataUrl}).
+ */
+ submitStatusesFor(sliceId?: number): readonly number[];
+ /** First status seen for a slice; `undefined` if it has not responded yet.
*/
+ submitStatusFor(sliceId?: number): number | undefined;
+ /** Poll/fetch events are counted, not flagged: on a busy dashboard they
arrive per chart. */
+ readonly taskStatusPollCount: number;
+ /** Chart-data re-requests that were served synchronously (200) after a 202.
*/
+ readonly cachedRereadCount: number;
+ readonly sawTaskStatusPoll: boolean;
+ /** True once some slice went 202 -> 200: a full async round trip completed.
*/
+ readonly sawAsyncRoundTrip: boolean;
+}
+
+/**
+ * Records the GAQ lifecycle signals seen from now on.
+ *
+ * Under GTF the cycle is: `POST /api/v1/chart/data` with `async_mode` returns
+ * **202** with task ids; the client observes completion via
+ * `GET /api/v1/task/status_changes` (the poll transport, which is what runs
+ * unless `WEBSOCKET_ENABLE` is on); it then **re-issues the same POST**, which
+ * returns **200** from the per-query cache the tasks warmed. There is no
+ * separate result-fetch endpoint any more -- the old `/chart/data/qc-<hash>`
+ * replay route was removed with the GTF migration.
+ *
+ * Attach only once the traffic you care about is the *next* thing to happen --
+ * an initial dashboard load fires the same signals, so tracking from before it
+ * would attribute that load's cycle to whatever you trigger after.
+ *
+ * Reads are live getters rather than a snapshot, so callers can poll them from
+ * inside an `expect(...).toPass()` retry block.
+ */
+export function trackGaqSignals(page: Page): GaqSignals {
+ const submitStatuses = new Map<number | undefined, number[]>();
+ let taskStatusPollCount = 0;
+ let cachedRereadCount = 0;
+
+ page.on('response', response => {
+ const request = response.request();
+ const url = response.url();
+
+ if (request.method() === 'POST' && url.includes('/api/v1/chart/data')) {
+ const sliceId = sliceIdFromChartDataUrl(url);
+ const seen = submitStatuses.get(sliceId) ?? [];
+ // A 200 following a 202 for the same slice is the re-request being
served
+ // from the warmed cache -- the completion half of the round trip.
+ if (response.status() === 200 && seen.includes(202)) {
+ cachedRereadCount += 1;
+ }
+ submitStatuses.set(sliceId, [...seen, response.status()]);
+ return;
+ }
+ if (
+ request.method() === 'GET' &&
+ url.includes(GAQ.TASK_STATUS_CHANGES_PATH)
+ ) {
+ taskStatusPollCount += 1;
+ }
+ });
+
+ return {
+ submitStatusesFor: sliceId => submitStatuses.get(sliceId) ?? [],
+ submitStatusFor: sliceId => submitStatuses.get(sliceId)?.[0],
+ get taskStatusPollCount() {
+ return taskStatusPollCount;
+ },
+ get cachedRereadCount() {
+ return cachedRereadCount;
+ },
+ get sawTaskStatusPoll() {
+ return taskStatusPollCount > 0;
+ },
+ get sawAsyncRoundTrip() {
+ return cachedRereadCount > 0;
+ },
+ };
+}
+
+interface SetupFilteredDashboardOptions {
+ /** Dataset backing both the chart and the filter's value lookup -- see
{@link CreateDashboardWithChartsOptions}. */
+ datasetName?: string;
+ datasetId?: number;
+ /** Prefix for the generated chart and dashboard names. */
+ namePrefix: string;
+ /** Column the native filter targets. */
+ filterColumn: string;
+ /** Label shown in the filter bar (default: the column name). */
+ filterName?: string;
+}
+
+interface SetupFilteredDashboardResult {
+ dashboardId: number;
+ chartId: number;
+ dashboard: DashboardPage;
+ filterBar: DashboardFilterBar;
+ /** Big-number value locator for the dashboard's single chart. */
+ value: Locator;
+}
+
+/**
+ * Builds a dashboard with one big-number chart plus a single-select native
+ * filter scoped to it. Does NOT navigate: some callers must attach network
+ * listeners before the first load (the filter's value fetch fires during the
+ * filter panel's own initialization).
+ */
+export async function setupDashboardWithSelectFilter(
+ page: Page,
+ testAssets: TestAssets,
+ testInfo: TestInfo,
+ options: SetupFilteredDashboardOptions,
+): Promise<SetupFilteredDashboardResult> {
+ const { dashboardId, charts } = await createDashboardWithCharts(
+ page,
+ testAssets,
+ testInfo,
+ {
+ datasetName: options.datasetName,
+ datasetId: options.datasetId,
+ chartNamePrefix: options.namePrefix,
+ chartSpecs: [BIG_NUMBER_COUNT_SPEC],
Review Comment:
You're right, and it was worse than "would fail" — it *was* failing,
silently. I confirmed the mechanism: `CreateDatasetCommand` calls
`fetch_metadata`, which only discovers columns and seeds no metrics, so a
saved-metric reference against an API-created dataset raises `Metric
'%(metric)s' does not exist` (`superset/models/helpers.py:4748`). That chart
has been erroring on every run of this test, and because the test only ever
looked at the filter dropdown, nothing caught it.
Fixed in d56f007ffc:
- `setupDashboardWithSelectFilter` now takes an optional `chartSpec`
(default unchanged: `BIG_NUMBER_COUNT_SPEC`), and the doc on that constant now
says plainly that its `count` is the *saved* metric and will not resolve on an
API-created dataset.
- The filter-dropdown test passes `BIG_NUMBER_ADHOC_COUNT_SPEC` — an ad-hoc
`COUNT(name)`, shared as `ADHOC_COUNT_NAME_METRIC`, which also replaces the
inline copy the cold-first-load test had.
- The test now asserts on the chart too, so this can't regress unnoticed: it
renders a digit, has no `.ant-alert-error`, and completes its own `202 -> 200`
round trip.
Green on the current run (test 9, both matrix legs).
--
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]
