Skip to content

POC: run Alert and Dialog tests in real-browser vitest mode#2638

Open
matyasf wants to merge 2 commits into
masterfrom
vitest_browser
Open

POC: run Alert and Dialog tests in real-browser vitest mode#2638
matyasf wants to merge 2 commits into
masterfrom
vitest_browser

Conversation

@matyasf

@matyasf matyasf commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Add real-browser Vitest (Playwright/chromium) test POC, that replaces both Cypress component tests AND Vitest unit tests.

Summary

  • Port Alert (ui-alerts) and Dialog (ui-dialog) suites to vitest/browser
  • add test:browser scripts and new vitest configs.

Test Plan

  • pnpm run test:browser runs the browser tests in the console
  • pnpm run test:browser-ui runs the browser tests in a browser
  • Check the code diff, it should do the same thing. Also check how you like the new syntax, any pros or cons
  • Read about it on https://vitest.dev/guide/browser/ or watch a short Youtube video

Future additions:

  • Vitest can automatically screenshot test failures, we could add it to the CI somehow
  • Vitest browser has a Visual regression test add-on, we could use it.

Fixes INSTUI-5098

🤖 Generated with Claude Code

@matyasf matyasf self-assigned this Jul 14, 2026
@matyasf matyasf changed the title test(ui-alerts,ui-dialog): run Alert and Dialog tests in real-browser vitest mode POC: run Alert and Dialog tests in real-browser vitest mode Jul 14, 2026
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://instructure.design/pr-preview/pr-2638/

Built to branch gh-pages at 2026-07-14 19:38 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

})
DialogExample.displayName = 'DialogExample'

const NestedDialogExample = (props: DialogProps) => {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This was not used by the test at all

@github-actions

Copy link
Copy Markdown

Visual regression report

No changes.

Status Count
Unchanged 32
Changed 0
New 0
Removed 0

📊 View full report

Baselines come from the visual-baselines branch. They refresh on every merge to master.

github-actions Bot pushed a commit that referenced this pull request Jul 14, 2026
Comment thread vitest.config.mts
enabled: true,
provider: playwright(),
headless: true,
instances: [{ browser: 'chromium' }],

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

we can add here other browsers easily to run our test suite on e.g. Firefox (but CI support might be tricky)

@joyenjoyer joyenjoyer Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good idea; I would add Safari if it is possible, it has many tricky cases when it comes to a11y

… vitest mode

Add a Vitest browser-mode project (Playwright/chromium) as a Cypress
component-test replacement POC and port the Alert and Dialog suites to it.

- Convert Alert.test.tsx and Dialog.test.tsx from jsdom/RTL to
  vitest-browser-react + vitest/browser (page/userEvent), and fold the
  former cypress Alerts/Dialog specs in as `Component tests` blocks.
- Add a `browser` project to vitest.config.mts (source aliases, moment
  locale alias, browser setup) and exclude ui-alerts/ui-dialog from the
  jsdom `web` project to avoid double-running.
- Add `test:browser`/`test:browser-watch` scripts and the
  playwright/@vitest/browser/vitest-browser-react dev deps.
- Ignore `.vitest-attachments/`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Comment thread vitest.config.mts
/// <reference types="vitest" />

import { defineConfig } from 'vitest/config'
import { defineConfig, configDefaults } from 'vitest/config'

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I havent checked this config thoroughly, we should do it, if this is the way forward

…lled

The default `test:vitest` ran all projects including the new `browser`
project, which failed on CI because the Playwright chromium binary was
never downloaded.

- Scope `test:vitest`/`test:vitest-watch` to the `web` + `node` projects
  so the unit-test and release pipelines stay pure-jsdom.
- Add a `vitest-browser-tests` job to pr-validation that installs the
  Playwright browser and runs `test:browser`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- name: Install dependencies
run: pnpm install --frozen-lockfile
- name: Install Playwright browser
run: pnpm exec playwright install --with-deps chromium

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why isnt it installed from dev deps?

Comment thread vitest.config.mts
find: `${pkgName}/src`,
replacement: path.join(pkgPath, 'src')
})
// Fallback bare-name -> package dir (for packages without an exports map)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this an existing scenario or just AI being cautious?

Comment thread vitest.config.mts
@@ -49,6 +126,14 @@ export default defineConfig({
// Allows using APIs like Jest without importing them
globals: true,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not part of the changeset, but maybe we should consider changing this to false?

Comment thread vitest.setup.browser.ts

// @instructure/console only outputs logs if process.env.NODE_ENV !== 'production'
// these messages are sometimes used in test assertions
if (typeof globalThis.process === 'undefined') {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this file is a bit hard to parse. is it only needed because of the console package?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I dont know yet, it could be that other errors surface when we dont mock it

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.

3 participants