codeant-ai-for-open-source[bot] commented on code in PR #42328: URL: https://github.com/apache/superset/pull/42328#discussion_r3640081112
########## superset-frontend/playwright/tests/tools/capture-viz-thumbnails.spec.ts: ########## @@ -0,0 +1,309 @@ +/** + * 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. + */ +/** + * Crawls every dashboard on the target instance and refreshes viz-picker + * gallery thumbnails from live example charts. + * + * This is a maintenance tool, not a test: charts are DISCOVERED via the + * API (dashboards -> charts), so it keeps working as example dashboards + * evolve. For each viz type found, one representative chart is rendered + * standalone at 512x512 and the plugin's `thumbnail.png` is overwritten. + * The only static piece is the viz type -> image path map below, which + * changes when plugins are added or moved — never when examples change. + * Viz types found on dashboards but missing from the map are reported at + * the end without failing the run. + * + * It only runs when CAPTURE_THUMBNAILS=1 is set, so the regular + * Playwright suites never execute it. + * + * Usage (requires a running Superset with examples loaded): + * npm run playwright:thumbnails + * VIZ_TYPES=bullet,rose npm run playwright:thumbnails # subset + * + * Notes: + * - Only light-theme images are captured; `thumbnail-dark.png` variants + * are left untouched. + * - New gallery images (e.g. the Line percent-change example) still need + * to be registered in the plugin's metadata before they render in the + * gallery; the capture logs a reminder for any non-thumbnail output. + */ +import * as fs from 'fs'; +import * as path from 'path'; +import { test, expect, Page } from '@playwright/test'; + +const THUMBNAIL_SIZE = 512; +const RENDERED_CHART_SELECTOR = + '[data-test="chart-container"]:has(svg, canvas, table):not(:has([data-test="loading-indicator"]))'; Review Comment: **Suggestion:** The render-wait selector is too restrictive because it only accepts chart containers that contain `svg`, `canvas`, or `table`. Several viz types can render as plain HTML (for example text/metric style charts), so the wait can time out even when the chart is successfully rendered, causing false capture failures. Use a readiness condition that relies on chart-container visibility/loading completion rather than specific child element tags. [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Thumbnail capture fails for ag-grid-table example charts. - ⚠️ Maintenance workflow for gallery thumbnails becomes unreliable. ``` </details> <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Start a Superset instance with example dashboards loaded and an `ag-grid-table` example chart placed on a dashboard (this chart type is discovered via `discoverDashboardCharts` in `superset-frontend/playwright/tests/tools/capture-viz-thumbnails.spec.ts:183-207`). 2. Run the maintenance tool as documented: set `CAPTURE_THUMBNAILS=1` and execute `npm run playwright:thumbnails`, which runs the Playwright spec in `capture-viz-thumbnails.spec.ts:235-308`. 3. In the main loop at `capture-viz-thumbnails.spec.ts:272-287`, the `ag-grid-table` viz type is processed because it has a mapping in `VIZ_TYPE_THUMBNAILS` at lines 65-68, and `captureChart()` is called for its representative chart. 4. Inside `captureChart()` at lines 209-233, Playwright evaluates `page.locator(RENDERED_CHART_SELECTOR).first().waitFor({ state: 'visible', timeout: 60_000 })`; for `ag-grid-table` the chart container renders only div-based HTML (no `svg`, `canvas`, or `table`), so the selector defined at lines 51-52 never matches, `waitFor` times out, an exception is thrown, `failures` is populated, and the final assertion `expect(failures, failures.join('\n')).toEqual([])` at line 307 fails the run. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d0e58721a24f4f71904f83473ddd2270&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=d0e58721a24f4f71904f83473ddd2270&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/tools/capture-viz-thumbnails.spec.ts **Line:** 51:52 **Comment:** *Logic Error: The render-wait selector is too restrictive because it only accepts chart containers that contain `svg`, `canvas`, or `table`. Several viz types can render as plain HTML (for example text/metric style charts), so the wait can time out even when the chart is successfully rendered, causing false capture failures. Use a readiness condition that relies on chart-container visibility/loading completion rather than specific child element tags. 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%2F42328&comment_hash=29d5750340890a8dfa7fc72b03970bd952b8ff4ab4d81014147e07cda39cc49c&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42328&comment_hash=29d5750340890a8dfa7fc72b03970bd952b8ff4ab4d81014147e07cda39cc49c&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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
