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

[core] Use glob to find the test files in parseTest #36305

Merged
merged 3 commits into from
Apr 14, 2023

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Feb 23, 2023

Effect on mui-x: mui/mui-x#8483

Revert #34789 (see sindresorhus/globby#193)

On X, most of our tests are in files named {component}/tests/{feature}.{component}.test.tsx
For example src/TimeField/tests/editing.TimeField.test.tsx.

This allow us to have several test files per component, especially for big ones with often several thousands of lines of test.

In it's current form, parseTest does not support those test files, so our doc does not have the correct forwardsRefTo, inheritComponent and spread properties.

To fix that, I introduced a glob matching in parseTest.
But globby is pure ESM and our api builder process is not.
So I downgraded globby to the last version working correctly with our current infra.

If you have another solution, I'm interested because I am fully aware that downgrading a library like that is not great.
But on the other hand, migrating a thousand long set of scripts across 2 repo just for this use case will take a lot of time :/

@flaviendelangle flaviendelangle self-assigned this Feb 23, 2023
@flaviendelangle flaviendelangle added the core Infrastructure work going on behind the scenes label Feb 23, 2023
@mui-bot
Copy link

mui-bot commented Feb 23, 2023

Netlify deploy preview

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

Bundle size report

No bundle size changes

Generated by 🚫 dangerJS against 3e940e4

@flaviendelangle flaviendelangle added the scope: docs-infra Specific to the docs-infra product label Feb 23, 2023
@michaldudak
Copy link
Member

Is using fast-glob instead feasible in your case?

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 27, 2023
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 3, 2023
@flaviendelangle
Copy link
Member Author

@michaldudak seems to work fine indeed, thanks !

@flaviendelangle
Copy link
Member Author

@michaldudak if you can please have a look at the PR 👼

Co-authored-by: Michał Dudak <michal.dudak@gmail.com>
Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>
Copy link
Member

@michaldudak michaldudak left a comment

Choose a reason for hiding this comment

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

Works like a charm now.

@flaviendelangle flaviendelangle merged commit 66a69ec into mui:master Apr 14, 2023
@flaviendelangle flaviendelangle deleted the test-files-parsing branch April 14, 2023 13:37
@oliviertassinari oliviertassinari added scope: code-infra Specific to the core-infra product scope: docs-infra Specific to the docs-infra product and removed scope: docs-infra Specific to the docs-infra product scope: code-infra Specific to the core-infra product labels Apr 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes scope: docs-infra Specific to the docs-infra product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants