test(dashboard): strengthen chart render + per-chart query assertions in load spec

Address round-2 review feedback on the dashboard-load Playwright migration:

- Render proof: wait on each expected `#chart-id-<id>` (the ChartRenderer
  render marker) instead of snapshotting holder count after the first holder
  attaches. A chart that never renders now times out and fails the test
  rather than passing on a partial count.
- Query proof: collect chart-data POSTs keyed by the slice_id encoded in the
  `form_data` query param, then assert every expected chart issued a 200 —
  not just that one chart-data request succeeded.
- Reuse `extractIdFromResponse` for both chart and dashboard creation instead
  of re-implementing the `result?.id ?? id` normalization twice.
- Extract the ROOT/GRID/ROW/CHART position_json scaffold into a typed
  `buildDashboardPositionJson` helper in dashboard-test-helpers.ts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

superset-frontend/playwright/pages/DashboardPage.ts

@@ -145,36 +145,30 @@ export class DashboardPage {
   }
 
   /**
-   * Wait for every chart on the dashboard to finish rendering and return how
-   * many charts rendered.
+   * Wait for every expected chart on the dashboard to finish rendering.
    *
-   * A chart's `#chart-id-<id>` element only becomes visible once the chart has
-   * fetched its data and rendered (the same signal the legacy Cypress
-   * `waitForChartLoad` helper relied on). The chart set is derived from the
-   * dashboard itself via the `[data-test="chart-grid-component"]` holders, so
-   * this adapts to any dashboard without hard-coding chart names or counts.
+   * A chart's `#chart-id-<id>` element is rendered by `ChartRenderer` only in
+   * the non-loading, non-failed branch of `Chart` (a loading chart shows a
+   * spinner instead; a failed chart early-returns an error container), so its
+   * visibility is a genuine "this chart rendered" signal — the same one the
+   * legacy Cypress `waitForChartLoad` helper relied on.
+   *
+   * The expected chart IDs are passed in rather than discovered from the DOM:
+   * waiting on each specific `#chart-id-<id>` means a chart that never renders
+   * makes this call time out and fail the test, and it avoids the partial-count
+   * race of snapshotting `holders.count()` after only the first holder attaches.
    */
-  async waitForAllChartsRendered(options?: {
-    timeout?: number;
-  }): Promise<number> {
+  async waitForAllChartsRendered(
+    expectedChartIds: number[],
+    options?: { timeout?: number },
+  ): Promise<void> {
     // Charts issue real backend queries; allow generous time for slow viz types.
     const timeout = options?.timeout ?? TIMEOUT.API_RESPONSE * 2;
-    const holders = this.page.locator('[data-test="chart-grid-component"]');
-    await holders.first().waitFor({ state: 'attached', timeout });
-
-    const count = await holders.count();
-    for (let i = 0; i < count; i += 1) {
-      const chartId = await holders.nth(i).getAttribute('data-test-chart-id');
-      if (!chartId) {
-        throw new Error(
-          `Chart holder ${i} is missing its data-test-chart-id attribute`,
-        );
-      }
+    for (const chartId of expectedChartIds) {
       await this.page
         .locator(`#chart-id-${chartId}`)
         .waitFor({ state: 'visible', timeout });
     }
-    return count;
   }
 
   /**

superset-frontend/playwright/tests/dashboard/dashboard-load.spec.ts

@@ -37,8 +37,10 @@ import { testWithAssets, expect } from '../../helpers/fixtures';
 import { apiPostChart, apiPutChart } from '../../helpers/api/chart';
 import { apiPostDashboard } from '../../helpers/api/dashboard';
 import { getDatasetByName } from '../../helpers/api/dataset';
+import { extractIdFromResponse } from '../../helpers/api/assertions';
 import { TIMEOUT } from '../../utils/constants';
 import { DashboardPage } from '../../pages/DashboardPage';
+import { buildDashboardPositionJson } from './dashboard-test-helpers';
 
 const DATASET_NAME = 'birth_names';
 
@@ -101,51 +103,14 @@ testWithAssets(
         params: JSON.stringify(spec.params),
       });
       expect(resp.ok()).toBe(true);
-      const body = await resp.json();
-      const chartId: number = body.result?.id ?? body.id;
-      if (!chartId) {
-        throw new Error(
-          `Chart creation for ${spec.viz_type} returned no id: ${JSON.stringify(body)}`,
-        );
-      }
+      const chartId = await extractIdFromResponse(resp);
       testAssets.trackChart(chartId);
       charts.push({ id: chartId, sliceName });
     }
     const chartIds = charts.map(chart => chart.id);
 
     // Lay all charts out in a single row.
-    const chartKeys = chartIds.map(id => `CHART-${id}`);
-    const positionJson: Record<string, unknown> = {
-      DASHBOARD_VERSION_KEY: 'v2',
-      ROOT_ID: { type: 'ROOT', id: 'ROOT_ID', children: ['GRID_ID'] },
-      GRID_ID: {
-        type: 'GRID',
-        id: 'GRID_ID',
-        children: ['ROW-1'],
-        parents: ['ROOT_ID'],
-      },
-      'ROW-1': {
-        type: 'ROW',
-        id: 'ROW-1',
-        children: chartKeys,
-        parents: ['ROOT_ID', 'GRID_ID'],
-        meta: { background: 'BACKGROUND_TRANSPARENT' },
-      },
-    };
-    chartIds.forEach((chartId, index) => {
-      positionJson[chartKeys[index]] = {
-        type: 'CHART',
-        id: chartKeys[index],
-        children: [],
-        parents: ['ROOT_ID', 'GRID_ID', 'ROW-1'],
-        meta: {
-          chartId,
-          width: 4,
-          height: 50,
-          sliceName: charts[index].sliceName,
-        },
-      };
-    });
+    const positionJson = buildDashboardPositionJson(charts);
 
     const dashResp = await apiPostDashboard(page, {
       dashboard_title: `load_smoke_${uniqueSuffix}`,
@@ -153,8 +118,7 @@ testWithAssets(
       position_json: JSON.stringify(positionJson),
     });
     expect(dashResp.ok()).toBe(true);
-    const dashBody = await dashResp.json();
-    const dashboardId: number = dashBody.result?.id ?? dashBody.id;
+    const dashboardId = await extractIdFromResponse(dashResp);
     testAssets.trackDashboard(dashboardId);
 
     // Associate every chart with the dashboard so they actually render.
@@ -162,15 +126,31 @@ testWithAssets(
       await apiPutChart(page, chartId, { dashboards: [dashboardId] });
     }
 
-    // Record the real chart-data round-trips the dashboard makes on load.
-    const chartDataStatuses: number[] = [];
+    // Record the real chart-data round-trips the dashboard makes on load,
+    // keyed by the chart each one queried for. The chart-data POST carries its
+    // slice id in the encoded `form_data={"slice_id":<id>}` query param (see
+    // chartAction.ts), so parsing it lets us prove every chart queried — not
+    // just that some chart did.
+    const chartDataStatusBySliceId = new Map<number, number>();
     page.on('response', response => {
       const request = response.request();
       if (
-        request.method() === 'POST' &&
-        response.url().includes('/api/v1/chart/data')
+        request.method() !== 'POST' ||
+        !response.url().includes('/api/v1/chart/data')
       ) {
-        chartDataStatuses.push(response.status());
+        return;
+      }
+      const formData = new URL(response.url()).searchParams.get('form_data');
+      if (!formData) {
+        return;
+      }
+      try {
+        const sliceId = JSON.parse(formData).slice_id;
+        if (typeof sliceId === 'number') {
+          chartDataStatusBySliceId.set(sliceId, response.status());
+        }
+      } catch {
+        // Not a slice-id form_data payload; ignore.
       }
     });
 
@@ -178,15 +158,22 @@ testWithAssets(
     await dashboard.gotoById(dashboardId);
     await dashboard.waitForLoad();
 
-    // Every chart grid component must reach its rendered state.
-    const renderedCount = await dashboard.waitForAllChartsRendered();
-    expect(renderedCount).toBe(chartIds.length);
+    // Each expected chart must reach its rendered state; a chart that never
+    // renders makes this time out and fail rather than passing silently.
+    await dashboard.waitForAllChartsRendered(chartIds);
 
-    // The render came from real backend queries, and all of them succeeded.
-    expect(chartDataStatuses.length).toBeGreaterThan(0);
-    expect(
-      chartDataStatuses.every(status => status === 200),
-      `all /api/v1/chart/data responses should be 200, got [${chartDataStatuses}]`,
-    ).toBe(true);
+    // The render came from real backend queries: every chart issued its own
+    // chart-data POST and each one succeeded.
+    for (const chartId of chartIds) {
+      const status = chartDataStatusBySliceId.get(chartId);
+      expect(
+        status,
+        `chart ${chartId} should have issued a /api/v1/chart/data POST`,
+      ).toBeDefined();
+      expect(
+        status,
+        `chart ${chartId}'s /api/v1/chart/data response should be 200`,
+      ).toBe(200);
+    }
   },
 );

superset-frontend/playwright/tests/dashboard/dashboard-test-helpers.ts

@@ -26,6 +26,56 @@ interface TestDashboardResult {
   name: string;
 }
 
+/** A chart to place in a generated dashboard layout. */
+export interface DashboardChartLayout {
+  id: number;
+  sliceName: string;
+}
+
+/**
+ * Build a v2 `position_json` that lays the given charts out in a single row.
+ *
+ * Centralizes the ROOT → GRID → ROW → CHART scaffold that every dashboard-
+ * building E2E test would otherwise hand-roll.
+ */
+export function buildDashboardPositionJson(
+  charts: DashboardChartLayout[],
+): Record<string, unknown> {
+  const chartKeys = charts.map(chart => `CHART-${chart.id}`);
+  const positionJson: Record<string, unknown> = {
+    DASHBOARD_VERSION_KEY: 'v2',
+    ROOT_ID: { type: 'ROOT', id: 'ROOT_ID', children: ['GRID_ID'] },
+    GRID_ID: {
+      type: 'GRID',
+      id: 'GRID_ID',
+      children: ['ROW-1'],
+      parents: ['ROOT_ID'],
+    },
+    'ROW-1': {
+      type: 'ROW',
+      id: 'ROW-1',
+      children: chartKeys,
+      parents: ['ROOT_ID', 'GRID_ID'],
+      meta: { background: 'BACKGROUND_TRANSPARENT' },
+    },
+  };
+  charts.forEach((chart, index) => {
+    positionJson[chartKeys[index]] = {
+      type: 'CHART',
+      id: chartKeys[index],
+      children: [],
+      parents: ['ROOT_ID', 'GRID_ID', 'ROW-1'],
+      meta: {
+        chartId: chart.id,
+        width: 4,
+        height: 50,
+        sliceName: chart.sliceName,
+      },
+    };
+  });
+  return positionJson;
+}
+
 interface CreateTestDashboardOptions {
   /** Prefix for generated name (default: 'test_dashboard') */
   prefix?: string;