Skip to content

test: fix CI OOM crashes in DatasourceControl test and flaky FileHandleer test#38430

Merged
rusackas merged 6 commits intomasterfrom
fix-master-ci
Mar 5, 2026
Merged

test: fix CI OOM crashes in DatasourceControl test and flaky FileHandleer test#38430
rusackas merged 6 commits intomasterfrom
fix-master-ci

Conversation

@sadpandajoe
Copy link
Member

SUMMARY

Fixes consistent Jest worker OOM crashes in master CI (sharded-jest-tests shard 7, sometimes 5/6) and a flaky FileHandler test.

DatasourceControl OOM fix (fe5f0b0):

  • Mock DatasourceEditor (2,620 lines, 150+ imports) in DatasourceControl.test.tsx — the heavy editor was rendered 4+ times per shard without mocking, compounding memory until the Jest worker crashed
  • Scope all mocks properly (no mockImplementationOnce), await all userEvent calls, use named fetchMock routes with proper cleanup
  • Heap: 615 MB → 287 MB (54% reduction)

Regression guards (289aa1e):

  • afterEach assertion for unmatched fetchMock calls (catches leaked routes)
  • Pre-commit hook banning mockImplementationOnce in DatasourceControl tests
  • Scoped ESLint testing-library/await-async-events rule for this test file
  • New CI job: heap check (520 MB threshold), mockImplementationOnce guard, un-awaited userEvent guard

FileHandler flaky test fix (471205c):

  • setupLaunchQueue used setTimeout(fn, 0) without cleanup — pending timers leaked between tests
  • Store timer ID and clear in afterEach

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A — test-only changes

TESTING INSTRUCTIONS

# DatasourceControl: all 21 tests pass, heap under 520 MB
npm run test -- --testPathPatterns DatasourceControl.test --maxWorkers=1

# FileHandler: all 11 tests pass, no flakiness
npm run test -- --testPathPatterns FileHandler/index.test --maxWorkers=1

# Pre-commit guard
pre-commit run no-mock-implementation-once-datasource --all-files

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

🤖 Generated with Claude Code

sadpandajoe and others added 3 commits March 4, 2026 20:49
DatasourceControl.test.tsx consistently OOM-crashes Jest workers in CI
because the last 4 tests render the full DatasourceEditor (2,500+ lines,
150+ imports) without mocking. Each test mounts this massive tree,
compounding memory until crash.

Mock DatasourceEditor with a lightweight stub since these tests only
verify DatasourceControl's callback wiring through the modal save flow,
not editor internals (covered by DatasourceEditor.test.tsx). Also scope
the SupersetClient.get spy into its single consuming test with explicit
restore to eliminate cross-test mock leaks, fix missing await on
userEvent calls, and rename tests to reflect their actual scope.

Results: heap 615MB→287MB, heavy tests 5500ms→350ms each.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Prevent re-introducing the OOM crash fixed in fe5f0b0 by adding:
- fetchMock unmatched call assertion in afterEach (catches leaked routes)
- pre-commit hook banning mockImplementationOnce in DatasourceControl tests
- scoped ESLint await-async-events rule for this test file
- CI heap-check job with 520MB threshold, mockImpl guard, and await guard

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The setupLaunchQueue helper used setTimeout(fn, 0) to defer the
consumer callback but never cleaned up the timer. Under CI load,
pending timers from one test could fire during the next test's
execution, causing non-deterministic behavior.

Store the timer ID and clear it in afterEach to prevent leaks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the github_actions Pull requests that update GitHub Actions code label Mar 5, 2026
@sadpandajoe sadpandajoe requested a review from Copilot March 5, 2026 06:53
@github-actions github-actions bot removed the github_actions Pull requests that update GitHub Actions code label Mar 5, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces Superset frontend CI flakiness by lowering Jest memory pressure in DatasourceControl unit tests and cleaning up a timer leak in FileHandler tests, while adding CI guards to prevent regressions.

Changes:

  • Mock DatasourceEditor in DatasourceControl.test.tsx, simplify save-flow tests, and add stricter fetch-mock cleanup checks to prevent Jest worker OOMs.
  • Fix FileHandler test flakiness by tracking and clearing pending setTimeout work between tests.
  • Add targeted lint/pre-commit/CI guards (heap threshold, banned mockImplementationOnce, and un-awaited userEvent checks).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

Show a summary per file
File Description
superset-frontend/src/pages/FileHandler/index.test.tsx Clears leaked timers and resets window.launchQueue to prevent cross-test interference.
superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx Mocks heavy editor component, improves async event handling, and adds fetch-mock leak detection to stabilize CI memory usage.
superset-frontend/.eslintrc.js Adds a file-scoped rule requiring awaited userEvent calls for the targeted test file.
.pre-commit-config.yaml Adds a pre-commit guard intended to ban mockImplementationOnce in the targeted test file.
.github/workflows/superset-frontend.yml Adds a dedicated CI job to enforce heap usage and regression guards for the DatasourceControl test.

@netlify
Copy link

netlify bot commented Mar 5, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 471205c
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/69a928165cbfbe0008458373
😎 Deploy Preview https://deploy-preview-38430--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@sadpandajoe sadpandajoe changed the title test: fix CI OOM crashes and flaky frontend tests test: fix CI OOM crashes in DatasourceControl test and flaky FileHandleer test Mar 5, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

@netlify
Copy link

netlify bot commented Mar 5, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 133127c
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/69a929379d1c0800085140cb
😎 Deploy Preview https://deploy-preview-38430--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

sadpandajoe and others added 2 commits March 4, 2026 23:17
Track pending setTimeout IDs in a Set instead of a single variable
to avoid orphaning timers if setConsumer is called multiple times.
Replace raw DOM .click() with await userEvent.click() to respect
React async/act boundaries.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Revert the pre-commit hook, scoped ESLint rule, and CI heap check
job added in 289aa1e. These single-test guards don't scale; the
DatasourceEditor mock is the actual OOM fix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Ensure cleanup (clearHistory, removeRoutes, restoreAllMocks) runs
even when the unmatched-call guard throws, preventing cascading
failures in subsequent tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sadpandajoe sadpandajoe marked this pull request as ready for review March 5, 2026 16:57
@dosubot dosubot bot added the change:frontend Requires changing the frontend label Mar 5, 2026
@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 5, 2026

Code Review Agent Run #7629e9

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: fe5f0b0..0e7b9dd
    • superset-frontend/src/explore/components/controls/DatasourceControl/DatasourceControl.test.tsx
    • superset-frontend/src/pages/FileHandler/index.test.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@rusackas rusackas merged commit 5f0efd2 into master Mar 5, 2026
126 checks passed
@rusackas rusackas deleted the fix-master-ci branch March 5, 2026 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants