You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2025/10/17 17:55:38 UTC

(superset) branch fix-flakey-filters-config-modal-test created (now cda7458655)

This is an automated email from the ASF dual-hosted git repository.

jli pushed a change to branch fix-flakey-filters-config-modal-test
in repository https://gitbox.apache.org/repos/asf/superset.git


      at cda7458655 fix(dashboard): eliminate race condition in FiltersConfigModal test

This branch includes the following new commits:

     new cda7458655 fix(dashboard): eliminate race condition in FiltersConfigModal test

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(superset) 01/01: fix(dashboard): eliminate race condition in FiltersConfigModal test

Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jli pushed a commit to branch fix-flakey-filters-config-modal-test
in repository https://gitbox.apache.org/repos/asf/superset.git

commit cda74586551392bbf9788052a2735ead87dfb55e
Author: Joe Li <jo...@preset.io>
AuthorDate: Fri Oct 17 10:54:55 2025 -0700

    fix(dashboard): eliminate race condition in FiltersConfigModal test
    
    The "validates the pre-filter value" test had a race condition that caused
    intermittent failures and required a 50-second timeout.
    
    **Root Cause:**
    - Test used synchronous `userEvent.click()` calls
    - Immediately called `jest.runOnlyPendingTimers()` (wrong method)
    - User interactions didn't complete before timer flush
    - Created race where validation timers might not execute
    
    **Fix:**
    - Await `userEvent.click()` calls (v12-compatible async approach)
    - Use `jest.runAllTimers()` instead of `runOnlyPendingTimers()`
    - Wrap timer manipulation in try/finally for cleanup
    - Add explicit `waitFor()` after restoring timers
    
    **Results:**
    - Test now passes consistently (3/3 runs)
    - Runs in ~25-29s vs 50s timeout
    - No regressions (16 passed, 2 skipped)
    
    🤖 Generated with [Claude Code](https://claude.com/claude-code)
    
    Co-Authored-By: Claude <no...@anthropic.com>
---
 .../FiltersConfigModal/FiltersConfigModal.test.tsx | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx
index 01c50755ff..a419cd7936 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx
@@ -337,19 +337,21 @@ test.skip('validates the default value', async () => {
 
 test('validates the pre-filter value', async () => {
   jest.useFakeTimers();
+  try {
+    defaultRender();
 
-  defaultRender();
+    await userEvent.click(screen.getByText(FILTER_SETTINGS_REGEX));
+    await userEvent.click(getCheckbox(PRE_FILTER_REGEX));
 
-  userEvent.click(screen.getByText(FILTER_SETTINGS_REGEX));
-  userEvent.click(getCheckbox(PRE_FILTER_REGEX));
-
-  jest.runOnlyPendingTimers();
-  jest.useRealTimers();
+    jest.runAllTimers();
+  } finally {
+    jest.useRealTimers();
+  }
 
-  expect(
-    await screen.findByText(PRE_FILTER_REQUIRED_REGEX),
-  ).toBeInTheDocument();
-}, 50000); // Slow-running test, increase timeout to 50 seconds.
+  await waitFor(() => {
+    expect(screen.getByText(PRE_FILTER_REQUIRED_REGEX)).toBeInTheDocument();
+  });
+});
 
 // eslint-disable-next-line jest/no-disabled-tests
 test.skip("doesn't render time range pre-filter if there are no temporal columns in datasource", async () => {