Skip to content

[test] Sync regression test layout with Base UI / Material UI #17564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

[test] Sync regression test layout with Base UI / Material UI #17564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bd67283
[test] Sync regression test layout with Base UI / Material UI
oliviertassinari Apr 26, 2025
e326205
fix
oliviertassinari Apr 27, 2025
2da00b5
avoid distractions
oliviertassinari Apr 28, 2025
e12b23c
simplify
oliviertassinari Apr 28, 2025
60a3728
ready
oliviertassinari Apr 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions package.json
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

In #17214 I tried to move deps & config towards where they're used. Calling the commands such as mocha from the root means we need all the deps for all the test setups (unit, regressions, e2e) mashed into the root package.json.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, a variant of this PR: #17583, but I don't really see the value, having the same depends between the different test env seems OK.

Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,11 @@
"test:e2e": "pnpm run release:build && cd test/e2e && pnpm run start",
"test:e2e-website": "npx playwright test test/e2e-website --config test/e2e-website/playwright.config.ts",
"test:e2e-website:dev": "PLAYWRIGHT_TEST_BASE_URL=http://localhost:3001 npx playwright test test/e2e-website --config test/e2e-website/playwright.config.ts",
"test:regressions": "pnpm run release:build && cd test/regressions && pnpm run start",
"test:regressions:dev": "cd test/regressions && pnpm run start",
Copy link
Member Author

@oliviertassinari oliviertassinari Apr 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

test:regressions:dev doesn't work

"test:regressions": "cross-env NODE_ENV=production pnpm test:regressions:build && concurrently --success first --kill-others \"pnpm test:regressions:run\" \"pnpm test:regressions:server\"",
"test:regressions:build": "cross-env TEST_BUILD=true vite build --config test/regressions/vite.config.js",
"test:regressions:dev": "pnpm test:regressions:build && pnpm test:regressions:server",
Copy link
Member Author

@oliviertassinari oliviertassinari Apr 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I would want to use this here:

Suggested change
"test:regressions:dev": "pnpm test:regressions:build && pnpm test:regressions:server",
"test:regressions:dev": "cross-env TEST_BUILD=true vite --config test/regressions/vite.config.js --port 5001",

but Vite seems unable to build properly in dev mode compared to webpack, so falling back to this 🙃. Sad, I don't understand.

"test:regressions:run": "mocha --config test/regressions/.mocharc.cjs 'test/regressions/**/*.test.{js,ts,tsx}'",
"test:regressions:server": "serve test/regressions -p 5001",
"test:argos": "node ./scripts/pushArgos.mjs test/regressions/screenshots/chrome",
"typescript": "lerna run --no-bail --parallel typescript",
"typescript:ci": "lerna run --concurrency 1 --no-bail --no-sort typescript",
Expand Down
39 changes: 0 additions & 39 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/regressions/.mocharc.js → test/regressions/.mocharc.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export default {
module.exports = {
extension: ['js', 'ts', 'tsx'],
recursive: true,
slow: 500,
Expand Down
21 changes: 1 addition & 20 deletions test/regressions/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,9 @@
"version": "0.0.0",
"private": true,
"type": "module",
"scripts": {
Copy link
Member Author

Choose a reason for hiding this comment

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

It seems simpler to have all the actions at the root. I don't actually mind actually, but we might as well copy the references (Base UI, Material UI).

"start": "cross-env NODE_ENV=production pnpm build && concurrently --success first --kill-others \"pnpm run mocha\" \"pnpm run server\"",
"build": "cross-env TEST_BUILD=true vite build",
"dev": "concurrently \"pnpm run build --watch\" \"pnpm run server\"",
"mocha": "mocha --config .mocharc.js '**/*.test.{js,ts,tsx}'",
"server": "serve . -p 5001"
},
"scripts": {},
"devDependencies": {
"@babel/runtime": "^7.27.0",
"@mui/x-charts": "workspace:*",
"@mui/x-charts-pro": "workspace:*",
"@mui/x-charts-vendor": "workspace:*",
"@mui/x-data-grid": "workspace:*",
"@mui/x-data-grid-generator": "workspace:^",
"@mui/x-data-grid-premium": "workspace:^",
"@mui/x-data-grid-pro": "workspace:*",
"@mui/x-date-pickers": "workspace:*",
"@mui/x-date-pickers-pro": "workspace:*",
"@mui/x-internals": "workspace:^",
"@mui/x-license": "workspace:*",
"@mui/x-tree-view": "workspace:*",
"@mui/x-tree-view-pro": "workspace:*",
"@playwright/test": "^1.52.0",
"@types/chai": "^4.3.20",
"@types/karma": "^6.3.9",
Expand Down
41 changes: 40 additions & 1 deletion test/regressions/vite.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import { defineConfig, transformWithEsbuild } from 'vite';
import react from '@vitejs/plugin-react';

export default defineConfig({
root: path.join(process.cwd(), 'test/regressions'),
optimizeDeps: {
include: ['prismjs', 'clipboard-copy'],
},
build: {
outDir: 'build',
},
Expand All @@ -15,6 +19,39 @@ export default defineConfig({
import.meta.dirname,
'../../node_modules/@mui/monorepo/packages/mui-docs/src',
),
'@mui/x-data-grid': path.resolve(import.meta.dirname, '../../packages/x-data-grid/src'),
'@mui/x-data-grid-generator': path.resolve(
import.meta.dirname,
'../../packages/x-data-grid-generator/src',
),
'@mui/x-data-grid-pro': path.resolve(
import.meta.dirname,
'../../packages/x-data-grid-pro/src',
),
'@mui/x-data-grid-premium': path.resolve(
import.meta.dirname,
'../../packages/x-data-grid-premium/src',
),
'@mui/x-date-pickers': path.resolve(import.meta.dirname, '../../packages/x-date-pickers/src'),
'@mui/x-date-pickers-pro': path.resolve(
import.meta.dirname,
'../../packages/x-date-pickers-pro/src',
),
'@mui/x-charts': path.resolve(import.meta.dirname, '../../packages/x-charts/src'),
'@mui/x-charts-pro': path.resolve(import.meta.dirname, '../../packages/x-charts-pro/src'),
'@mui/x-tree-view': path.resolve(import.meta.dirname, '../../packages/x-tree-view/src'),
'@mui/x-tree-view-pro': path.resolve(
import.meta.dirname,
'../../packages/x-tree-view-pro/src',
),
'@mui/x-license': path.resolve(import.meta.dirname, '../../packages/x-license/src'),
'@mui/x-telemetry': path.resolve(import.meta.dirname, '../../packages/x-telemetry/src'),
'@mui/x-internals': path.resolve(import.meta.dirname, '../../packages/x-internals/src'),
'@mui/material-nextjs': path.resolve(
import.meta.dirname,
'../../node_modules/@mui/monorepo/packages/mui-material-nextjs/src',
),
docs: path.resolve(import.meta.dirname, '../../node_modules/@mui/monorepo/docs'),
Comment on lines +22 to +54
Copy link
Contributor

Choose a reason for hiding this comment

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

I have tried to move away from alias lists. If we do want them back, we should probably find a way to avoid having multiple copies of it.

Copy link
Member Author

@oliviertassinari oliviertassinari Apr 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Without this, we have to build the whole project and have it in watch mode to debug/run the visual regression tests, which feels super slow (+200s on my end). Too slow to be usable in practice. #17584

https://github.com/mui/base-ui/blob/a7451a08424f12d913c8355cccf3274ef5f3e055/package.json#L38 or https://github.com/mui/material-ui/blob/a705e1f15075b2deb59263868bfa7b1d9f84cdd4/package.json#L75 seems better.

we should probably find a way to avoid having multiple copies of it.

Oh yeah, once we remove babel alias. I'm not worried about this, I think it's about copying the Base UI repo or Material UI repo. They should get this right. We shouldn't have to think about this in the MUI X repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yeah, once we remove babel alias. I'm not worried about this, I think it's about copying the Base UI repo or Material UI repo. They should get this right. We shouldn't have to think about this in the MUI X repo.

Sorry I don't understand what you mean there. Could we create an aliases.js file at the root to avoid repeating the alias list?

docsx: path.resolve(import.meta.dirname, '../../docs'),
},
},
Expand All @@ -27,7 +64,9 @@ export default defineConfig({
name: 'replace-code',
enforce: 'post',
async transform(code) {
return code.replaceAll('DISABLE_CHANCE_RANDOM', 'true');
return code
.replaceAll('DISABLE_CHANCE_RANDOM', 'true')
.replaceAll('LICENSE_DISABLE_CHECK', 'true');
},
},
{
Expand Down
Loading