Repository navigation
test(desktop): one DOM for the whole run and no module mocks - #8319
Merged
Merged
Conversation
Two things made desktop tests depend on file order without --isolate. mock.module is process-wide in bun. Fakes of renderer/lib/cloud-trpc, host-service-client, @superset/workspace-client, ../env and others reached later files. Tests now use real providers with fake links, or the code takes its dependency as an optional parameter. 58 files registered happy-dom and 52 unregistered it. After the first unregister, that window's timers never fire again, and react-dom captured them at import, so React updates outside act() never rendered in later files. happy-dom is now registered once in the preload. A test also left the ja locale active, and six files called a bare mock.restore() that tore down the preload's electron mock.
Contributor
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (135)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
saddlepaddle
marked this pull request as ready for review
October 8, 2026 05:57
Contributor
 |
Contributor
Comments Outside DiffThese findings could not be posted inline.
|
saddlepaddle
added a commit
that referenced
this pull request
Oct 8, 2026
* test: run suites without --isolate and remove three flaky tests The cross-file leaks that --isolate hid are fixed (#8314, #8315, #8316, #8319), so each package's suite now runs in one process. The desktop suite takes about 38 s without the flag and about 73 s with it. apps/relay still had one leak: host-tunnel.test.ts replaced partyserver with a stub that has no getServerByName, which broke index.test.ts in some file orders. The test passes against the real module, so the stub is gone. Removed as flaky: - workspace-fs: FsWatcherManager resumes a subscription after the root is deleted and recreated - i18n: locale changes preserve state and focus (provider-state) - agent-setup: Pi completion notifications, which poll up to 1 s for a detached script * test(trpc): keep the opt-in Postgres watch test out of the shared suite It sets DATABASE_URL and then imports the db client. In one process the client may already exist, so the test would write its fixtures to the database the first file connected to. As an .integration.ts file it runs only when named, in its own process, like the other opt-in suites.
pgilmore
added a commit
to patiently/superset
that referenced
this pull request
Oct 11, 2026
Merges upstream main into the watcher build. Upstream now runs each test suite in one process and bans module mocks in the desktop suite (ParadevAI#8319, ParadevAI#8336), so the hover-card test drives the real hooks against a fake host instead of mocking them. Tests run on the bun canary upstream's CI uses, because bun 1.4.2 can hang in a synchronous spawn. Claude-Session: https://claude.ai/code/session_01CZBCrRCZ2rEBHv4DH3Wh3k
MagnusPladsen
added a commit
to MagnusPladsen/superset
that referenced
this pull request
Oct 13, 2026
Resolve the V2ProjectSettings conflict from ParadevAI#8319 by moving the Color row into the new V2ProjectSettingsBody, wired to onHostProjectChanged. Regenerate the i18n catalogs so entry order stays deterministic. Co-Authored-By: AI Assistant <noreply@example.com>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of removing cross-file test leaks so suites do not depend on
bun test --isolate. Draft: the tests pass in any order, but five production components were restructured and none was checked in the running app yet (list at the end).Why tests depended on file order
mock.moduleis process-wide in bun. Fakes ofrenderer/lib/cloud-trpc,host-service-client,@superset/workspace-client,../env,@superset/local-db,reactand others reached every file that loaded later.queueMicrotaskandsetTimeoutnever fire again. react-dom captures both at import, so in every later DOM file a React update outsideact()never rendered. This was the cause of theuseDiffCodeViewItemsfailures.LanguageAwareI18nProvider.test.tsxleft thejalocale active (8月22日in date tests); six files called a baremock.restore(), which also tears down the preload'selectronmock; two tests replacedglobalThis.localStorage.What changed
test-setup.tsregisters happy-dom once, in place of the hand-writtendocument/windowstubs, and never unregisters. It then puts back the nativefetch,Request,Response,Headers,URL,WebSocket, abort classes, streams, timers andqueueMicrotask, so non-DOM tests keep the runtime they had. Per-file register/unregister is removed from about 55 files. Two nested-process fixtures got anisRegisteredguard.mock.moduleand no baremock.restore()remain inapps/desktop/srcoutside thefixtures/files that run in a nested process. Renderer tests use the real tRPC providers with a fake link, or a localBun.serve.DaemonTerminalManager,restartDaemon,tryListExistingDaemonSessions, the host-service coordinator and lock,ai-name,WorkerTaskRunner,killPortTarget,terminal-ws-transportandbrowserRuntimeRegistrytake an optional dependency that defaults to the previous import.openNewWorkspace,openNewSession,openNewWorkspaceForLocalProject,importFolderFirst,invalidateGitQueries,resolveStatusRefetchInterval,useIsV1FlipLockedFor,useIsV2CloudEnabledFor.PostHogLocaleTaggermoved from insideLanguageAwareI18nProviderto its first child inroutes/-layout.tsx, the only place that renders the provider.useDiffCodeViewItemsreads the client fromworkspaceTrpc.useUtils();WorkspaceClientProviderpasses the same client to both.Components split into a container and a props-driven part
PagesViewPagesListsearchDebounceMs.PageWatcherMenuPageWatcherPopoverAgentSessionPickerAgentSessionSelectopen/onOpenChangeprops carrying the same state.ExperimentalSettingsExperimentalSettingsListV2ProjectSettingsV2ProjectSettingsBodyTests
V2ProjectSettings.test.tsx; its three tests asserted that mocked children appeared. Replaced byV2ProjectSettingsBody.test.tsx, which renders the real children and guards the Project icons are missing in 1.16 #5843 regression (icon picker present and enabled).PagesView.test→PagesList.test(19),PageWatcherMenu.test→PageWatcherPopover.test(26),useFolderFirstImport.test→importFolderFirst.test,useGitStatus.test→invalidateGitQueries.testplus a newuseGitStatus.testagainst a local WebSocket host.ExperimentalSettings: four tests became three.V2ProjectSettingsstops rendering the body or drops one of its three passed-in sections.useOpenNewWorkspaceForLocalProjectis tested as a function, not through the hook, because a mountedauthClient.useSession()makes a real request.Verification
bun 1.4.2,
bun test --randomize --seed N: seeds 1-14 pass without--isolate(about 38 s) and seeds 1-13 pass with it (about 73 s). One--isolaterun of seed 1 had four git and shell tests time out while the machine was overloaded, and passed on rerun. Repo typecheck, biome andcheck:i18npass.Before this leaves draft
Check in the running app: the Pages list (search, favorites, create with agent), the page watcher menu, the agent session picker, Experimental settings, V2 project settings, folder-first repository import, and that the PostHog locale tag is still set.
Summary by cubic
Makes the desktop test suite pass in any order without
bun test --isolateby removing the cross-file leaks behind the order dependence: process-widemock.modulecalls, per-file happy-dom registration, and leftover global state. Still a draft: five production components were split and need checking in the running app (Pages list, page watcher menu, agent session picker, Experimental/V2 project settings, folder-first import, PostHog locale tag). Verified withbun test --randomizeseeds 1–14 without--isolateand 1–13 with it.Why tests depended on order
mock.moduleis process-wide in bun, so mocks from one file reached every later file's imports.queueMicrotaskandsetTimeout, so React updates outsideact()never rendered in later DOM files.jalocale stayed active, six files called a baremock.restore()that tore down the preload'selectronmock, and two tests replacedglobalThis.localStorage.What changed
test-setup.tsregisters happy-dom once per process, never unregisters it, and restores native fetch, WebSocket, streams, timers, andqueueMicrotask; per-file register/unregister removed from ~55 files.mock.moduleor baremock.restore()remain outsidefixtures/; renderer tests use real tRPC providers with a fake link or a localBun.serve.PagesList,PageWatcherPopover,AgentSessionSelect,ExperimentalSettingsList,V2ProjectSettingsBody);V2ProjectSettings.test.tsxreplaced byV2ProjectSettingsBody.test.tsx.Written for commit 1dfd5d5. Summary will update on new commits.