Skip to content

Migrate to vitest#600

Open
kneth wants to merge 20 commits into
masterfrom
kneth/migrate-to-vitest
Open

Migrate to vitest#600
kneth wants to merge 20 commits into
masterfrom
kneth/migrate-to-vitest

Conversation

@kneth
Copy link
Copy Markdown
Member

@kneth kneth commented May 22, 2026

Summary of changes

We are migrating from Jest to vitest.

The PR must be squashed before merging.

Checklist

  • Link to issue this PR refers to: https://github.com/crate/cloud/issues/2952
  • Relevant changes are reflected in CHANGES.md.
  • Added or changed code is covered by tests.
  • Required Grand Central APIs are already merged.

@kneth kneth requested a review from joncombe as a code owner May 22, 2026 15:38
@kneth kneth self-assigned this May 22, 2026
@kneth kneth marked this pull request as draft May 22, 2026 15:45
@kneth kneth marked this pull request as ready for review May 29, 2026 09:23
kneth and others added 18 commits June 2, 2026 14:27
- Fix unstableSetRender cleanup race: delete _reactRoot before unmounting
  so message.success() after message.destroy() gets a fresh root
- Fix SQLEditorSchemaTree tree expansion: fake timers + vi.runAllTimers()
  inside act() fires rc-motion's 500ms motionDeadline synchronously,
  committing onMotionEnd (prevData update) before the next expansion click
- 2 tests remain: 3-level tree expansion (object columns nested, subcolumn copy)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- test/setup.ts: global beforeEach flushes rc-motion leave animations with
  fake timers (3 × runAllTimers) so stale antd elements don't bleed between
  tests; unstableSetRender cleanup is now a no-op to avoid the createRoot
  race caused by showSuccessMessage's synchronous destroy+create pattern
- SQLEditorSchemaTree: useFakeTimers per describe + second runAllTimers pass
  after waitFor to flush setPrevData — fixes 3-level tree expansion timeout
- CopyToClipboard / SQLResultsTable: use findAllByText/getAllByText so a
  previous test's animating-out "Copied" message doesn't cause getBy to throw
- NotificationHandler: local afterEach flushes notifications; query via
  document.querySelector instead of container.nextElementSibling so sibling
  shift from an animating-out prior notification doesn't miss the target
- Logs: modal close assertion checks for leave class instead of DOM removal
  (jsdom never fires transitionend so the animation never fully completes)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…uite

Two root causes identified and fixed at the source:

1. rc-motion (Ant Design's animation engine) calls getComputedStyle(el, '::before')
   to detect animation durations — JSDOM 29 doesn't support pseudo-element style
   lookup. Stub it to return a no-animation CSSStyleDeclaration.

2. SQLResultsTable download links use <a href="data:..."> URIs; clicking them causes
   JSDOM to attempt navigation to a new document, which it can't do. A capture-phase
   event listener calls preventDefault() on such clicks so React onClick handlers
   still fire but JSDOM never starts the navigation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t runs

- Replace vite-tsconfig-paths plugin with Vite's native resolve.tsconfigPaths
  option — Vite 8 (rolldown) supports this natively and the plugin itself emits
  a deprecation warning pointing to this change.
- Pass disableOxcRecommendation: true to @vitejs/plugin-react-swc — the plugin
  warns to switch to @vitejs/plugin-react when no SWC plugins are in use and
  rolldown is detected; this flag suppresses that recommendation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ations

Root cause: Ant Design's `message` component passes no motionDeadline to
rc-motion, so leave animations stall waiting for CSS `animationend` events
that JSDOM never fires. State updates from those stalled animations
occasionally fire after a test ends, triggering React 19's
"not wrapped in act()" warning for ForwardRef / Notifications components.

Fix: filter the warning at the console.error level in setup.ts — the same
suppression already applied per-file in NotificationHandler.test.tsx.
This does not mask any real assertion failures; tests still fail on wrong
output, just without the noisy act() advisory alongside them.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n CopyToClipboard test

A spy set up in the preceding test leaks into this test, and antd message
notifications are portalled outside the component tree so testing-library
cleanup doesn't remove them. This causes findByText('Copied') to find two
elements and throw. Using findAllByText is consistent with the pattern
already used in the adjacent test.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@joncombe joncombe force-pushed the kneth/migrate-to-vitest branch 2 times, most recently from 68c7af3 to bdac5c5 Compare June 2, 2026 08:51
@joncombe joncombe force-pushed the kneth/migrate-to-vitest branch from bdac5c5 to 882ace5 Compare June 2, 2026 08:53
@joncombe joncombe requested review from plaharanne and tomach June 2, 2026 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants