fix(embedded-e2e): strengthen assertion signal-to-noise from review

Address /review-code findings — the previous round's hardening fixed
flake but a few assertions still gave weak signals:

- The chart-rendered selector matched a still-loading chart cell, since
  Superset's `Loading` spinner itself renders an SVG. Exclude the spinner
  via `:not(:has([data-test="loading-indicator"]))` and centralize the
  selector as `EmbeddedPage.RENDERED_CHART_SELECTOR`.
- The "dashboard renders" test only proved iframe/header chrome, not the
  dashboard. Add `waitForChartRendered()` so the test name matches what
  it asserts.
- The `hideTitle` test passed for the wrong reason if the locator
  drifted (`toBeHidden()` succeeds for absent elements). Add an explicit
  `toHaveCount(0)` so the contrast against the baseline visibility check
  in test 1 is load-bearing.
- `tokenCallCount` was a `>=1` check that any rendered dashboard would
  satisfy. Tighten to `=== 1` to actually exercise the SDK's caching
  contract.
- Drop the redundant `appUrl` shadow of `appServer.url`.
- Move `import os` to module top in the docker-light config; document
  the strict `"true"`-only env-var truthiness convention.

Pre-commit clean (type-check, prettier, oxlint, ruff, mypy). Local
re-verification blocked by an unrelated worktree env issue (semantic
layers feature has incomplete state — the docker-compose-light stack
doesn't bind-mount superset-core/, so the image's stale copy lacks the
new submodule); CI on the chromium-embedded project will validate.
Changes are strictly stronger assertions and refactors so they cannot
turn a previously-passing test into a false positive.

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

docker/pythonpath_dev/superset_config_docker_light.py

@@ -18,6 +18,8 @@
 # Configuration for docker-compose-light.yml - disables Redis and uses minimal services
 
 # Import all settings from the main config first
+import os
+
 from flask_caching.backends.filesystemcache import FileSystemCache
 
 from superset_config import *  # noqa: F403
@@ -39,9 +41,9 @@ CELERY_CONFIG = None  # type: ignore[assignment,misc]
 
 # Honor SUPERSET_FEATURE_<NAME> env vars on top of any flags inherited from
 # superset_config. Lets local dev/e2e enable features (e.g. EMBEDDED_SUPERSET)
-# without editing shipped config files.
-import os  # noqa: E402
-
+# without editing shipped config files. Only the literal string "true"
+# (case-insensitive) is treated as enabled — "1"/"yes"/"on" are not, matching
+# the strict-string convention used elsewhere in Superset's env parsing.
 FEATURE_FLAGS = {
     **FEATURE_FLAGS,  # noqa: F405
     **{

superset-frontend/playwright/pages/EmbeddedPage.ts

@@ -116,18 +116,27 @@ export class EmbeddedPage {
   }
 
   /**
-   * Wait for at least one chart to finish rendering — proven by the chart
-   * container holding a real viz element (svg, canvas, or table) rather than
-   * just a loading spinner.
+   * Matches a chart cell that has finished loading: it contains a real viz
+   * element (svg, canvas, table) AND no longer hosts the `Loading` spinner
+   * (`data-test="loading-indicator"`). Excluding the spinner matters —
+   * the spinner itself renders an SVG, so a `:has(svg)`-only check can match
+   * a still-loading chart for the wrong reason.
+   */
+  static readonly RENDERED_CHART_SELECTOR =
+    '[data-test="chart-container"]:has(svg, canvas, table):not(:has([data-test="loading-indicator"]))';
+
+  /**
+   * Wait for at least one chart to finish rendering — viz drawn AND no
+   * loading spinner in that cell.
    */
   async waitForChartRendered(options?: { timeout?: number }): Promise<void> {
-    const timeout = options?.timeout ?? EMBEDDED.CHART_RENDER;
     await this.iframe
-      .locator(
-        '[data-test="chart-container"]:has(svg, canvas, table, [class*="big-number"])',
-      )
+      .locator(EmbeddedPage.RENDERED_CHART_SELECTOR)
       .first()
-      .waitFor({ state: 'visible', timeout });
+      .waitFor({
+        state: 'visible',
+        timeout: options?.timeout ?? EMBEDDED.CHART_RENDER,
+      });
   }
 
   /**

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

@@ -152,7 +152,6 @@ test.describe('Embedded Dashboard E2E', () => {
   test.setTimeout(60000);
 
   let appServer: EmbedAppServer;
-  let appUrl: string;
   let accessToken: string;
   let embedUuid: string;
   let dashboardId: number;
@@ -167,7 +166,7 @@ test.describe('Embedded Dashboard E2E', () => {
       getGuestToken(page, dashboardId, { accessToken }),
     );
     await embeddedPage.goto({
-      appUrl,
+      appUrl: appServer.url,
       uuid: embedUuid,
       supersetDomain: SUPERSET_DOMAIN,
     });
@@ -184,7 +183,6 @@ test.describe('Embedded Dashboard E2E', () => {
     );
 
     appServer = await startEmbedAppServer();
-    appUrl = appServer.url;
 
     // Use a fresh context with auth to set up test data via API
     const context = await createAdminContext(browser);
@@ -236,11 +234,15 @@ test.describe('Embedded Dashboard E2E', () => {
     ).toHaveAttribute('src', new RegExp(`/embedded/${embedUuid}`));
 
     // Verify no errors in the test app
-    const error = await embeddedPage.getError();
-    expect(error).toBe('');
+    expect(await embeddedPage.getError()).toBe('');
 
-    // Baseline: title should be visible when hideTitle is not set
+    // Baseline: title should be visible when hideTitle is not set. This
+    // doubles as a positive existence check the `hideTitle` test relies on
+    // for distinguishing "title was hidden" from "selector is wrong".
     await expect(embeddedPage.titleLocator).toBeVisible();
+
+    // Prove the dashboard actually renders, not just the chrome.
+    await embeddedPage.waitForChartRendered();
   });
 
   test('UI config hideTitle hides dashboard title', async ({ page }) => {
@@ -249,7 +251,7 @@ test.describe('Embedded Dashboard E2E', () => {
       getGuestToken(page, dashboardId, { accessToken }),
     );
     await embeddedPage.goto({
-      appUrl,
+      appUrl: appServer.url,
       uuid: embedUuid,
       supersetDomain: SUPERSET_DOMAIN,
       hideTitle: true,
@@ -262,18 +264,21 @@ test.describe('Embedded Dashboard E2E', () => {
       page.locator('iframe[title="Embedded Dashboard"]'),
     ).toHaveAttribute('src', /uiConfig=/);
 
-    // Verify the title is actually hidden inside the iframe
+    // hideTitle removes the header from the DOM (rather than CSS-hiding it),
+    // so toBeHidden + toHaveCount(0) together assert: not visible AND
+    // confirmed-removed (so the test can't pass for the wrong reason if the
+    // selector ever drifts — the baseline test asserts the selector matches
+    // when hideTitle is off).
     await expect(embeddedPage.titleLocator).toBeHidden();
+    await expect(embeddedPage.titleLocator).toHaveCount(0);
   });
 
   test('charts render inside embedded iframe', async ({ page }) => {
     const embeddedPage = await setupEmbeddedPage(page);
 
-    // Wait for at least one chart to fully render (not just its container).
-    // Superset adds `.rendered` to `.chart-container` after the viz draws.
     await embeddedPage.waitForChartRendered();
     const renderedCharts = embeddedPage.iframe.locator(
-      '[data-test="chart-container"]:has(svg, canvas, table, [class*="big-number"])',
+      EmbeddedPage.RENDERED_CHART_SELECTOR,
     );
     expect(await renderedCharts.count()).toBeGreaterThan(0);
   });
@@ -306,7 +311,7 @@ test.describe('Embedded Dashboard E2E', () => {
       );
 
       await embeddedPage.goto({
-        appUrl,
+        appUrl: appServer.url,
         uuid: restrictedEmbed.uuid,
         supersetDomain: SUPERSET_DOMAIN,
       });
@@ -330,7 +335,7 @@ test.describe('Embedded Dashboard E2E', () => {
     });
 
     await embeddedPage.goto({
-      appUrl,
+      appUrl: appServer.url,
       uuid: embedUuid,
       supersetDomain: SUPERSET_DOMAIN,
     });
@@ -338,12 +343,13 @@ test.describe('Embedded Dashboard E2E', () => {
     await embeddedPage.waitForDashboardContent();
     await embeddedPage.waitForChartRendered();
 
-    // The SDK should have called fetchGuestToken at least once
-    expect(tokenCallCount).toBeGreaterThanOrEqual(1);
+    // The SDK fetches the token exactly once per embed (caching is the
+    // SDK's responsibility, not ours) — assert the stronger invariant.
+    expect(tokenCallCount).toBe(1);
 
     // Confirm at least one chart actually rendered with data, not just its shell
     const renderedCharts = embeddedPage.iframe.locator(
-      '[data-test="chart-container"]:has(svg, canvas, table, [class*="big-number"])',
+      EmbeddedPage.RENDERED_CHART_SELECTOR,
     );
     expect(await renderedCharts.count()).toBeGreaterThan(0);
   });