Skip to content
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.

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

[test] Restore the t command #40430

Merged
merged 3 commits into from
Jan 5, 2024
Merged

Conversation

michaldudak
Copy link
Member

This change restores the pre-pnpm behavior of the t script.
With yarn, it was possible to run the test CLI by executing yarn t <test-pattern>. However, pnpm treats t as an alias for test. I introduced a small script that's run whenever pnpm t or pnpm test is executed. It checks if it has any arguments and calls either the test CLI (previous yarn t) or the extended validation suite (previously yarn test)

@michaldudak michaldudak requested a review from a team January 4, 2024 11:41
@michaldudak michaldudak added the scope: code-infra Specific to the core-infra product label Jan 4, 2024
@mui-bot
Copy link

mui-bot commented Jan 4, 2024

Netlify deploy preview

https://deploy-preview-40430--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 386bb61

@@ -0,0 +1,15 @@
import { spawn } from 'node:child_process';

Copy link
Member

@Janpot Janpot Jan 4, 2024

Choose a reason for hiding this comment

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

Can you comment here and link to this PR (#40430)? We would likely never have set it up this way if we had started out with pnpm and so that makes the why documented for future generations.

Another thing we could do at some point is add some output saying "pnpm t <pattern> is deprecated, use pnpm tw <pattern> instead. This way of running the tests will be removed in a future version of the MUI core repository". This gives contributors enough time to adapt their muscle memory and us the option of removing the complexity of this script in the future.

Copy link
Member

@Janpot Janpot left a comment

Choose a reason for hiding this comment

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

Approving already, but best to add that comment I mention. The rest of my review (in italics) is a suggestion and fully at your own discretion.

@michaldudak michaldudak merged commit 906df2c into mui:master Jan 5, 2024
19 checks passed
@michaldudak michaldudak deleted the restore-t-command branch January 5, 2024 10:30
mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: code-infra Specific to the core-infra product test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants