You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "sadpandajoe (via GitHub)" <gi...@apache.org> on 2026/06/25 19:26:21 UTC
[PR] test(dashboard): migrate dashboard force-refresh control to Playwright [superset]
sadpandajoe opened a new pull request, #41433:
URL: https://github.com/apache/superset/pull/41433
### SUMMARY
Migrates the genuine end-to-end case from the legacy Cypress **Dashboard top-level controls** suite (`dashboard/controls.test.ts`) to the existing Playwright framework, as part of the dashboard Cypress→Playwright migration.
The suite had two cases:
- **`should allow dashboard level force refresh`** — a true E2E behaviour: the dashboard-level force refresh must re-query every chart with `force=true`, bypassing the cache. **Migrated here.**
- **`should allow chart level refresh`** — only asserted the menu item's `ant-dropdown-menu-item-disabled` class with no backend round-trip, and was already `it.skip`-ped as flaky. It belongs in component/RTL coverage and is **not** migrated.
Design decisions:
- The dashboard is built from scratch via the API (`testWithAssets` fixture), so the test is hermetic, self-cleaning, and deterministic. Initial load warms the cache; the force refresh must then bypass it.
- The test inspects the real `POST /api/v1/chart/data?...&force=true` round-trips and asserts each one returns `200` with a freshly computed (uncached) result. The backend reports an uncached result as a falsy `is_cached` (it returns `null` in current master; the legacy Cypress test asserted `=== false`), whereas a cached result reports `true` — so the assertion is `is_cached` is falsy, which fails correctly if force refresh ever serves stale cache.
- Adds `DashboardPage.forceRefresh()`, a reusable page-object method that triggers the force refresh through the header actions menu.
### TESTING INSTRUCTIONS
```
cd superset-frontend
PLAYWRIGHT_BASE_URL=http://localhost:8088 npx playwright test --project=chromium dashboard/dashboard-controls.spec.ts
```
Verified locally against a running Superset stack (passes in ~7s).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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(dashboard): migrate dashboard force-refresh control to Playwright [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #41433:
URL: https://github.com/apache/superset/pull/41433
--
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(dashboard): migrate dashboard force-refresh control to Playwright [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41433:
URL: https://github.com/apache/superset/pull/41433#issuecomment-4803501230
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41433?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 64.44%. Comparing base ([`9965c05`](https://app.codecov.io/gh/apache/superset/commit/9965c0569952e25bbb8a25894bcaaa443dc75583?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`cd35852`](https://app.codecov.io/gh/apache/superset/commit/cd35852070acf5f96079a9cc2be892234026b687?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #41433 +/- ##
=======================================
Coverage 64.44% 64.44%
=======================================
Files 2662 2662
Lines 145883 145883
Branches 33664 33664
=======================================
Hits 94018 94018
Misses 50163 50163
Partials 1702 1702
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/41433/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/41433/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.84% <ø> (ø)` | |
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/41433?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(dashboard): migrate dashboard force-refresh control to Playwright [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41433:
URL: https://github.com/apache/superset/pull/41433#discussion_r3642052813
##########
superset-frontend/playwright/tests/dashboard/dashboard-test-helpers.ts:
##########
@@ -72,3 +105,87 @@ export async function createTestDashboard(
return { id, name };
}
+
+export interface DashboardChartSpec {
+ /** Sent as the chart's top-level `viz_type` and injected into its params. */
+ viz_type: string;
+ /**
+ * Chart params minus `datasource` and `viz_type` — the helper injects both
+ * (the datasource is resolved from the dataset, so callers never thread the
+ * dataset id through their spec).
+ */
+ params: Record<string, unknown>;
+}
+
+interface CreateDashboardWithChartsOptions {
+ /** Example dataset the charts query (e.g. 'birth_names'). */
+ datasetName: string;
+ /** Chart slice-name prefix: `${chartNamePrefix}_${viz_type}_${suffix}`. */
+ chartNamePrefix: string;
+ /** Dashboard title prefix: `${dashboardTitlePrefix}_${suffix}`. */
+ dashboardTitlePrefix: string;
+ chartSpecs: DashboardChartSpec[];
+}
+
+/**
+ * Builds a published dashboard via the API: creates each chart, lays them out in
+ * a single row, and associates them so they render. Every created chart and the
+ * dashboard are registered for fixture cleanup. Charts are returned in the same
+ * order as `chartSpecs`, so callers can pair them back to per-spec metadata by
+ * index.
+ */
+export async function createDashboardWithCharts(
+ page: Page,
+ testAssets: TestAssets,
+ testInfo: TestInfo,
+ options: CreateDashboardWithChartsOptions,
+): Promise<{ dashboardId: number; charts: DashboardLayoutChart[] }> {
+ const dataset = await getDatasetByName(page, options.datasetName);
+ if (!dataset) {
+ throw new Error(`Dataset ${options.datasetName} not found`);
+ }
+ const datasource = `${dataset.id}__table`;
+
+ // Parallel-safe suffix so chart/dashboard names never collide across workers.
+ const uniqueSuffix = `${Date.now()}_${testInfo.parallelIndex}`;
+
+ const charts: DashboardLayoutChart[] = [];
+ for (const spec of options.chartSpecs) {
+ const sliceName = `${options.chartNamePrefix}_${spec.viz_type}_${uniqueSuffix}`;
+ const resp = await apiPostChart(page, {
+ slice_name: sliceName,
+ viz_type: spec.viz_type,
+ datasource_id: dataset.id,
+ datasource_type: 'table',
+ params: JSON.stringify({
+ // Caller params first so the helper-owned datasource/viz_type always win
+ // and a stray key in a spec cannot repoint the chart at another dataset.
+ ...spec.params,
+ datasource,
+ viz_type: spec.viz_type,
+ }),
+ });
+ expect(resp.ok()).toBe(true);
+ const chartId = await extractIdFromResponse(resp);
+ testAssets.trackChart(chartId);
+ charts.push({ id: chartId, sliceName });
+ }
+
+ // Lay all charts out in a single row.
+ const positionJson = buildSingleRowDashboardLayout(charts);
+ const dashResp = await apiPostDashboard(page, {
+ dashboard_title: `${options.dashboardTitlePrefix}_${uniqueSuffix}`,
+ published: true,
+ position_json: JSON.stringify(positionJson),
+ });
+ expect(dashResp.ok()).toBe(true);
+ const dashboardId = await extractIdFromResponse(dashResp);
+ testAssets.trackDashboard(dashboardId);
+
+ // Associate every chart with the dashboard so they actually render.
+ for (const chart of charts) {
+ await apiPutChart(page, chart.id, { dashboards: [dashboardId] });
+ }
Review Comment:
**Suggestion:** The chart-to-dashboard association API calls are awaited but their success is never validated, so failed associations are silently ignored and downstream tests fail later with misleading symptoms. Validate each response (or throw) in this loop to fail at the true point of error. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- critical Dashboard force-refresh Playwright test may mask setup failures.
- warning Future dashboard E2Es may misdiagnose chart association issues.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Playwright test
`superset-frontend/playwright/tests/dashboard/dashboard-controls.spec.ts` (the new
force-refresh E2E) which calls `createDashboardWithCharts` from
`superset-frontend/playwright/tests/dashboard/dashboard-test-helpers.ts` at lines 137–191.
2. Inside `createDashboardWithCharts`, the helper successfully creates charts via
`apiPostChart` (lines 153–172, responses validated with `expect(resp.ok()).toBe(true)`),
then creates a dashboard via `apiPostDashboard` (lines 175–181, also validated).
3. The helper then associates each chart to the dashboard in the loop at lines 186–188:
`for (const chart of charts) { await apiPutChart(page, chart.id, { dashboards:
[dashboardId] }); }`. Unlike the previous API calls, these `apiPutChart` responses are not
checked for `ok()` or any status, so a failing association (for example, a 500 or 403
returned by `/api/v1/chart/{id}` due to backend or permission issues) is silently ignored
and the helper still returns `{ dashboardId, charts }`.
4. When the calling test proceeds to `dashboard.gotoById(dashboardId)` and
`dashboard.waitForChartsToLoad()` (lines 88–90 in `dashboard-controls.spec.ts`), the
dashboard may render with missing or unassociated charts. Subsequent assertions that every
chart issues forced `/api/v1/chart/data?force=true` requests (lines 134–149) will then
fail with misleading messages about missing slice IDs or timeouts, rather than clearly
indicating that the chart-to-dashboard association API failed in the setup loop. Adding
explicit validation, e.g. `expect(resp.ok()).toBe(true)` inside the loop, would cause the
test to fail immediately at the true point of error.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=da8c6f1143954773abec8667790f0963&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=da8c6f1143954773abec8667790f0963&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/playwright/tests/dashboard/dashboard-test-helpers.ts
**Line:** 186:188
**Comment:**
*Possible Bug: The chart-to-dashboard association API calls are awaited but their success is never validated, so failed associations are silently ignored and downstream tests fail later with misleading symptoms. Validate each response (or throw) in this loop to fail at the true point of error.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41433&comment_hash=53a666e95c2b2f145ef6543447199ce9026b0bf771767c766ceb2f305f654c0f&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41433&comment_hash=53a666e95c2b2f145ef6543447199ce9026b0bf771767c766ceb2f305f654c0f&reaction=dislike'>👎</a>
--
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(dashboard): migrate dashboard force-refresh control to Playwright [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41433:
URL: https://github.com/apache/superset/pull/41433#issuecomment-5074471823
**Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it
--
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(dashboard): migrate dashboard force-refresh control to Playwright [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41433:
URL: https://github.com/apache/superset/pull/41433#issuecomment-5064622399
<!-- Bito Reply -->
The flagged issue is correct. The `apiPutChart` calls in the loop at lines 514–516 of `superset-frontend/playwright/tests/dashboard/dashboard-test-helpers.ts` do not validate the response, which can lead to silent failures during dashboard setup.
To resolve this, you should add an assertion to verify that each API call succeeds, similar to how other API calls in the same file are handled. Here is the corrected loop:
```typescript
// Associate every chart with the dashboard so they actually render.
for (const chart of charts) {
const resp = await apiPutChart(page, chart.id, { dashboards: [dashboardId] });
expect(resp.ok()).toBe(true);
}
```
I have checked the PR comments and there are no other actionable suggestions to address at this time.
**superset-frontend/playwright/tests/dashboard/dashboard-test-helpers.ts**
```
// Associate every chart with the dashboard so they actually render.
for (const chart of charts) {
const resp = await apiPutChart(page, chart.id, { dashboards: [dashboardId] });
expect(resp.ok()).toBe(true);
}
```
--
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(dashboard): migrate dashboard force-refresh control to Playwright [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41433:
URL: https://github.com/apache/superset/pull/41433#issuecomment-5065507453
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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(dashboard): migrate dashboard force-refresh control to Playwright [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #41433:
URL: https://github.com/apache/superset/pull/41433#issuecomment-5074058915
The bot re-run came back clean, so still LGTM. All yours to merge, @sadpandajoe!
--
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(dashboard): migrate dashboard force-refresh control to Playwright [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41433:
URL: https://github.com/apache/superset/pull/41433#issuecomment-4803429099
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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(dashboard): migrate dashboard force-refresh control to Playwright [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #41433:
URL: https://github.com/apache/superset/pull/41433#discussion_r3642089324
##########
superset-frontend/playwright/tests/dashboard/dashboard-test-helpers.ts:
##########
@@ -72,3 +105,87 @@ export async function createTestDashboard(
return { id, name };
}
+
+export interface DashboardChartSpec {
+ /** Sent as the chart's top-level `viz_type` and injected into its params. */
+ viz_type: string;
+ /**
+ * Chart params minus `datasource` and `viz_type` — the helper injects both
+ * (the datasource is resolved from the dataset, so callers never thread the
+ * dataset id through their spec).
+ */
+ params: Record<string, unknown>;
+}
+
+interface CreateDashboardWithChartsOptions {
+ /** Example dataset the charts query (e.g. 'birth_names'). */
+ datasetName: string;
+ /** Chart slice-name prefix: `${chartNamePrefix}_${viz_type}_${suffix}`. */
+ chartNamePrefix: string;
+ /** Dashboard title prefix: `${dashboardTitlePrefix}_${suffix}`. */
+ dashboardTitlePrefix: string;
+ chartSpecs: DashboardChartSpec[];
+}
+
+/**
+ * Builds a published dashboard via the API: creates each chart, lays them out in
+ * a single row, and associates them so they render. Every created chart and the
+ * dashboard are registered for fixture cleanup. Charts are returned in the same
+ * order as `chartSpecs`, so callers can pair them back to per-spec metadata by
+ * index.
+ */
+export async function createDashboardWithCharts(
+ page: Page,
+ testAssets: TestAssets,
+ testInfo: TestInfo,
+ options: CreateDashboardWithChartsOptions,
+): Promise<{ dashboardId: number; charts: DashboardLayoutChart[] }> {
+ const dataset = await getDatasetByName(page, options.datasetName);
+ if (!dataset) {
+ throw new Error(`Dataset ${options.datasetName} not found`);
+ }
+ const datasource = `${dataset.id}__table`;
+
+ // Parallel-safe suffix so chart/dashboard names never collide across workers.
+ const uniqueSuffix = `${Date.now()}_${testInfo.parallelIndex}`;
+
+ const charts: DashboardLayoutChart[] = [];
+ for (const spec of options.chartSpecs) {
+ const sliceName = `${options.chartNamePrefix}_${spec.viz_type}_${uniqueSuffix}`;
+ const resp = await apiPostChart(page, {
+ slice_name: sliceName,
+ viz_type: spec.viz_type,
+ datasource_id: dataset.id,
+ datasource_type: 'table',
+ params: JSON.stringify({
+ // Caller params first so the helper-owned datasource/viz_type always win
+ // and a stray key in a spec cannot repoint the chart at another dataset.
+ ...spec.params,
+ datasource,
+ viz_type: spec.viz_type,
+ }),
+ });
+ expect(resp.ok()).toBe(true);
+ const chartId = await extractIdFromResponse(resp);
+ testAssets.trackChart(chartId);
+ charts.push({ id: chartId, sliceName });
+ }
+
+ // Lay all charts out in a single row.
+ const positionJson = buildSingleRowDashboardLayout(charts);
+ const dashResp = await apiPostDashboard(page, {
+ dashboard_title: `${options.dashboardTitlePrefix}_${uniqueSuffix}`,
+ published: true,
+ position_json: JSON.stringify(positionJson),
+ });
+ expect(dashResp.ok()).toBe(true);
+ const dashboardId = await extractIdFromResponse(dashResp);
+ testAssets.trackDashboard(dashboardId);
+
+ // Associate every chart with the dashboard so they actually render.
+ for (const chart of charts) {
+ await apiPutChart(page, chart.id, { dashboards: [dashboardId] });
+ }
Review Comment:
The PUT is already validated one layer down: `apiPutChart` → `apiPut` sets `failOnStatusCode: options?.failOnStatusCode ?? true` (`playwright/helpers/api/requests.ts:194`), so a non-2xx/3xx throws inside the loop rather than being ignored, and `withTransientRetry` rethrows non-transient errors. The `expect(resp.ok())` calls on the POSTs above are redundant for the same reason (`apiPost` shares that default), so adding one here would assert a condition that can never be false. Keeping the loop as-is.
--
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(dashboard): migrate dashboard force-refresh control to Playwright [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #41433:
URL: https://github.com/apache/superset/pull/41433#issuecomment-4861606575
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 8afa5cf830b98e74e82d6d51574a0a78ad5ea9cc |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a45ca0cc12a86000778c3b9 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-41433--superset-docs-preview.netlify.app](https://deploy-preview-41433--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><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#q1YqKMrPLShRslJS0lFKKkrMS84AslMSizOS8hOLUnQLynWT8_NKivJzioEKUlILcvIrPVOASswSTUyTEw2Skw2NEi3MDAwMzM0tko2TLJVqAQ) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-41433--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