diff --git a/superset-frontend/scripts/check-custom-rules.js b/superset-frontend/scripts/check-custom-rules.js index d3502ace105f..d623cd319261 100755 --- a/superset-frontend/scripts/check-custom-rules.js +++ b/superset-frontend/scripts/check-custom-rules.js @@ -650,32 +650,141 @@ function checkUntranslatedStrings(ast, filepath) { } /** - * Process a single file + * Whether a node is a `userEvent.setup()` call. */ -function processFile(filepath) { - const code = fs.readFileSync(filepath, 'utf8'); +function isUserEventSetupCall(node) { + return ( + !!node && + node.type === 'CallExpression' && + node.callee.type === 'MemberExpression' && + node.callee.object.type === 'Identifier' && + node.callee.object.name === 'userEvent' && + node.callee.property.type === 'Identifier' && + node.callee.property.name === 'setup' + ); +} + +/** + * Whether an identifier resolves to a `userEvent.setup()` session. + * + * Only a declarator initialized from the call counts. A session assigned + * separately from its declaration (`let user; user = userEvent.setup()`) is not + * recognized, and no test file uses that form. + * + * @returns true when `name` is bound to a user-event session in this scope + */ +function isUserEventSession(scope, name) { + const binding = scope.getBinding(name); + return ( + !!binding && + binding.path.node.type === 'VariableDeclarator' && + isUserEventSetupCall(binding.path.node.init) + ); +} +/** + * Check that `userEvent` interactions are awaited. + * + * Every `@testing-library/user-event` API returns a promise, so a call used as a + * bare statement is fire-and-forget: its events are still being dispatched when + * the next query or assertion runs. That either charges the dispatch time to a + * later `waitFor` budget or lets an assertion observe the state from before the + * interaction, and both surface as flaky tests. + * + * Both call styles are checked: `userEvent.click(el)` and the session style, + * `const user = userEvent.setup(); user.click(el)`. The session receiver is + * resolved through the scope, so only a binding initialized from + * `userEvent.setup()` is treated as one. + * + * Only bare expression statements are reported. A call whose promise is stored + * or returned (`const pending = userEvent.click(el)`) is left alone, because it + * may be awaited elsewhere. `userEvent.setup()` is itself exempt: it is + * synchronous and returns a session object rather than a promise. + */ +function checkAwaitedUserEvent(ast, filepath) { + traverse(ast, { + ExpressionStatement(path) { + const { expression } = path.node; + if ( + expression.type !== 'CallExpression' || + expression.callee.type !== 'MemberExpression' + ) { + return; + } + const { object, property } = expression.callee; + if (object.type !== 'Identifier' || property.type !== 'Identifier') { + return; + } + if ( + object.name === 'userEvent' + ? property.name === 'setup' + : !isUserEventSession(path.scope, object.name) + ) { + return; + } + const line = path.node.loc ? path.node.loc.start.line : 0; + // eslint-disable-next-line no-console + console.error( + `${RED}✖${RESET} ${filepath}:${line}: un-awaited ${object.name}.${property.name}(). ` + + `user-event APIs return promises; await the call so its events are ` + + `dispatched before the next query or assertion.`, + ); + errorCount += 1; + }, + }); +} + +/** + * Parse a file, reporting an unparseable file as a warning. + * + * @returns the AST, or null when the file could not be parsed + */ +function parseFile(filepath) { try { - const ast = parser.parse(code, { + return parser.parse(fs.readFileSync(filepath, 'utf8'), { sourceType: 'module', plugins: ['jsx', 'typescript', 'decorators-legacy'], attachComments: true, }); - - // Run all checks - checkNoLiteralColors(ast, filepath); - checkNoFaIcons(ast, filepath); - checkNoDirectAntdImports(ast, filepath); - checkI18nTemplates(ast, filepath); - checkEagerTranslationsInConfig(ast, filepath); - checkUntranslatedStrings(ast, filepath); } catch (error) { // eslint-disable-next-line no-console console.warn( `${YELLOW}⚠${RESET} Could not parse ${filepath}: ${error.message}`, ); warningCount += 1; + return null; + } +} + +/** + * Process a single file + */ +function processFile(filepath) { + const ast = parseFile(filepath); + if (!ast) { + return; } + + // Run all checks + checkNoLiteralColors(ast, filepath); + checkNoFaIcons(ast, filepath); + checkNoDirectAntdImports(ast, filepath); + checkI18nTemplates(ast, filepath); + checkEagerTranslationsInConfig(ast, filepath); + checkUntranslatedStrings(ast, filepath); +} + +/** + * Process a single test file. The checks in `processFile` deliberately skip + * tests, so the test-only rules run from here instead. + */ +function processTestFile(filepath) { + const ast = parseFile(filepath); + if (!ast) { + return; + } + + checkAwaitedUserEvent(ast, filepath); } /** @@ -685,6 +794,11 @@ function processFile(filepath) { const TS_ONLY_SOURCE_PATTERN = /^(src|packages\/[^/]+\/src|plugins\/[^/]+\/src)\//; +/** + * Jest test and spec files, which the test-only rules apply to. + */ +const TEST_FILE_PATTERN = /\.(test|spec)\.(ts|tsx|js|jsx)$/; + /** * Enforce the TypeScript-only frontend convention: no `.js`/`.jsx` files may be * added under the application source trees (including test files). Build @@ -757,6 +871,30 @@ function main() { : args.map(f => f.replace(/^superset-frontend\//, '')); checkTypeScriptOnlySource(tsOnlyCandidates); + // Run the test-only rules. Like the TypeScript-only check above, this works + // from the raw file list, because the ignore patterns below strip out tests. + const testCandidates = + args.length === 0 + ? glob.sync('**/*.{test,spec}.{ts,tsx,js,jsx}', { + ignore: [ + '**/node_modules/**', + '**/esm/**', + '**/lib/**', + '**/dist/**', + ], + }) + : args + .map(f => f.replace(/^superset-frontend\//, '')) + .filter(f => TEST_FILE_PATTERN.test(f)); + testCandidates.forEach(file => { + const resolvedPath = path.resolve(file); + if (fs.existsSync(resolvedPath)) { + processTestFile(resolvedPath); + } else if (fs.existsSync(file)) { + processTestFile(file); + } + }); + // If no files specified, check all if (files.length === 0) { files = glob.sync('src/**/*.{ts,tsx,js,jsx}', { @@ -832,4 +970,5 @@ export default { checkI18nTemplates, checkUntranslatedStrings, checkTypeScriptOnlySource, + checkAwaitedUserEvent, }; diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.test.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.test.tsx index 3e2f72ec1f2a..b870fc2fd0a6 100644 --- a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.test.tsx +++ b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.test.tsx @@ -349,7 +349,7 @@ test('semantic-view datasource resolves via the structure endpoint and renders t }, }); - userEvent.click(screen.getByTestId('open-semantic-context-menu')); + await userEvent.click(screen.getByTestId('open-semantic-context-menu')); // The menu opens without crashing on the dimension-derived shape. expect(await screen.findByRole('menu')).toBeInTheDocument(); diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/Header/Header.test.tsx index 1aaab1469877..3b6cbd1e6c83 100644 --- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx @@ -832,8 +832,8 @@ test('shows Excel export progress after the header menu closes', async () => { }); await openActionsDropdown(); - userEvent.hover(screen.getByText('Download')); - userEvent.click(await screen.findByText('Export Data to Excel')); + await userEvent.hover(screen.getByText('Download')); + await userEvent.click(await screen.findByText('Export Data to Excel')); await waitFor(() => { expect(addInfoToast).toHaveBeenCalledWith( diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/GroupByFilterCard.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/GroupByFilterCard.test.tsx index 54a920f8a93f..e756a7e03ad3 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/GroupByFilterCard.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/GroupByFilterCard.test.tsx @@ -142,7 +142,7 @@ test('maps columns to options honouring filterable and verbose_name', async () = ); const combobox = await screen.findByRole('combobox'); - userEvent.click(combobox); + await userEvent.click(combobox); expect(await screen.findByText('Deal Size')).toBeInTheDocument(); expect(screen.getByText('city')).toBeInTheDocument(); @@ -160,7 +160,7 @@ test('fetch failure fires the danger toast and leaves options empty', async () = expect(String(mockedAddDangerToast.mock.calls[0][0])).toMatch(/303/); const combobox = await screen.findByRole('combobox'); - userEvent.click(combobox); + await userEvent.click(combobox); expect(screen.queryByText('Deal Size')).not.toBeInTheDocument(); }); @@ -217,7 +217,7 @@ test('semantic-view target lists only the view dimensions under an id collision' renderCard([{ datasetId: 306, datasourceType: DatasourceType.SemanticView }]); const combobox = await screen.findByRole('combobox'); - userEvent.click(combobox); + await userEvent.click(combobox); expect(await screen.findByText('Orders Status')).toBeInTheDocument(); expect(screen.queryByText('address_line1')).not.toBeInTheDocument(); @@ -239,8 +239,8 @@ test('selecting a dimension persists a target that still carries datasourceType' renderCard([{ datasetId: 307, datasourceType: DatasourceType.SemanticView }]); const combobox = await screen.findByRole('combobox'); - userEvent.click(combobox); - userEvent.click(await screen.findByText('Orders Users City')); + await userEvent.click(combobox); + await userEvent.click(await screen.findByText('Orders Users City')); await waitFor(() => expect(setPendingChartCustomization).toHaveBeenCalled()); const persisted = @@ -268,8 +268,8 @@ test('clearing the selection keeps the datasource binding intact', async () => { renderCard([{ datasetId: 308, datasourceType: DatasourceType.SemanticView }]); const combobox = await screen.findByRole('combobox'); - userEvent.click(combobox); - userEvent.click(await screen.findByText('Orders State')); + await userEvent.click(combobox); + await userEvent.click(await screen.findByText('Orders State')); await waitFor(() => expect(setPendingChartCustomization).toHaveBeenCalled()); setPendingChartCustomization.mockClear(); @@ -277,8 +277,10 @@ test('clearing the selection keeps the datasource binding intact', async () => { // datasource binding rather than collapsing to an empty object. After // selection the label exists twice (selection tag + dropdown option); // toggle via the option role. - userEvent.click(combobox); - userEvent.click(await screen.findByRole('option', { name: 'Orders State' })); + await userEvent.click(combobox); + await userEvent.click( + await screen.findByRole('option', { name: 'Orders State' }), + ); await waitFor(() => expect(setPendingChartCustomization).toHaveBeenCalled()); const cleared = setPendingChartCustomization.mock.calls[ @@ -314,7 +316,7 @@ test('semantic structure failure renders empty options and toasts exactly once w expect(mockedAddDangerToast).toHaveBeenCalledTimes(1); const combobox = await screen.findByRole('combobox'); - userEvent.click(combobox); + await userEvent.click(combobox); expect(screen.queryByText('Orders Status')).not.toBeInTheDocument(); // Never a cross-type fallback. expect(fetchMock.callHistory.calls('glob:*/api/v1/dataset/*')).toHaveLength( @@ -348,7 +350,7 @@ test('switching from a failed binding to a healthy one never toasts the healthy // strictly after the switch, so any spurious toast would already have fired // and been counted by the time it settles (no wall-clock sleep needed). const combobox = await screen.findByRole('combobox'); - userEvent.click(combobox); + await userEvent.click(combobox); expect(await screen.findByText('city')).toBeInTheDocument(); // No toast ever names the healthy datasource 311. diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.test.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.test.tsx index 91ce34eea261..f4e40f27b0e5 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.test.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.test.tsx @@ -360,7 +360,7 @@ const renderSemanticPopover = ( const openDimensionsDropdown = async () => { const combobox = screen.getByRole('combobox', { name: 'Dimensions' }); - userEvent.click(combobox); + await userEvent.click(combobox); await getDropdown(); return combobox; }; @@ -410,8 +410,10 @@ test('Simple-only callers can select a saved-only semantic dimension', async () ); } await openDimensionsDropdown(); - userEvent.click(within(await getDropdown()).getByText('Product Category')); - userEvent.click(screen.getByRole('button', { name: 'Save' })); + await userEvent.click( + within(await getDropdown()).getByText('Product Category'), + ); + await userEvent.click(screen.getByRole('button', { name: 'Save' })); await waitFor(() => expect(onChange).toHaveBeenCalledWith(SEMANTIC_COLUMNS[1]), ); @@ -492,10 +494,10 @@ test('lists every expression-less dimension as a Saved option without mutating m expect(within(dropdown).getByText('Product Category')).toBeInTheDocument(); expect(within(dropdown).getByText('region')).toBeInTheDocument(); - userEvent.click(within(dropdown).getByText('Order Date')); + await userEvent.click(within(dropdown).getByText('Order Date')); const saveButton = screen.getByTestId('ColumnEdit#save'); await waitFor(() => expect(saveButton).toBeEnabled()); - userEvent.click(saveButton); + await userEvent.click(saveButton); await waitFor(() => expect(onChange).toHaveBeenCalledWith(SEMANTIC_COLUMNS[0]), @@ -551,7 +553,7 @@ test('clearing the Saved-only dimension resets the selection', async () => { ) as HTMLElement; expect(clearButton).toBeInTheDocument(); - userEvent.click(clearButton); + await userEvent.click(clearButton); await waitFor(() => expect(setLabel).toHaveBeenCalledWith('')); }); @@ -585,7 +587,7 @@ test('clearing the Simple-mode item resets the selection', async () => { ) as HTMLElement; expect(clearButton).toBeInTheDocument(); - userEvent.click(clearButton); + await userEvent.click(clearButton); await waitFor(() => expect(setLabel).toHaveBeenCalledWith('')); }); @@ -723,10 +725,10 @@ test('an edited dimension that became incompatible cannot be saved until replace await openDimensionsDropdown(); const dropdown = await getDropdown(); - userEvent.click(within(dropdown).getByText('Order Date')); + await userEvent.click(within(dropdown).getByText('Order Date')); await waitFor(() => expect(saveButton).toBeEnabled()); - userEvent.click(saveButton); + await userEvent.click(saveButton); await waitFor(() => expect(onChange).toHaveBeenCalledWith(SEMANTIC_COLUMNS[0]), ); @@ -761,10 +763,10 @@ test('a legacy edited adhoc value opens Saved, stays inspectable, and blocks Sav fireEvent.click(screen.getByRole('tab', { name: 'Saved' })); await openDimensionsDropdown(); const dropdown = await getDropdown(); - userEvent.click(within(dropdown).getByText('Order Date')); + await userEvent.click(within(dropdown).getByText('Order Date')); await waitFor(() => expect(saveButton).toBeEnabled()); - userEvent.click(saveButton); + await userEvent.click(saveButton); await waitFor(() => expect(onChange).toHaveBeenCalledWith(SEMANTIC_COLUMNS[0]), ); @@ -806,7 +808,7 @@ test('disables Saved metrics by the compatible-metric list, not the dimension li const combobox = screen.getByRole('combobox', { name: 'Dimensions and metrics', }); - userEvent.click(combobox); + await userEvent.click(combobox); expect(await getOptionItem('Total Sales')).not.toHaveClass( 'ant-select-item-option-disabled', @@ -827,7 +829,7 @@ test('a metrics-only semantic view still renders the Saved select', async () => const combobox = screen.getByRole('combobox', { name: 'Dimensions and metrics', }); - userEvent.click(combobox); + await userEvent.click(combobox); expect(await getOptionItem('Total Sales')).toBeInTheDocument(); expect(await getOptionItem('Tax Amount')).toBeInTheDocument(); @@ -868,7 +870,7 @@ test('table datasources keep expression-based classification and enabled modes', const combobox = screen.getByRole('combobox', { name: 'Columns and metrics', }); - userEvent.click(combobox); + await userEvent.click(combobox); const dropdown = await getDropdown(); expect(within(dropdown).getByText('plain_col')).toBeInTheDocument(); expect(within(dropdown).queryByText('calc_col')).not.toBeInTheDocument(); @@ -934,17 +936,17 @@ test('a feature-declaring semantic view keeps expression-based classification an const simpleCombobox = screen.getByRole('combobox', { name: 'Columns and metrics', }); - userEvent.click(simpleCombobox); + await userEvent.click(simpleCombobox); const dropdown = await getDropdown(); expect(within(dropdown).getByText('Plain Dimension')).toBeInTheDocument(); expect( within(dropdown).queryByText('calc_dimension'), ).not.toBeInTheDocument(); - userEvent.click(within(dropdown).getByText('Plain Dimension')); + await userEvent.click(within(dropdown).getByText('Plain Dimension')); const saveButton = screen.getByTestId('ColumnEdit#save'); await waitFor(() => expect(saveButton).toBeEnabled()); - userEvent.click(saveButton); + await userEvent.click(saveButton); await waitFor(() => expect(onChange).toHaveBeenCalledWith(columns[0])); expect(screen.queryByRole('alert')).not.toBeInTheDocument(); @@ -976,7 +978,7 @@ test('non-semantic datasources still filter Saved options by compatibility metad { store }, ); - userEvent.click( + await userEvent.click( screen.getByRole('combobox', { name: 'Columns and metrics' }), ); const dropdown = await getDropdown(); diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx index e63a9e64278c..0fd49b66f67f 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx @@ -649,7 +649,7 @@ test('saved-only semantic view disables Simple and Custom SQL modes in the picke { useDndKit: true, store: semanticViewStore() }, ); - userEvent.click(screen.getByText(/Drop columns here or click/i)); + await userEvent.click(screen.getByText(/Drop columns here or click/i)); await waitFor(() => { expect(screen.getByRole('tab', { name: 'Saved' })).toBeInTheDocument(); @@ -678,7 +678,7 @@ test('semantic view declaring adhoc expressions keeps existing modes in the pick }, ); - userEvent.click(screen.getByText(/Drop columns here or click/i)); + await userEvent.click(screen.getByText(/Drop columns here or click/i)); await waitFor(() => { expect(screen.getByRole('tab', { name: 'Simple' })).toBeInTheDocument(); @@ -710,21 +710,21 @@ test('commits a visible Cube dimension in two interactions after opening the pic { useDndKit: true, store: semanticViewStore() }, ); - userEvent.click(screen.getByText(/Drop columns here or click/i)); + await userEvent.click(screen.getByText(/Drop columns here or click/i)); const combobox = await screen.findByRole('combobox', { name: 'Dimensions', }); // Interaction 1: select the visible dimension. - userEvent.click(combobox); + await userEvent.click(combobox); const option = await screen.findByRole('option', { name: /Order Date/i }); - userEvent.click(option); + await userEvent.click(option); // Interaction 2: save. const saveButton = await screen.findByTestId('ColumnEdit#save'); await waitFor(() => expect(saveButton).toBeEnabled()); - userEvent.click(saveButton); + await userEvent.click(saveButton); await waitFor(() => { expect(mockOnChange).toHaveBeenCalledWith(['order_date']); @@ -743,7 +743,7 @@ test('commits a searched Cube dimension in no more than three interactions', asy { useDndKit: true, store: semanticViewStore() }, ); - userEvent.click(screen.getByText(/Drop columns here or click/i)); + await userEvent.click(screen.getByText(/Drop columns here or click/i)); const combobox = await screen.findByRole('combobox', { name: 'Dimensions', @@ -756,12 +756,12 @@ test('commits a searched Cube dimension in no more than three interactions', asy const option = await screen.findByRole('option', { name: /Product Category/i, }); - userEvent.click(option); + await userEvent.click(option); // Interaction 3: save. const saveButton = await screen.findByTestId('ColumnEdit#save'); await waitFor(() => expect(saveButton).toBeEnabled()); - userEvent.click(saveButton); + await userEvent.click(saveButton); await waitFor(() => { expect(mockOnChange).toHaveBeenCalledWith(['category']); @@ -794,7 +794,7 @@ test('anchors the "add column" popover to a block-level trigger box (sc-120502)' { useDndKit: true, store }, ); - userEvent.click(screen.getByText(/Drop columns here or click/i)); + await userEvent.click(screen.getByText(/Drop columns here or click/i)); await waitFor(() => { expect(screen.getByRole('tab', { name: 'Simple' })).toBeInTheDocument(); diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx index 72899a9b7f9d..0a14eb9c33ef 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx @@ -1370,7 +1370,7 @@ test('Filter subject lists and commits an expression-less Cube dimension', async const subjectSelect = screen.getByRole('combobox', { name: 'Select subject', }); - userEvent.click(subjectSelect); + await userEvent.click(subjectSelect); const dropdown = await waitFor(() => { const list = document.querySelector( @@ -1382,7 +1382,7 @@ test('Filter subject lists and commits an expression-less Cube dimension', async expect(within(dropdown).getByText('Order Date')).toBeInTheDocument(); expect(within(dropdown).getByText('Product Category')).toBeInTheDocument(); - userEvent.click(within(dropdown).getByText('Product Category')); + await userEvent.click(within(dropdown).getByText('Product Category')); await waitFor(() => { expect(props.onChange).toHaveBeenCalledWith( diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/AdhocMetricEditPopover.test.tsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/AdhocMetricEditPopover.test.tsx index 0e4f93a45122..56a6073b95c9 100644 --- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/AdhocMetricEditPopover.test.tsx +++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover/AdhocMetricEditPopover.test.tsx @@ -474,7 +474,7 @@ test('disables saved metrics absent from a verified compatibility result', async }, }); - userEvent.click( + await userEvent.click( screen.getByRole('combobox', { name: 'Select saved metrics' }), ); @@ -500,7 +500,7 @@ test('keeps every saved metric enabled after a failed compatibility request', as }, }); - userEvent.click( + await userEvent.click( screen.getByRole('combobox', { name: 'Select saved metrics' }), );