mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
3.82k stars 1.14k forks source link

[code-infra] "pnpm test" works poorly #12935

Closed joshkel closed 2 weeks ago

joshkel commented 2 weeks ago

Steps to reproduce

Link to live example: N/A (development environments only)

Steps:

  1. Run pnpm test

Current behavior

pnpm test has two problems.

First, it runs extremely slowly: it invokes Lerna for each workspace that has a test target, and those test targets run Mocha with their workspace directories as spec arguments. However, command-line spec arguments in Mocha are added to, rather than overriding, the spec in .mocharc.js (see here), so it ends up running the full test suite one time per workspace.

Second, it doesn't set TZ=UTC, so many timezone-dependent tests fail.

Expected behavior

pnpm test is an obvious target for new developers, so it should work as expected (e.g., perhaps execute pnpm test:unit?) or indicate what developers should do instead.

Context

No response

Your environment

npx @mui/envinfo ``` System: OS: macOS 14.4.1 Binaries: Node: 18.18.2 - ~/.nvm/versions/node/v18.18.2/bin/node npm: 9.8.1 - ~/.nvm/versions/node/v18.18.2/bin/npm pnpm: 8.15.5 - /opt/homebrew/bin/pnpm Browsers: Chrome: 124.0.6367.91 Edge: 124.0.2478.67 Safari: 17.4.1 npmPackages: @emotion/react: ^11.11.4 => 11.11.4 @emotion/styled: ^11.11.0 => 11.11.0 @mui/base: 5.0.0-alpha.122 @mui/core-downloads-tracker: 5.11.14 @mui/icons-material: ^5.15.14 => 5.15.15 @mui/internal-markdown: ^1.0.3 => 1.0.3 @mui/joy: 5.0.0-alpha.72 @mui/material: ^5.15.14 => 5.15.15 @mui/monorepo: github:mui/material-ui#afffc2ffc3db1e6abf48f3b3e13a41e8b758f874 => 6.0.0-alpha.4 @mui/private-theming: 5.11.13 @mui/styled-engine: 5.11.11 @mui/styles: 5.11.13 @mui/system: 5.11.14 @mui/types: 7.2.3 @mui/utils: ^5.15.14 => 5.15.14 @mui/x-charts: 7.3.1 @mui/x-codemod: 7.0.0 @mui/x-date-pickers: 7.3.1 @mui/x-date-pickers-pro: 7.3.1 @types/react: ^18.2.60 => 18.2.60 react: ^18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 styled-components: 5.3.9 typescript: ^5.4.5 => 5.4.5 ```

Search keywords: pnpm test

michelengelen commented 2 weeks ago

@LukasTy could you have a look?

LukasTy commented 2 weeks ago

@joshkel @michelengelen the test command/script in this repo seemed kind of pointless. I've aligned the setup to be closer to the Core in the linked PR. Running pnpm t or pnpm test will now trigger similar/same behavior as test:unit. Hopefully, this will be more clear to contributors. 🙏

WDYT @joshkel, does the proposed change make sense to you? 🤔

joshkel commented 2 weeks ago

@LukasTy Yes, makes sense to me. Thanks!

github-actions[bot] commented 2 weeks ago

:warning: This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@joshkel: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.