You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "mikebridge (via GitHub)" <gi...@apache.org> on 2026/10/08 20:10:34 UTC

[PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

mikebridge opened a new pull request, #45103:
URL: https://github.com/apache/superset/pull/45103

   ### SUMMARY
   
   The Playwright suite has no coverage for dashboards built on semantic views. Every semantic-layer provider needs a live warehouse, and the semantic-view structure endpoint calls the provider directly, so browser-level regressions there can only be caught by hand today.
   
   This PR adds a test-only in-memory semantic-layer provider and the first two specs that use it.
   
   - **Stub provider** (`tests/e2e_extensions/semantic_stub/`): a backend-only `LOCAL_EXTENSIONS` bundle that registers with the public `@semantic_layer` decorator, inside the extension context. Its layer type is therefore `extensions.superset-e2e.semantic-stub.stub`, and the host code under test is the same code a real provider runs through. It has one `orders` view (dimensions `category`/`region`; metrics `total_amount` SUM and `order_count` COUNT) over four fixed rows. Query shapes it doesn't implement (HAVING, other operators, ad-hoc ordering, group limits) raise an error rather than returning unfiltered rows. A future change to the host's query shape then fails the test instead of rendering plausible but wrong data.
   - **Test-only config** (`tests/integration_tests/superset_test_config_semantic.py`): the base test config plus `ENABLE_EXTENSIONS`, `SEMANTIC_LAYERS` and `LOCAL_EXTENSIONS`. Only the new E2E step uses it, so the stub never loads for the unit, integration or required Playwright runs. It follows the existing `superset_test_config_gaq.py` pattern. A narrow `.gitignore` negation commits the bundle's `dist/` directory, which the extension loader requires.
   - **Playwright helpers:** seeding for a layer and its view over the REST API, with the view id resolved through `GET /api/v1/datasource/` (the semantic-view API has no GET). `testAssets` gains `trackSemanticLayer`; deleting a layer deletes its views. `buildSelectFilter` gains an optional `datasourceType: 'semantic_view'`.
   - **Specs** (`playwright/tests/semantic-view/`):
     1. A semantic-view table chart renders on a dashboard: chart-data returns 200, and the totals are Books 15, Games 7, Music 3.
     2. A native filter on a semantic-view dimension (region = West) filters that chart: Books 5, Music 3, no Games row. With the filter dropped, Books would show 15, which the spec rejects.
   - **Skip vs fail:** the specs skip unless the stub is registered, so the required run collects and skips them. The new step sets `PLAYWRIGHT_REQUIRE_SEMANTIC_STUB=true`, which turns a missing stub into a failure. A broken extension load can't pass as an empty run.
   - **Unit test** (`tests/unit_tests/semantic_layers/e2e_stub_extension_test.py`): loads the bundle through `SupersetAppInitializer.init_extensions()`, asserts the extension-prefixed registration, and runs `/api/v1/chart/data` through the host mapper into the stub, for unfiltered, dimension-filtered and unsupported-operator requests.
   
   **Follow-up:** a third spec is ready and will come in a separate PR once #45069 merges. It opens the native-filter config modal on a semantic-view dashboard and asserts that the semantic view is the preselected datasource and that it is never requested as a SQL dataset. It's left out here because it fails until #45069 lands.
   
   **CI cost:** one new step per E2E matrix entry (`app_root` `""` and `/app/prefix`), shaped like the existing Soft-delete step: it boots its own gunicorn, then runs two specs. Locally, the two specs take about 1.2–1.6 s each, and a full run including global login takes 4.4–5.4 s. With the gunicorn boot, expect roughly 20–40 s per matrix entry. No new services, containers or dependencies.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   N/A (tests only).
   
   ### TESTING INSTRUCTIONS
   
   Unit test:
   
   ```bash
   pytest tests/unit_tests/semantic_layers/e2e_stub_extension_test.py
   ```
   
   Playwright, against a backend started with the semantic test config:
   
   ```bash
   export SUPERSET_CONFIG=tests.integration_tests.superset_test_config_semantic
   superset db upgrade && superset load_test_users && superset init
   # build the frontend, then start the server (e.g. gunicorn "superset.app:create_app()" on :8088)
   cd superset-frontend
   PLAYWRIGHT_REQUIRE_SEMANTIC_STUB=true npx playwright test semantic-view/
   ```
   
   Local results with retries disabled: each spec passed 3 runs out of 3, and a parallel pass (`--workers=2 --repeat-each=5`) passed 10/10. All semantic layers the tests created were cleaned up.
   
   The local run used SQLite and in-process caches; this PR's E2E job is the Postgres/Redis check. One thing that job verifies: CI runs `superset init` under the base config, so the semantic-layer API permissions have to be created when gunicorn starts. If the job reports a 403 on `/api/v1/semantic_layer/`, the step also needs `superset init` under the semantic config.
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [x] Required feature flags: `SEMANTIC_LAYERS`, `ENABLE_EXTENSIONS` (test config only)
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #45103:
URL: https://github.com/apache/superset/pull/45103#discussion_r4254497951


##########
.github/workflows/bashlib.sh:
##########
@@ -295,6 +295,30 @@ playwright-run() {
   return $status
 }
 
+playwright-run-semantic() {
+  # The semantic-layer REST APIs only register when SEMANTIC_LAYERS is on, and
+  # Superset builds AppBuilder with update_perms=False, so their permissions
+  # exist only after a `superset init` that sees the flag. playwright_testdata
+  # ran init under the base test config, so sync roles again under this step's
+  # config before `playwright-run` boots gunicorn with it -- the same thing a
+  # deployment does after turning the flag on. Without it Admin gets 403 on
+  # POST /api/v1/semantic_layer/.

Review Comment:
   https://github.com/apache/superset/pull/45061, already on master, registers the semantic APIs regardless of `SEMANTIC_LAYERS`, so the base `superset init` creates these perms and this rationale (and the yml comment) is false after merge. No suggestion: the re-init is needed until you rebase.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #45103:
URL: https://github.com/apache/superset/pull/45103#issuecomment-6068534121

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/45103?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 82.62%. Comparing base ([`34b3d65`](https://app.codecov.io/gh/apache/superset/commit/34b3d65cc208600e0152017ba0db705a3d2b57bd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`1d55953`](https://app.codecov.io/gh/apache/superset/commit/1d559533e0155e48e9a2e23d4561591850486fe2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #45103      +/-   ##
   ==========================================
   - Coverage   82.64%   82.62%   -0.03%     
   ==========================================
     Files        3007     3007              
     Lines      187318   187330      +12     
     Branches    43399    43405       +6     
   ==========================================
   - Hits       154811   154781      -30     
   - Misses      29742    29752      +10     
   - Partials     2765     2797      +32     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/45103/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/45103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `36.18% <ø> (ø)` | |
   | [javascript](https://app.codecov.io/gh/apache/superset/pull/45103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.38% <ø> (-0.01%)` | :arrow_down: |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/45103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/45103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/45103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.04% <ø> (ø)` | |
   | [python](https://app.codecov.io/gh/apache/superset/pull/45103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `86.42% <ø> (-0.03%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/45103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `54.85% <ø> (ø)` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/45103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `79.77% <ø> (+0.06%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/45103?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #45103:
URL: https://github.com/apache/superset/pull/45103


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #45103:
URL: https://github.com/apache/superset/pull/45103#issuecomment-6089642326

   ## CodeAnt PR Risk: Low Risk
   
   - The PR appears safe to merge; the changes add semantic-view end-to-end coverage and keep the stub in a dedicated test configuration.
   - Semantic layers are tracked for cleanup, and the tests check both chart results and dimension-filter behavior.
   
   **Assessed commit:** `cbcc3d09248d`


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on code in PR #45103:
URL: https://github.com/apache/superset/pull/45103#discussion_r4252493720


##########
superset-frontend/playwright/tests/semantic-view/semantic-view-dashboard.spec.ts:
##########
@@ -0,0 +1,194 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Semantic-view dashboards against the in-memory stub provider
+ * (tests/e2e_extensions/semantic_stub). The stub's `orders` view has fixed
+ * rows, so totals are exact: Books 15 (East 10 + West 5), Games 7, Music 3.
+ *
+ * These specs skip unless the stub is loaded; CI runs them in a dedicated
+ * step against `superset_test_config_semantic`.
+ */
+
+import type { Locator, Page } from '@playwright/test';
+import {
+  testWithAssets,
+  expect,
+  type TestAssets,
+} from '../../helpers/fixtures';
+import { apiPost, apiPut } from '../../helpers/api/requests';
+import {
+  apiPostDashboard,
+  buildSingleRowDashboardLayout,
+} from '../../helpers/api/dashboard';
+import { extractIdFromResponse } from '../../helpers/api/assertions';
+import {
+  apiCreateStubSemanticView,
+  skipUnlessSemanticStub,
+  StubSemanticView,
+} from '../../helpers/api/semanticLayer';
+import { DashboardPage } from '../../pages/DashboardPage';
+import { TIMEOUT } from '../../utils/constants';
+import {
+  buildFilterJsonMetadata,
+  buildSelectFilter,
+  type NativeFilterConfig,
+} from '../dashboard/dashboard-test-helpers';
+
+const SLICE_NAME = 'Orders by category';
+
+interface SemanticDashboard {
+  view: StubSemanticView;
+  chartId: number;
+  dashboardId: number;
+}
+
+/** One table chart (category × total_amount) on a one-chart dashboard. */
+async function createSemanticDashboard(
+  page: Page,
+  testAssets: TestAssets,
+  nativeFilters: (
+    view: StubSemanticView,
+    chartId: number,
+  ) => NativeFilterConfig[],
+): Promise<SemanticDashboard> {
+  const suffix = `${Date.now()}_${Math.random().toString(36).slice(2, 8)}`;
+  const view = await apiCreateStubSemanticView(page, suffix);
+  testAssets.trackSemanticLayer(view.layerUuid);
+
+  const params = {
+    datasource: view.datasource,
+    viz_type: 'table',
+    query_mode: 'aggregate',
+    groupby: ['category'],
+    metrics: ['total_amount'],
+    adhoc_filters: [],
+    row_limit: 100,
+  };
+  const chartResp = await apiPost(page, 'api/v1/chart/', {
+    slice_name: `${SLICE_NAME} ${suffix}`,
+    viz_type: 'table',
+    datasource_id: view.viewId,
+    datasource_type: 'semantic_view',
+    params: JSON.stringify(params),
+  });
+  const chartId = await extractIdFromResponse(chartResp);
+  testAssets.trackChart(chartId);
+
+  const dashResp = await apiPostDashboard(page, {
+    dashboard_title: `semantic_stub_${suffix}`,
+    published: true,
+    position_json: JSON.stringify(
+      buildSingleRowDashboardLayout([
+        { id: chartId, sliceName: SLICE_NAME, width: 6, height: 50 },
+      ]),
+    ),
+    json_metadata: JSON.stringify(
+      buildFilterJsonMetadata({
+        chartsInScope: [chartId],
+        nativeFilters: nativeFilters(view, chartId),
+      }),
+    ),
+  });
+  const dashboardId = await extractIdFromResponse(dashResp);
+  testAssets.trackDashboard(dashboardId);
+  await apiPut(page, `api/v1/chart/${chartId}`, { dashboards: [dashboardId] });
+
+  return { view, chartId, dashboardId };
+}
+
+/**
+ * The table row for a category. Cells are named by their column header, so
+ * match on the cell's text instead of its accessible name.
+ */
+function rowFor(chart: Locator, category: string): Locator {
+  return chart.getByRole('row').filter({
+    // `has` is queried inside each row, so the inner locator must not be
+    // rooted at `chart`.
+    has: chart
+      .page()
+      .getByRole('cell')
+      .filter({ hasText: new RegExp(`^${category}$`) }),
+  });
+}
+
+testWithAssets.beforeEach(async ({ page }) => {
+  await skipUnlessSemanticStub(page);
+});
+
+testWithAssets(
+  'semantic-view chart renders on a dashboard',
+  async ({ page, testAssets }) => {
+    testWithAssets.setTimeout(TIMEOUT.SLOW_TEST);
+    const { chartId, dashboardId } = await createSemanticDashboard(
+      page,
+      testAssets,
+      () => [],
+    );
+
+    const chartData = page.waitForResponse(
+      response =>
+        response.url().includes('/api/v1/chart/data') &&
+        response.request().method() === 'POST',
+    );
+    const dashboardPage = new DashboardPage(page);
+    await dashboardPage.gotoById(dashboardId);
+    await dashboardPage.waitForLoad();
+    expect((await chartData).status()).toBe(200);
+    await dashboardPage.waitForChartsToLoad();
+
+    const chart = dashboardPage.getChart(chartId);
+    await expect(rowFor(chart, 'Books')).toContainText('15');
+    await expect(rowFor(chart, 'Games')).toContainText('7');
+    await expect(rowFor(chart, 'Music')).toContainText('3');
+  },
+);
+
+testWithAssets(
+  'native filter on a semantic-view dimension filters the chart',
+  async ({ page, testAssets }) => {
+    testWithAssets.setTimeout(TIMEOUT.SLOW_TEST);
+    const { chartId, dashboardId } = await createSemanticDashboard(
+      page,
+      testAssets,
+      (view, id) => [
+        buildSelectFilter({
+          datasetId: view.viewId,
+          datasourceType: 'semantic_view',
+          column: 'region',
+          chartsInScope: [id],
+          name: 'Region',
+          defaultValue: 'West',

Review Comment:
   Good catch — the saved West mask proved the chart predicate but did not prove the Region control could load semantic-view options. I added assertions that the control shows West and that its dropdown loads both East and West. Omitting `datasourceType` as a control made the test fail at the Region control; restoring it passed the focused E2E test against the semantic stub.
   
   Implemented in d71bfb2fc4. The test change was reviewed independently by Claude Sonnet 5.5; implementation and reply drafted with Codex assistance.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on PR #45103:
URL: https://github.com/apache/superset/pull/45103#issuecomment-6178635263

   @mikebridge wondering why the e2e tests would live with the core Apache Superset tests and not in a set as part of the extension itself?


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on PR #45103:
URL: https://github.com/apache/superset/pull/45103#issuecomment-6158972043

   @sadpandajoe could you take a look when you have a moment? This touches `.github/workflows/`, so it needs a code owner approval. I've approved at d7b612d238.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #45103:
URL: https://github.com/apache/superset/pull/45103#issuecomment-6099916760

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | 117b14dfef1a385ada9e1bf0256261b8d5ad00b7 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6aca6cde2dbba300081d4ac1 |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-45103--superset-docs-preview.netlify.app](https://deploy-preview-45103--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQ1MTAzLS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.j61VCMSoj_IGhBIDJ_oytv2va3FNRKxV5KeEC1o10SY)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpBCoAgEADAv-w5Qcsk-kHPWHclBStRIyT6e97mMC-kfB2pwgowgM14ku8uJNQ4a72IFLE9Oey-iuIOPGsgUept-2aX4tU27t8goSF2I1uLk5RyUayRFHw_) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-45103--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #45103:
URL: https://github.com/apache/superset/pull/45103#discussion_r4238643022


##########
superset-frontend/playwright/helpers/api/semanticLayer.ts:
##########
@@ -0,0 +1,125 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { APIResponse, Page, test } from '@playwright/test';
+import rison from 'rison';
+import { apiDelete, apiGet, apiPost, ApiRequestOptions } from './requests';
+
+/**
+ * Layer type of the in-memory provider in tests/e2e_extensions/semantic_stub.
+ * The prefix comes from the extension loader, so a stub that registered
+ * outside the extension mechanism would not match.
+ */
+export const STUB_SEMANTIC_LAYER_TYPE =
+  'extensions.superset-e2e.semantic-stub.stub';
+
+/** The stub's only view, with dimensions `category`/`region`. */
+export const STUB_VIEW_NAME = 'orders';
+
+const ENDPOINTS = {
+  SEMANTIC_LAYER: 'api/v1/semantic_layer/',
+  SEMANTIC_VIEW: 'api/v1/semantic_view/',
+  DATASOURCE: 'api/v1/datasource/',
+} as const;
+
+export interface StubSemanticView {
+  layerUuid: string;
+  viewId: number;
+  /** `<id>__semantic_view`, the form-data datasource key. */
+  datasource: string;
+}
+
+/**
+ * Stand down unless the stub provider is registered on this instance.
+ *
+ * The stub loads only under `superset_test_config_semantic`, so the required
+ * run collects and skips these specs. The dedicated CI step sets
+ * `PLAYWRIGHT_REQUIRE_SEMANTIC_STUB=true`, which turns a missing stub into a
+ * failure: a broken extension load must not pass as an empty run.
+ */
+export async function skipUnlessSemanticStub(page: Page): Promise<void> {
+  const response = await apiGet(page, `${ENDPOINTS.SEMANTIC_LAYER}types`, {
+    failOnStatusCode: false,
+  });
+  const types: Array<{ id: string }> = response.ok()
+    ? (await response.json()).result
+    : [];
+  const registered = types.some(type => type.id === STUB_SEMANTIC_LAYER_TYPE);
+  if (process.env.PLAYWRIGHT_REQUIRE_SEMANTIC_STUB?.toLowerCase() === 'true') {
+    if (!registered) {
+      throw new Error(
+        `${STUB_SEMANTIC_LAYER_TYPE} is not registered; check LOCAL_EXTENSIONS ` +
+          'and ENABLE_EXTENSIONS in superset_test_config_semantic.',
+      );
+    }
+    return;
+  }
+  test.skip(
+    !registered,
+    'The E2E semantic stub is not loaded on this instance.',
+  );
+}
+
+/**
+ * Create a stub semantic layer and its `orders` view. Track the returned
+ * layer for cleanup; deleting a layer deletes its views.
+ */
+export async function apiCreateStubSemanticView(
+  page: Page,
+  uniqueSuffix: string,
+): Promise<StubSemanticView> {
+  const layerResp = await apiPost(page, ENDPOINTS.SEMANTIC_LAYER, {
+    name: `e2e_semantic_stub_${uniqueSuffix}`,
+    type: STUB_SEMANTIC_LAYER_TYPE,
+    configuration: {},
+  });
+  const layerUuid: string = (await layerResp.json()).result.uuid;
+
+  const viewResp = await apiPost(page, ENDPOINTS.SEMANTIC_VIEW, {

Review Comment:
   If this view POST (or the datasource lookup below) fails, the helper throws before returning `layerUuid`, so the caller never reaches `trackSemanticLayer` and the layer created on line 87 is never deleted. On CI the database is throwaway, but local runs and retries would leave `e2e_semantic_stub_*` layers behind. Would it make sense to delete the layer in a `catch` here, or accept a tracking callback so the layer is registered right after it is created?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #45103:
URL: https://github.com/apache/superset/pull/45103#discussion_r4235681225


##########
tests/e2e_extensions/semantic_stub/dist/backend/src/superset_e2e_semantic_stub/__init__.py:
##########
@@ -0,0 +1,256 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+In-memory semantic layer for end-to-end tests.
+
+Loaded only as a ``LOCAL_EXTENSIONS`` entry by the Playwright semantic-view
+configuration (``tests.integration_tests.superset_test_config_semantic``). It
+registers through the public ``@semantic_layer`` decorator like any provider,
+so the host code under test is the code that runs in production; only the
+warehouse is replaced by a fixed table.
+
+Anything the stub does not implement raises instead of being ignored, so a
+host change that starts sending a new query shape fails the browser test
+rather than rendering plausible but unfiltered data.
+"""
+
+from __future__ import annotations
+
+from collections import defaultdict
+from operator import itemgetter
+from typing import Any
+
+import pyarrow as pa
+from pydantic import BaseModel, ConfigDict
+from superset_core.semantic_layers.decorators import semantic_layer
+from superset_core.semantic_layers.layer import SemanticLayer
+from superset_core.semantic_layers.types import (
+    AggregationType,
+    Dimension,
+    Filter,
+    FilterValues,
+    Metric,
+    Operator,
+    OrderDirection,
+    PredicateType,
+    SemanticQuery,
+    SemanticRequest,
+    SemanticResult,
+)
+from superset_core.semantic_layers.view import SemanticView
+
+VIEW_NAME: str = "orders"
+
+CATEGORY: Dimension = Dimension(
+    "category", "category", pa.string(), verbose_name="Category"
+)
+REGION: Dimension = Dimension("region", "region", pa.string(), verbose_name="Region")
+TOTAL_AMOUNT: Metric = Metric(
+    "total_amount",
+    "total_amount",
+    pa.int64(),
+    "SUM(amount)",
+    aggregation=AggregationType.SUM,
+    verbose_name="Total amount",
+)
+ORDER_COUNT: Metric = Metric(
+    "order_count",
+    "order_count",
+    pa.int64(),
+    "COUNT(*)",
+    aggregation=AggregationType.COUNT,
+    verbose_name="Order count",
+)
+
+# Small enough to reason about in an assertion: Books totals 10 + 5 = 15.
+ROWS: tuple[dict[str, Any], ...] = (
+    {"category": "Books", "region": "East", "amount": 10},
+    {"category": "Books", "region": "West", "amount": 5},
+    {"category": "Games", "region": "East", "amount": 7},
+    {"category": "Music", "region": "West", "amount": 3},
+)
+
+SUPPORTED_OPERATORS: frozenset[Operator] = frozenset(
+    {Operator.EQUALS, Operator.NOT_EQUALS, Operator.IN, Operator.NOT_IN}
+)
+
+
+class StubConfiguration(BaseModel):
+    """The stub needs no connection settings; reject anything supplied."""
+
+    model_config = ConfigDict(extra="forbid")
+
+
+def _matches(row: dict[str, Any], filter_: Filter) -> bool:
+    if filter_.type != PredicateType.WHERE or not isinstance(filter_.column, Dimension):
+        raise ValueError("The E2E stub only supports WHERE filters on dimensions")
+    if filter_.operator not in SUPPORTED_OPERATORS:
+        raise ValueError(f"The E2E stub does not support {filter_.operator.value}")
+
+    actual: Any = row[filter_.column.id]
+    values: frozenset[FilterValues] = (
+        frozenset(filter_.value)
+        if isinstance(filter_.value, (frozenset, tuple))
+        else frozenset({filter_.value})
+    )
+    if filter_.operator in {Operator.EQUALS, Operator.IN}:
+        return actual in values
+    return actual not in values
+
+
+def _filtered_rows(filters: set[Filter] | None) -> list[dict[str, Any]]:
+    return [row for row in ROWS if all(_matches(row, f) for f in filters or set())]
+
+
+def _aggregate(metric: Metric, rows: list[dict[str, Any]]) -> int:
+    if metric.aggregation == AggregationType.SUM:
+        return sum(row["amount"] for row in rows)
+    if metric.aggregation == AggregationType.COUNT:
+        return len(rows)
+    raise ValueError(f"The E2E stub cannot aggregate {metric.id}")
+
+
+class StubOrdersView(SemanticView):
+    """A single fixed view: two string dimensions and two additive metrics."""
+
+    name: str = VIEW_NAME
+
+    def uid(self) -> str:
+        return f"e2e-stub:{VIEW_NAME}"
+
+    def get_dimensions(self) -> set[Dimension]:
+        return {CATEGORY, REGION}
+
+    def get_metrics(self) -> set[Metric]:
+        return {TOTAL_AMOUNT, ORDER_COUNT}
+
+    def get_values(
+        self,
+        dimension: Dimension,
+        filters: set[Filter] | None = None,
+    ) -> SemanticResult:
+        values: list[Any] = sorted(
+            {row[dimension.id] for row in _filtered_rows(filters)}
+        )
+        return SemanticResult(
+            requests=[SemanticRequest("stub", f"values of {dimension.id}")],
+            results=pa.table({dimension.name: pa.array(values, dimension.type)}),
+        )
+
+    def get_table(self, query: SemanticQuery) -> SemanticResult:
+        if query.group_limit is not None:
+            raise ValueError("The E2E stub does not support group limits")
+
+        groups: dict[tuple[Any, ...], list[dict[str, Any]]] = defaultdict(list)
+        for row in _filtered_rows(query.filters):
+            groups[tuple(row[d.id] for d in query.dimensions)].append(row)
+        if not query.dimensions and not groups:
+            groups[()] = []  # an aggregate-only query always returns one row
+
+        records: list[dict[str, Any]] = [
+            {
+                **{d.name: key[i] for i, d in enumerate(query.dimensions)},
+                **{m.name: _aggregate(m, rows) for m in query.metrics},
+            }
+            for key, rows in groups.items()
+        ]
+        # Deterministic output: requested order first, then dimension values.
+        records.sort(key=lambda r: tuple(r[d.name] for d in query.dimensions))
+        for element, direction in reversed(query.order or []):
+            if not isinstance(element, (Dimension, Metric)):
+                raise ValueError("The E2E stub does not support adhoc ordering")
+            records.sort(
+                key=itemgetter(element.name),
+                reverse=direction == OrderDirection.DESC,
+            )
+        records = records[query.offset or 0 :]
+        if query.limit is not None:
+            records = records[: query.limit]
+
+        schema: pa.Schema = pa.schema(
+            [pa.field(d.name, d.type) for d in query.dimensions]
+            + [pa.field(m.name, m.type) for m in query.metrics]
+        )
+        return SemanticResult(
+            requests=[SemanticRequest("stub", f"table of {VIEW_NAME}")],
+            results=pa.Table.from_pylist(records, schema=schema),
+        )
+
+    def get_row_count(self, query: SemanticQuery) -> SemanticResult:
+        count: int = self.get_table(query).results.num_rows
+        return SemanticResult(
+            requests=[SemanticRequest("stub", f"row count of {VIEW_NAME}")],
+            results=pa.table({"COUNT": pa.array([count], pa.int64())}),
+        )
+
+    def get_compatible_metrics(
+        self,
+        selected_metrics: set[Metric],
+        selected_dimensions: set[Dimension],
+    ) -> set[Metric]:
+        return self.get_metrics()
+
+    def get_compatible_dimensions(
+        self,
+        selected_metrics: set[Metric],
+        selected_dimensions: set[Dimension],
+    ) -> set[Dimension]:
+        return self.get_dimensions()
+
+
+@semantic_layer(
+    id="stub",
+    name="E2E Semantic Stub",
+    description="Test-only in-memory semantic layer",
+)
+class StubSemanticLayer(SemanticLayer[StubConfiguration, StubOrdersView]):
+    configuration_class: type[StubConfiguration] = StubConfiguration
+
+    @classmethod
+    def from_configuration(cls, configuration: dict[str, Any]) -> StubSemanticLayer:
+        StubConfiguration.model_validate(configuration)
+        return cls()

Review Comment:
   `from_configuration` validates the config and then returns a bare `cls()`, so the instance has no `.configuration`. The host's `SemanticLayerRestApi.runtime_schema` passes `layer.implementation.configuration` to `get_runtime_schema`, so `POST /api/v1/semantic_layer/<uuid>/runtime_schema` for a stub layer would hit an `AttributeError` and return a 400. The seeding helper creates layers and views over the REST API directly, so none of the added tests would notice. Since the stub is meant to run through the same host code as a real provider, should it keep the validated `StubConfiguration` on the instance (e.g. `self.configuration`)?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on PR #45103:
URL: https://github.com/apache/superset/pull/45103#issuecomment-6179109219

   @sadpandajoe Good question. The specs test core host code, not a provider: the semantic-view structure and chart-data paths, the mapper, and native filters on a semantic-view dashboard. The real providers (Snowflake, dbt and so on) live outside apache/superset and need live warehouses, so they can't run in this CI. The stub is the smallest provider that exercises the host through the public `@semantic_layer` extension API, so breakage in core gets caught here. It's test-only: it loads only in the new E2E step through its own test config, never in the unit, integration or required Playwright runs. Each real provider's own tests still belong with that provider. If you'd rather the stub live somewhere else in the tree, I'm open to moving it.
   
   _(Reply drafted with Claude (AI) on behalf of @mikebridge.)_
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on code in PR #45103:
URL: https://github.com/apache/superset/pull/45103#discussion_r4253966440


##########
tests/e2e_extensions/semantic_stub/dist/backend/src/superset_e2e_semantic_stub/__init__.py:
##########
@@ -0,0 +1,259 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+In-memory semantic layer for end-to-end tests.
+
+Loaded only as a ``LOCAL_EXTENSIONS`` entry by the Playwright semantic-view
+configuration (``tests.integration_tests.superset_test_config_semantic``). It
+registers through the public ``@semantic_layer`` decorator like any provider,
+so the host code under test is the code that runs in production; only the
+warehouse is replaced by a fixed table.
+
+Anything the stub does not implement raises instead of being ignored, so a
+host change that starts sending a new query shape fails the browser test
+rather than rendering plausible but unfiltered data.
+"""
+
+from __future__ import annotations
+
+from collections import defaultdict
+from operator import itemgetter
+from typing import Any
+
+import pyarrow as pa
+from pydantic import BaseModel, ConfigDict
+from superset_core.semantic_layers.decorators import semantic_layer
+from superset_core.semantic_layers.layer import SemanticLayer
+from superset_core.semantic_layers.types import (
+    AggregationType,
+    Dimension,
+    Filter,
+    FilterValues,
+    Metric,
+    Operator,
+    OrderDirection,
+    PredicateType,
+    SemanticQuery,
+    SemanticRequest,
+    SemanticResult,
+)
+from superset_core.semantic_layers.view import SemanticView
+
+VIEW_NAME: str = "orders"
+
+CATEGORY: Dimension = Dimension(
+    "category", "category", pa.string(), verbose_name="Category"
+)
+REGION: Dimension = Dimension("region", "region", pa.string(), verbose_name="Region")
+TOTAL_AMOUNT: Metric = Metric(
+    "total_amount",
+    "total_amount",
+    pa.int64(),
+    "SUM(amount)",
+    aggregation=AggregationType.SUM,
+    verbose_name="Total amount",
+)
+ORDER_COUNT: Metric = Metric(
+    "order_count",
+    "order_count",
+    pa.int64(),
+    "COUNT(*)",
+    aggregation=AggregationType.COUNT,
+    verbose_name="Order count",
+)
+
+# Small enough to reason about in an assertion: Books totals 10 + 5 = 15.
+ROWS: tuple[dict[str, Any], ...] = (
+    {"category": "Books", "region": "East", "amount": 10},
+    {"category": "Books", "region": "West", "amount": 5},
+    {"category": "Games", "region": "East", "amount": 7},
+    {"category": "Music", "region": "West", "amount": 3},
+)
+
+SUPPORTED_OPERATORS: frozenset[Operator] = frozenset(
+    {Operator.EQUALS, Operator.NOT_EQUALS, Operator.IN, Operator.NOT_IN}
+)
+
+
+class StubConfiguration(BaseModel):
+    """The stub needs no connection settings; reject anything supplied."""
+
+    model_config = ConfigDict(extra="forbid")
+
+
+def _matches(row: dict[str, Any], filter_: Filter) -> bool:
+    if filter_.type != PredicateType.WHERE or not isinstance(filter_.column, Dimension):
+        raise ValueError("The E2E stub only supports WHERE filters on dimensions")
+    if filter_.operator not in SUPPORTED_OPERATORS:
+        raise ValueError(f"The E2E stub does not support {filter_.operator.value}")
+
+    actual: Any = row[filter_.column.id]
+    values: frozenset[FilterValues] = (
+        frozenset(filter_.value)
+        if isinstance(filter_.value, (frozenset, tuple))
+        else frozenset({filter_.value})
+    )
+    if filter_.operator in {Operator.EQUALS, Operator.IN}:
+        return actual in values
+    return actual not in values
+
+
+def _filtered_rows(filters: set[Filter] | None) -> list[dict[str, Any]]:
+    return [row for row in ROWS if all(_matches(row, f) for f in filters or set())]

Review Comment:
   Confirmed and fixed in 0608e2203b. The stub validates every filter's predicate type, dimension column and operator before scanning rows, so a no-match `region IN ['North']` filter cannot hide an unsupported `category LIKE 'B%'` predicate.
   
   The regression forces both iteration orders across table results, value suggestions and row counts, and covers unsupported operators, HAVING predicates and metric columns. Before the fix, all nine cases with the no-match filter first failed to raise; afterward all 24 focused tests pass. A supported no-match query still returns an empty result through the host chart-data API. Changed-file and branch-wide hooks passed, along with all three helper Jest tests.
   
   I also checked the packaging: this `dist/` entrypoint is the checked-in fixture loaded directly through `LOCAL_EXTENSIONS`; there is no separate generated source to update. Codex implemented and verified the fix, and an independent Claude review approved the exact committed tree.
   



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on code in PR #45103:
URL: https://github.com/apache/superset/pull/45103#discussion_r4238406245


##########
tests/e2e_extensions/semantic_stub/dist/backend/src/superset_e2e_semantic_stub/__init__.py:
##########
@@ -0,0 +1,256 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+In-memory semantic layer for end-to-end tests.
+
+Loaded only as a ``LOCAL_EXTENSIONS`` entry by the Playwright semantic-view
+configuration (``tests.integration_tests.superset_test_config_semantic``). It
+registers through the public ``@semantic_layer`` decorator like any provider,
+so the host code under test is the code that runs in production; only the
+warehouse is replaced by a fixed table.
+
+Anything the stub does not implement raises instead of being ignored, so a
+host change that starts sending a new query shape fails the browser test
+rather than rendering plausible but unfiltered data.
+"""
+
+from __future__ import annotations
+
+from collections import defaultdict
+from operator import itemgetter
+from typing import Any
+
+import pyarrow as pa
+from pydantic import BaseModel, ConfigDict
+from superset_core.semantic_layers.decorators import semantic_layer
+from superset_core.semantic_layers.layer import SemanticLayer
+from superset_core.semantic_layers.types import (
+    AggregationType,
+    Dimension,
+    Filter,
+    FilterValues,
+    Metric,
+    Operator,
+    OrderDirection,
+    PredicateType,
+    SemanticQuery,
+    SemanticRequest,
+    SemanticResult,
+)
+from superset_core.semantic_layers.view import SemanticView
+
+VIEW_NAME: str = "orders"
+
+CATEGORY: Dimension = Dimension(
+    "category", "category", pa.string(), verbose_name="Category"
+)
+REGION: Dimension = Dimension("region", "region", pa.string(), verbose_name="Region")
+TOTAL_AMOUNT: Metric = Metric(
+    "total_amount",
+    "total_amount",
+    pa.int64(),
+    "SUM(amount)",
+    aggregation=AggregationType.SUM,
+    verbose_name="Total amount",
+)
+ORDER_COUNT: Metric = Metric(
+    "order_count",
+    "order_count",
+    pa.int64(),
+    "COUNT(*)",
+    aggregation=AggregationType.COUNT,
+    verbose_name="Order count",
+)
+
+# Small enough to reason about in an assertion: Books totals 10 + 5 = 15.
+ROWS: tuple[dict[str, Any], ...] = (
+    {"category": "Books", "region": "East", "amount": 10},
+    {"category": "Books", "region": "West", "amount": 5},
+    {"category": "Games", "region": "East", "amount": 7},
+    {"category": "Music", "region": "West", "amount": 3},
+)
+
+SUPPORTED_OPERATORS: frozenset[Operator] = frozenset(
+    {Operator.EQUALS, Operator.NOT_EQUALS, Operator.IN, Operator.NOT_IN}
+)
+
+
+class StubConfiguration(BaseModel):
+    """The stub needs no connection settings; reject anything supplied."""
+
+    model_config = ConfigDict(extra="forbid")
+
+
+def _matches(row: dict[str, Any], filter_: Filter) -> bool:
+    if filter_.type != PredicateType.WHERE or not isinstance(filter_.column, Dimension):
+        raise ValueError("The E2E stub only supports WHERE filters on dimensions")
+    if filter_.operator not in SUPPORTED_OPERATORS:
+        raise ValueError(f"The E2E stub does not support {filter_.operator.value}")
+
+    actual: Any = row[filter_.column.id]
+    values: frozenset[FilterValues] = (
+        frozenset(filter_.value)
+        if isinstance(filter_.value, (frozenset, tuple))
+        else frozenset({filter_.value})
+    )
+    if filter_.operator in {Operator.EQUALS, Operator.IN}:
+        return actual in values
+    return actual not in values
+
+
+def _filtered_rows(filters: set[Filter] | None) -> list[dict[str, Any]]:
+    return [row for row in ROWS if all(_matches(row, f) for f in filters or set())]
+
+
+def _aggregate(metric: Metric, rows: list[dict[str, Any]]) -> int:
+    if metric.aggregation == AggregationType.SUM:
+        return sum(row["amount"] for row in rows)
+    if metric.aggregation == AggregationType.COUNT:
+        return len(rows)
+    raise ValueError(f"The E2E stub cannot aggregate {metric.id}")
+
+
+class StubOrdersView(SemanticView):
+    """A single fixed view: two string dimensions and two additive metrics."""
+
+    name: str = VIEW_NAME
+
+    def uid(self) -> str:
+        return f"e2e-stub:{VIEW_NAME}"
+
+    def get_dimensions(self) -> set[Dimension]:
+        return {CATEGORY, REGION}
+
+    def get_metrics(self) -> set[Metric]:
+        return {TOTAL_AMOUNT, ORDER_COUNT}
+
+    def get_values(
+        self,
+        dimension: Dimension,
+        filters: set[Filter] | None = None,
+    ) -> SemanticResult:
+        values: list[Any] = sorted(
+            {row[dimension.id] for row in _filtered_rows(filters)}
+        )
+        return SemanticResult(
+            requests=[SemanticRequest("stub", f"values of {dimension.id}")],
+            results=pa.table({dimension.name: pa.array(values, dimension.type)}),
+        )
+
+    def get_table(self, query: SemanticQuery) -> SemanticResult:
+        if query.group_limit is not None:
+            raise ValueError("The E2E stub does not support group limits")
+
+        groups: dict[tuple[Any, ...], list[dict[str, Any]]] = defaultdict(list)
+        for row in _filtered_rows(query.filters):
+            groups[tuple(row[d.id] for d in query.dimensions)].append(row)
+        if not query.dimensions and not groups:
+            groups[()] = []  # an aggregate-only query always returns one row
+
+        records: list[dict[str, Any]] = [
+            {
+                **{d.name: key[i] for i, d in enumerate(query.dimensions)},
+                **{m.name: _aggregate(m, rows) for m in query.metrics},
+            }
+            for key, rows in groups.items()
+        ]
+        # Deterministic output: requested order first, then dimension values.
+        records.sort(key=lambda r: tuple(r[d.name] for d in query.dimensions))
+        for element, direction in reversed(query.order or []):
+            if not isinstance(element, (Dimension, Metric)):
+                raise ValueError("The E2E stub does not support adhoc ordering")
+            records.sort(
+                key=itemgetter(element.name),
+                reverse=direction == OrderDirection.DESC,
+            )
+        records = records[query.offset or 0 :]
+        if query.limit is not None:
+            records = records[: query.limit]
+
+        schema: pa.Schema = pa.schema(
+            [pa.field(d.name, d.type) for d in query.dimensions]
+            + [pa.field(m.name, m.type) for m in query.metrics]
+        )
+        return SemanticResult(
+            requests=[SemanticRequest("stub", f"table of {VIEW_NAME}")],
+            results=pa.Table.from_pylist(records, schema=schema),
+        )
+
+    def get_row_count(self, query: SemanticQuery) -> SemanticResult:
+        count: int = self.get_table(query).results.num_rows
+        return SemanticResult(
+            requests=[SemanticRequest("stub", f"row count of {VIEW_NAME}")],
+            results=pa.table({"COUNT": pa.array([count], pa.int64())}),
+        )
+
+    def get_compatible_metrics(
+        self,
+        selected_metrics: set[Metric],
+        selected_dimensions: set[Dimension],
+    ) -> set[Metric]:
+        return self.get_metrics()
+
+    def get_compatible_dimensions(
+        self,
+        selected_metrics: set[Metric],
+        selected_dimensions: set[Dimension],
+    ) -> set[Dimension]:
+        return self.get_dimensions()
+
+
+@semantic_layer(
+    id="stub",
+    name="E2E Semantic Stub",
+    description="Test-only in-memory semantic layer",
+)
+class StubSemanticLayer(SemanticLayer[StubConfiguration, StubOrdersView]):
+    configuration_class: type[StubConfiguration] = StubConfiguration
+
+    @classmethod
+    def from_configuration(cls, configuration: dict[str, Any]) -> StubSemanticLayer:
+        StubConfiguration.model_validate(configuration)
+        return cls()

Review Comment:
   Good catch — the stub now retains the validated `StubConfiguration` on its instance, so the host runtime-schema path can read `.configuration`. There is only one stub copy, under `dist/`.
   
   - Added a regression through the real extension loader, stored layer model, and `POST /api/v1/semantic_layer/<uuid>/schema/runtime`: it failed with the missing-attribute 400 before the fix and now returns the expected schema.
   - The stub and semantic API suites pass (128 tests), and touched-file hooks pass.
   
   Implemented and tested with Codex; independently reviewed by Claude.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on code in PR #45103:
URL: https://github.com/apache/superset/pull/45103#discussion_r4249669656


##########
superset-frontend/playwright/helpers/api/semanticLayer.ts:
##########
@@ -0,0 +1,125 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { APIResponse, Page, test } from '@playwright/test';
+import rison from 'rison';
+import { apiDelete, apiGet, apiPost, ApiRequestOptions } from './requests';
+
+/**
+ * Layer type of the in-memory provider in tests/e2e_extensions/semantic_stub.
+ * The prefix comes from the extension loader, so a stub that registered
+ * outside the extension mechanism would not match.
+ */
+export const STUB_SEMANTIC_LAYER_TYPE =
+  'extensions.superset-e2e.semantic-stub.stub';
+
+/** The stub's only view, with dimensions `category`/`region`. */
+export const STUB_VIEW_NAME = 'orders';
+
+const ENDPOINTS = {
+  SEMANTIC_LAYER: 'api/v1/semantic_layer/',
+  SEMANTIC_VIEW: 'api/v1/semantic_view/',
+  DATASOURCE: 'api/v1/datasource/',
+} as const;
+
+export interface StubSemanticView {
+  layerUuid: string;
+  viewId: number;
+  /** `<id>__semantic_view`, the form-data datasource key. */
+  datasource: string;
+}
+
+/**
+ * Stand down unless the stub provider is registered on this instance.
+ *
+ * The stub loads only under `superset_test_config_semantic`, so the required
+ * run collects and skips these specs. The dedicated CI step sets
+ * `PLAYWRIGHT_REQUIRE_SEMANTIC_STUB=true`, which turns a missing stub into a
+ * failure: a broken extension load must not pass as an empty run.
+ */
+export async function skipUnlessSemanticStub(page: Page): Promise<void> {
+  const response = await apiGet(page, `${ENDPOINTS.SEMANTIC_LAYER}types`, {
+    failOnStatusCode: false,
+  });
+  const types: Array<{ id: string }> = response.ok()
+    ? (await response.json()).result
+    : [];
+  const registered = types.some(type => type.id === STUB_SEMANTIC_LAYER_TYPE);
+  if (process.env.PLAYWRIGHT_REQUIRE_SEMANTIC_STUB?.toLowerCase() === 'true') {
+    if (!registered) {
+      throw new Error(
+        `${STUB_SEMANTIC_LAYER_TYPE} is not registered; check LOCAL_EXTENSIONS ` +
+          'and ENABLE_EXTENSIONS in superset_test_config_semantic.',
+      );
+    }
+    return;
+  }
+  test.skip(
+    !registered,
+    'The E2E semantic stub is not loaded on this instance.',
+  );
+}
+
+/**
+ * Create a stub semantic layer and its `orders` view. Track the returned
+ * layer for cleanup; deleting a layer deletes its views.
+ */
+export async function apiCreateStubSemanticView(
+  page: Page,
+  uniqueSuffix: string,
+): Promise<StubSemanticView> {
+  const layerResp = await apiPost(page, ENDPOINTS.SEMANTIC_LAYER, {
+    name: `e2e_semantic_stub_${uniqueSuffix}`,
+    type: STUB_SEMANTIC_LAYER_TYPE,
+    configuration: {},
+  });
+  const layerUuid: string = (await layerResp.json()).result.uuid;
+
+  const viewResp = await apiPost(page, ENDPOINTS.SEMANTIC_VIEW, {

Review Comment:
   Good catch — the layer was created before either operation, and the caller could not track it until this helper returned. I added cleanup around view creation and datasource lookup, preserving the original failure if cleanup also fails. A red-first Jest test covered both failure paths and now passes, with a third case for cleanup failure.
   
   Implemented in 9cf0b7a26a. Reviewed independently by Claude Sonnet 5.5; implementation and reply drafted with Codex assistance.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #45103:
URL: https://github.com/apache/superset/pull/45103#discussion_r4253621017


##########
tests/e2e_extensions/semantic_stub/dist/backend/src/superset_e2e_semantic_stub/__init__.py:
##########
@@ -0,0 +1,259 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+In-memory semantic layer for end-to-end tests.
+
+Loaded only as a ``LOCAL_EXTENSIONS`` entry by the Playwright semantic-view
+configuration (``tests.integration_tests.superset_test_config_semantic``). It
+registers through the public ``@semantic_layer`` decorator like any provider,
+so the host code under test is the code that runs in production; only the
+warehouse is replaced by a fixed table.
+
+Anything the stub does not implement raises instead of being ignored, so a
+host change that starts sending a new query shape fails the browser test
+rather than rendering plausible but unfiltered data.
+"""
+
+from __future__ import annotations
+
+from collections import defaultdict
+from operator import itemgetter
+from typing import Any
+
+import pyarrow as pa
+from pydantic import BaseModel, ConfigDict
+from superset_core.semantic_layers.decorators import semantic_layer
+from superset_core.semantic_layers.layer import SemanticLayer
+from superset_core.semantic_layers.types import (
+    AggregationType,
+    Dimension,
+    Filter,
+    FilterValues,
+    Metric,
+    Operator,
+    OrderDirection,
+    PredicateType,
+    SemanticQuery,
+    SemanticRequest,
+    SemanticResult,
+)
+from superset_core.semantic_layers.view import SemanticView
+
+VIEW_NAME: str = "orders"
+
+CATEGORY: Dimension = Dimension(
+    "category", "category", pa.string(), verbose_name="Category"
+)
+REGION: Dimension = Dimension("region", "region", pa.string(), verbose_name="Region")
+TOTAL_AMOUNT: Metric = Metric(
+    "total_amount",
+    "total_amount",
+    pa.int64(),
+    "SUM(amount)",
+    aggregation=AggregationType.SUM,
+    verbose_name="Total amount",
+)
+ORDER_COUNT: Metric = Metric(
+    "order_count",
+    "order_count",
+    pa.int64(),
+    "COUNT(*)",
+    aggregation=AggregationType.COUNT,
+    verbose_name="Order count",
+)
+
+# Small enough to reason about in an assertion: Books totals 10 + 5 = 15.
+ROWS: tuple[dict[str, Any], ...] = (
+    {"category": "Books", "region": "East", "amount": 10},
+    {"category": "Books", "region": "West", "amount": 5},
+    {"category": "Games", "region": "East", "amount": 7},
+    {"category": "Music", "region": "West", "amount": 3},
+)
+
+SUPPORTED_OPERATORS: frozenset[Operator] = frozenset(
+    {Operator.EQUALS, Operator.NOT_EQUALS, Operator.IN, Operator.NOT_IN}
+)
+
+
+class StubConfiguration(BaseModel):
+    """The stub needs no connection settings; reject anything supplied."""
+
+    model_config = ConfigDict(extra="forbid")
+
+
+def _matches(row: dict[str, Any], filter_: Filter) -> bool:
+    if filter_.type != PredicateType.WHERE or not isinstance(filter_.column, Dimension):
+        raise ValueError("The E2E stub only supports WHERE filters on dimensions")
+    if filter_.operator not in SUPPORTED_OPERATORS:
+        raise ValueError(f"The E2E stub does not support {filter_.operator.value}")
+
+    actual: Any = row[filter_.column.id]
+    values: frozenset[FilterValues] = (
+        frozenset(filter_.value)
+        if isinstance(filter_.value, (frozenset, tuple))
+        else frozenset({filter_.value})
+    )
+    if filter_.operator in {Operator.EQUALS, Operator.IN}:
+        return actual in values
+    return actual not in values
+
+
+def _filtered_rows(filters: set[Filter] | None) -> list[dict[str, Any]]:
+    return [row for row in ROWS if all(_matches(row, f) for f in filters or set())]

Review Comment:
   The stub can return a successful empty result for a query shape it is meant to reject. `all(...)` short-circuits, and `filters` is a set with no defined order. For a request with `region IN ['North']` plus an unsupported `category LIKE 'B%'`, if the `IN` filter is evaluated first it rejects every row and `_matches` never runs on the `LIKE` filter, so no error is raised. Whether it fails then depends on set iteration order. Should the filters be validated once up front, before the row scan, so an unsupported predicate always raises?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on PR #45103:
URL: https://github.com/apache/superset/pull/45103#issuecomment-6091302295

   Merged latest master in `14b0d4aed7` to restart CI after the Docker Hub image-pull failures. Focused local tests and changed-file checks passed; the merge adds no PR-specific code changes. Monitoring the new CI run.
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #45103:
URL: https://github.com/apache/superset/pull/45103#discussion_r4250718661


##########
superset-frontend/playwright/tests/semantic-view/semantic-view-dashboard.spec.ts:
##########
@@ -0,0 +1,194 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Semantic-view dashboards against the in-memory stub provider
+ * (tests/e2e_extensions/semantic_stub). The stub's `orders` view has fixed
+ * rows, so totals are exact: Books 15 (East 10 + West 5), Games 7, Music 3.
+ *
+ * These specs skip unless the stub is loaded; CI runs them in a dedicated
+ * step against `superset_test_config_semantic`.
+ */
+
+import type { Locator, Page } from '@playwright/test';
+import {
+  testWithAssets,
+  expect,
+  type TestAssets,
+} from '../../helpers/fixtures';
+import { apiPost, apiPut } from '../../helpers/api/requests';
+import {
+  apiPostDashboard,
+  buildSingleRowDashboardLayout,
+} from '../../helpers/api/dashboard';
+import { extractIdFromResponse } from '../../helpers/api/assertions';
+import {
+  apiCreateStubSemanticView,
+  skipUnlessSemanticStub,
+  StubSemanticView,
+} from '../../helpers/api/semanticLayer';
+import { DashboardPage } from '../../pages/DashboardPage';
+import { TIMEOUT } from '../../utils/constants';
+import {
+  buildFilterJsonMetadata,
+  buildSelectFilter,
+  type NativeFilterConfig,
+} from '../dashboard/dashboard-test-helpers';
+
+const SLICE_NAME = 'Orders by category';
+
+interface SemanticDashboard {
+  view: StubSemanticView;
+  chartId: number;
+  dashboardId: number;
+}
+
+/** One table chart (category × total_amount) on a one-chart dashboard. */
+async function createSemanticDashboard(
+  page: Page,
+  testAssets: TestAssets,
+  nativeFilters: (
+    view: StubSemanticView,
+    chartId: number,
+  ) => NativeFilterConfig[],
+): Promise<SemanticDashboard> {
+  const suffix = `${Date.now()}_${Math.random().toString(36).slice(2, 8)}`;
+  const view = await apiCreateStubSemanticView(page, suffix);
+  testAssets.trackSemanticLayer(view.layerUuid);
+
+  const params = {
+    datasource: view.datasource,
+    viz_type: 'table',
+    query_mode: 'aggregate',
+    groupby: ['category'],
+    metrics: ['total_amount'],
+    adhoc_filters: [],
+    row_limit: 100,
+  };
+  const chartResp = await apiPost(page, 'api/v1/chart/', {
+    slice_name: `${SLICE_NAME} ${suffix}`,
+    viz_type: 'table',
+    datasource_id: view.viewId,
+    datasource_type: 'semantic_view',
+    params: JSON.stringify(params),
+  });
+  const chartId = await extractIdFromResponse(chartResp);
+  testAssets.trackChart(chartId);
+
+  const dashResp = await apiPostDashboard(page, {
+    dashboard_title: `semantic_stub_${suffix}`,
+    published: true,
+    position_json: JSON.stringify(
+      buildSingleRowDashboardLayout([
+        { id: chartId, sliceName: SLICE_NAME, width: 6, height: 50 },
+      ]),
+    ),
+    json_metadata: JSON.stringify(
+      buildFilterJsonMetadata({
+        chartsInScope: [chartId],
+        nativeFilters: nativeFilters(view, chartId),
+      }),
+    ),
+  });
+  const dashboardId = await extractIdFromResponse(dashResp);
+  testAssets.trackDashboard(dashboardId);
+  await apiPut(page, `api/v1/chart/${chartId}`, { dashboards: [dashboardId] });
+
+  return { view, chartId, dashboardId };
+}
+
+/**
+ * The table row for a category. Cells are named by their column header, so
+ * match on the cell's text instead of its accessible name.
+ */
+function rowFor(chart: Locator, category: string): Locator {
+  return chart.getByRole('row').filter({
+    // `has` is queried inside each row, so the inner locator must not be
+    // rooted at `chart`.
+    has: chart
+      .page()
+      .getByRole('cell')
+      .filter({ hasText: new RegExp(`^${category}$`) }),
+  });
+}
+
+testWithAssets.beforeEach(async ({ page }) => {
+  await skipUnlessSemanticStub(page);
+});
+
+testWithAssets(
+  'semantic-view chart renders on a dashboard',
+  async ({ page, testAssets }) => {
+    testWithAssets.setTimeout(TIMEOUT.SLOW_TEST);
+    const { chartId, dashboardId } = await createSemanticDashboard(
+      page,
+      testAssets,
+      () => [],
+    );
+
+    const chartData = page.waitForResponse(
+      response =>
+        response.url().includes('/api/v1/chart/data') &&
+        response.request().method() === 'POST',
+    );
+    const dashboardPage = new DashboardPage(page);
+    await dashboardPage.gotoById(dashboardId);
+    await dashboardPage.waitForLoad();
+    expect((await chartData).status()).toBe(200);
+    await dashboardPage.waitForChartsToLoad();
+
+    const chart = dashboardPage.getChart(chartId);
+    await expect(rowFor(chart, 'Books')).toContainText('15');
+    await expect(rowFor(chart, 'Games')).toContainText('7');
+    await expect(rowFor(chart, 'Music')).toContainText('3');
+  },
+);
+
+testWithAssets(
+  'native filter on a semantic-view dimension filters the chart',
+  async ({ page, testAssets }) => {
+    testWithAssets.setTimeout(TIMEOUT.SLOW_TEST);
+    const { chartId, dashboardId } = await createSemanticDashboard(
+      page,
+      testAssets,
+      (view, id) => [
+        buildSelectFilter({
+          datasetId: view.viewId,
+          datasourceType: 'semantic_view',
+          column: 'region',
+          chartsInScope: [id],
+          name: 'Region',
+          defaultValue: 'West',

Review Comment:
   `defaultValue: 'West'` seeds the filter's `defaultDataMask.extraFormData`, so the chart receives the West predicate whether or not the filter control itself works for a semantic view. If the `datasourceType` forwarding added to `buildSelectFilter` regressed and the filter-bar options request fell back to a SQL dataset, every assertion below would still pass, because they only read the chart rows. Would it be worth also asserting that the Region control loads and shows `West` (or that its options request returns 200)?



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] test(e2e): add a stub semantic-layer provider for Playwright [superset]

Posted by "mikebridge (via GitHub)" <gi...@apache.org>.
mikebridge commented on code in PR #45103:
URL: https://github.com/apache/superset/pull/45103#discussion_r4254781485


##########
.github/workflows/bashlib.sh:
##########
@@ -295,6 +295,30 @@ playwright-run() {
   return $status
 }
 
+playwright-run-semantic() {
+  # The semantic-layer REST APIs only register when SEMANTIC_LAYERS is on, and
+  # Superset builds AppBuilder with update_perms=False, so their permissions
+  # exist only after a `superset init` that sees the flag. playwright_testdata
+  # ran init under the base test config, so sync roles again under this step's
+  # config before `playwright-run` boots gunicorn with it -- the same thing a
+  # deployment does after turning the flag on. Without it Admin gets 403 on
+  # POST /api/v1/semantic_layer/.

Review Comment:
   Thanks @aminghadersohi — agreed. Merged master in cd34a88a55 and removed the redundant `superset init` plus the stale bashlib/workflow comments in d7b612d238.
   
   Both semantic REST APIs are registered unconditionally on the merged tree. The base init calls `appbuilder.add_permissions(update_perms=True)` before syncing roles, so it already creates their permissions. The semantic stub config guard, step-scoped environment and required-stub check remain intact.
   
   The shell regression check and 24 focused stub tests pass, along with all branch-file hooks. [E2E run 38345256942](https://github.com/apache/superset/actions/runs/38345256942) passed on this exact head, including the Semantic View Tests at both `/` and `/app/prefix`, and both async jobs.
   
   _Checked and updated by Codex on Mike’s behalf._
   



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org