-
Notifications
You must be signed in to change notification settings - Fork 16.6k
test(playwright): additional dataset list playwright tests #36684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
0c95457
0a8a24d
ba0f01b
faf0b9d
3954bbd
8bdd1d9
4198ff3
470938d
427cf75
fac22b4
3aeb24e
426772d
5c0ea58
7cd0913
9d3127e
a42cc22
f807ca4
50581a8
e8501d9
057cfe1
8a7a06f
ab506b6
381d817
d7123ce
881e60f
82e85db
d11046e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -74,6 +74,9 @@ export default defineConfig({ | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| viewport: { width: 1280, height: 1024 }, | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Accept downloads without prompts (needed for export tests) | ||||||||||||||||||||||
| acceptDownloads: true, | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Screenshots and videos on failure | ||||||||||||||||||||||
| screenshot: 'only-on-failure', | ||||||||||||||||||||||
| video: 'retain-on-failure', | ||||||||||||||||||||||
|
|
@@ -117,10 +120,19 @@ export default defineConfig({ | |||||||||||||||||||||
| // Web server setup - disabled in CI (Flask started separately in workflow) | ||||||||||||||||||||||
| webServer: process.env.CI | ||||||||||||||||||||||
| ? undefined | ||||||||||||||||||||||
| : { | ||||||||||||||||||||||
| command: 'curl -f http://localhost:8088/health', | ||||||||||||||||||||||
| url: 'http://localhost:8088/health', | ||||||||||||||||||||||
| reuseExistingServer: true, | ||||||||||||||||||||||
| timeout: 5000, | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
| : (() => { | ||||||||||||||||||||||
| // Support custom base URL (e.g., http://localhost:9012/app/prefix/) | ||||||||||||||||||||||
| const baseUrl = | ||||||||||||||||||||||
| process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:8088'; | ||||||||||||||||||||||
| // Extract origin (scheme + host + port) for health check | ||||||||||||||||||||||
| // Health endpoint is always at /health regardless of app prefix | ||||||||||||||||||||||
| const healthUrl = new URL('/health', new URL(baseUrl).origin).href; | ||||||||||||||||||||||
| return { | ||||||||||||||||||||||
| // Quote URL to prevent shell injection via PLAYWRIGHT_BASE_URL | ||||||||||||||||||||||
| command: `curl -f '${healthUrl}'`, | ||||||||||||||||||||||
| url: healthUrl, | ||||||||||||||||||||||
| reuseExistingServer: true, | ||||||||||||||||||||||
|
Comment on lines
+130
to
+134
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shell injection risk in webServer command
The curl command uses single quotes around the healthUrl, but if PLAYWRIGHT_BASE_URL contains single quotes, it could lead to shell injection. Switching to double quotes prevents this since URLs typically don't contain double quotes. Code suggestionCheck the AI-generated fix before applying
Suggested change
Code Review Run #819970 Should Bito avoid suggestions like this for future reviews? (Manage Rules)
|
||||||||||||||||||||||
| timeout: 5000, | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
| })(), | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| /** | ||
| * 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 { Locator, Page } from '@playwright/test'; | ||
| import { Button, Checkbox, Table } from '../core'; | ||
|
|
||
| const BULK_SELECT_SELECTORS = { | ||
| CONTROLS: '[data-test="bulk-select-controls"]', | ||
| ACTION: '[data-test="bulk-select-action"]', | ||
| } as const; | ||
|
|
||
| /** | ||
| * BulkSelect component for Superset ListView bulk operations. | ||
| * Provides a reusable interface for bulk selection and actions across list pages. | ||
| * | ||
| * @example | ||
| * const bulkSelect = new BulkSelect(page, table); | ||
| * await bulkSelect.enable(); | ||
| * await bulkSelect.selectRow('my-dataset'); | ||
| * await bulkSelect.selectRow('another-dataset'); | ||
| * await bulkSelect.clickAction('Delete'); | ||
| */ | ||
| export class BulkSelect { | ||
| private readonly page: Page; | ||
| private readonly table: Table; | ||
|
|
||
| constructor(page: Page, table: Table) { | ||
| this.page = page; | ||
| this.table = table; | ||
| } | ||
|
|
||
| /** | ||
| * Gets the "Bulk select" toggle button | ||
| */ | ||
| getToggleButton(): Button { | ||
| return new Button( | ||
| this.page, | ||
| this.page.getByRole('button', { name: 'Bulk select' }), | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Enables bulk selection mode by clicking the toggle button | ||
| */ | ||
| async enable(): Promise<void> { | ||
| await this.getToggleButton().click(); | ||
| } | ||
|
|
||
| /** | ||
| * Gets the checkbox for a row by name | ||
| * @param rowName - The name/text identifying the row | ||
| */ | ||
| getRowCheckbox(rowName: string): Checkbox { | ||
| const row = this.table.getRow(rowName); | ||
| return new Checkbox(this.page, row.getByRole('checkbox')); | ||
| } | ||
|
|
||
| /** | ||
| * Selects a row's checkbox in bulk select mode | ||
| * @param rowName - The name/text identifying the row to select | ||
| */ | ||
| async selectRow(rowName: string): Promise<void> { | ||
| await this.getRowCheckbox(rowName).check(); | ||
| } | ||
|
|
||
| /** | ||
| * Deselects a row's checkbox in bulk select mode | ||
| * @param rowName - The name/text identifying the row to deselect | ||
| */ | ||
| async deselectRow(rowName: string): Promise<void> { | ||
| await this.getRowCheckbox(rowName).uncheck(); | ||
| } | ||
|
|
||
| /** | ||
| * Gets the bulk select controls container locator (for assertions) | ||
| */ | ||
| getControls(): Locator { | ||
| return this.page.locator(BULK_SELECT_SELECTORS.CONTROLS); | ||
| } | ||
|
|
||
| /** | ||
| * Gets a bulk action button by name | ||
| * @param actionName - The name of the bulk action (e.g., "Export", "Delete") | ||
| */ | ||
| getActionButton(actionName: string): Button { | ||
| const controls = this.getControls(); | ||
| return new Button( | ||
| this.page, | ||
| controls.locator(BULK_SELECT_SELECTORS.ACTION, { hasText: actionName }), | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Clicks a bulk action button by name (e.g., "Export", "Delete") | ||
| * @param actionName - The name of the bulk action to click | ||
| */ | ||
| async clickAction(actionName: string): Promise<void> { | ||
| await this.getActionButton(actionName).click(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| /** | ||
| * 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. | ||
| */ | ||
|
|
||
| // ListView-specific Playwright Components for Superset | ||
| export { BulkSelect } from './BulkSelect'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| /** | ||
| * 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 { Locator, Page } from '@playwright/test'; | ||
|
|
||
| const ACE_EDITOR_SELECTORS = { | ||
| TEXT_INPUT: '.ace_text-input', | ||
| TEXT_LAYER: '.ace_text-layer', | ||
| CONTENT: '.ace_content', | ||
| SCROLLER: '.ace_scroller', | ||
| } as const; | ||
|
|
||
| /** | ||
| * AceEditor component for interacting with Ace Editor instances in Playwright. | ||
| * Uses the ace editor API directly for reliable text manipulation. | ||
| */ | ||
| export class AceEditor { | ||
| readonly page: Page; | ||
| private readonly selector: string; | ||
| private readonly locator: Locator; | ||
|
|
||
| constructor(page: Page, selector: string) { | ||
| this.page = page; | ||
| this.selector = selector; | ||
| this.locator = page.locator(selector); | ||
| } | ||
|
|
||
| /** | ||
| * Gets the editor element locator | ||
| */ | ||
| get element(): Locator { | ||
| return this.locator; | ||
| } | ||
|
|
||
| /** | ||
| * Waits for the ace editor to be fully loaded and ready for interaction. | ||
| */ | ||
| async waitForReady(): Promise<void> { | ||
| await this.locator.waitFor({ state: 'visible' }); | ||
| await this.locator.locator(ACE_EDITOR_SELECTORS.CONTENT).waitFor(); | ||
| await this.locator.locator(ACE_EDITOR_SELECTORS.TEXT_LAYER).waitFor(); | ||
| } | ||
|
|
||
| /** | ||
| * Sets text in the ace editor using the ace API. | ||
| * @param text - The text to set | ||
| */ | ||
| async setText(text: string): Promise<void> { | ||
| await this.waitForReady(); | ||
| const editorId = this.extractEditorId(); | ||
| await this.page.evaluate( | ||
| ({ id, value }) => { | ||
| const editor = (window as any).ace.edit(id); | ||
| editor.setValue(value, 1); | ||
| editor.session.getUndoManager().reset(); | ||
| }, | ||
| { id: editorId, value: text }, | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Gets the text content from the ace editor. | ||
| * @returns The text content | ||
| */ | ||
| async getText(): Promise<string> { | ||
| await this.waitForReady(); | ||
| const editorId = this.extractEditorId(); | ||
| return this.page.evaluate(id => { | ||
| const editor = (window as any).ace.edit(id); | ||
| return editor.getValue(); | ||
| }, editorId); | ||
| } | ||
|
|
||
| /** | ||
| * Clears the text in the ace editor. | ||
| */ | ||
| async clear(): Promise<void> { | ||
| await this.setText(''); | ||
| } | ||
|
|
||
| /** | ||
| * Appends text to the existing content in the ace editor. | ||
| * @param text - The text to append | ||
| */ | ||
| async appendText(text: string): Promise<void> { | ||
| await this.waitForReady(); | ||
| const editorId = this.extractEditorId(); | ||
| await this.page.evaluate( | ||
| ({ id, value }) => { | ||
| const editor = (window as any).ace.edit(id); | ||
| const currentText = editor.getValue(); | ||
| const newText = currentText + (currentText ? '\n' : '') + value; | ||
| editor.setValue(newText, 1); | ||
| }, | ||
| { id: editorId, value: text }, | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Focuses the ace editor. | ||
| */ | ||
| async focus(): Promise<void> { | ||
| await this.waitForReady(); | ||
| const editorId = this.extractEditorId(); | ||
| await this.page.evaluate(id => { | ||
| const editor = (window as any).ace.edit(id); | ||
| editor.focus(); | ||
| }, editorId); | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the editor is visible. | ||
| */ | ||
| async isVisible(): Promise<boolean> { | ||
| return this.locator.isVisible(); | ||
| } | ||
|
|
||
| /** | ||
| * Extracts the editor ID from the selector. | ||
| * Handles selectors like '#ace-editor' or '[id="ace-editor"]' | ||
| */ | ||
| private extractEditorId(): string { | ||
| // Handle #id format | ||
| if (this.selector.startsWith('#')) { | ||
| return this.selector.slice(1); | ||
| } | ||
| // Handle [id="..."] format | ||
| const idMatch = this.selector.match(/id="([^"]+)"/); | ||
| if (idMatch) { | ||
| return idMatch[1]; | ||
| } | ||
| // Handle [data-test="..."] format - use the element's actual id | ||
| // This requires getting it from the DOM | ||
| return this.selector.replace(/[#[\]]/g, ''); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shell injection vulnerability: The
healthUrlvariable is constructed fromprocess.env.PLAYWRIGHT_BASE_URLand directly interpolated into a shell command with single quotes. While single quotes prevent most shell injection, a malicious URL containing single quotes could break out of the quoted string. Use a safer approach such as using Node.js's child_process with array arguments or properly escaping the URL.