test(frontend): await remaining userEvent calls and lint for un-awaited ones

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,
 };

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();
 

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(

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,17 +268,19 @@ 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();
 
   // Deselect (multi-select toggle) — the cleared target must keep the
   // 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.