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

[wip][poc] Add Pigment CSS screenshot test #43280

Merged
merged 33 commits into from
Sep 6, 2024

Conversation

mnajdova
Copy link
Member

@mnajdova mnajdova commented Aug 13, 2024

This PR sets up CI steps for testing screenshots test using Pigment CSS. It uses the pigment-css-vite-app that is already set up in the project for testing Material UI with Pigment CSS.

The fixtures are located in src/pages/fixtures directory. Any component added there will be automatically added as a screenshot (e.g. check the apps/pigment-css-vite-app/src/pages/fixtures/PigmentCSSDifferentChildren.js file).

The logic is very similar to how the other regressions tests are set up.

For demos screenshots, in the end I decided to use the components we have under the /material-ui routes. There were some issues with importing components from the docs, I kept getting the error ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL when using https://vitejs.dev/guide/features.html#glob-import

TODO:

  • Remove the dummy fixtures after the review, they are there just to show-case that we can also add additional files.

@mnajdova mnajdova added the test label Aug 13, 2024
@mui-bot
Copy link

mui-bot commented Aug 13, 2024

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 8702cf0

@aarongarciah aarongarciah changed the title [wip][poc] Add Pigment CSS scnreeshot test [wip][poc] Add Pigment CSS screenshot test Aug 13, 2024
@mnajdova mnajdova marked this pull request as ready for review August 15, 2024 12:23
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 19, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 4, 2024
@mnajdova mnajdova added the scope: code-infra Specific to the core-infra product label Sep 4, 2024
@mnajdova mnajdova requested review from brijeshb42 and a team and removed request for siriwatknp September 4, 2024 09:48
@brijeshb42
Copy link
Contributor

Changes look good to me. There are some small issues in the images (like first character of placeholder showing for Select) but we'll have to verify that locally first.

@mnajdova
Copy link
Member Author

mnajdova commented Sep 4, 2024

There are some small issues in the images (like first character of placeholder showing for Select) but we'll have to verify that locally first.

It's the same locally, it's likely some bug/issue we need to look into. At least the screenshot test caught it ;)

@brijeshb42
Copy link
Contributor

I dint see anything missing. LGTM

@DiegoAndai
Copy link
Member

@mnajdova may I ask you to add a fixture for the Select component? I don't see it in the added screenshots.

@mnajdova
Copy link
Member Author

mnajdova commented Sep 5, 2024

@mnajdova may I ask you to add a fixture for the Select component? I don't see it in the added screenshots.

Added the default demos and also one screenshot with an open select.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 5, 2024
@DiegoAndai
Copy link
Member

Added the default demos and also one screenshot with an open select.

Thanks!

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 6, 2024
@mnajdova mnajdova merged commit 46698de into mui:master Sep 6, 2024
20 checks passed
Michael-Hutchinson pushed a commit to Michael-Hutchinson/material-ui that referenced this pull request Sep 7, 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.

4 participants