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

[system] Need for consistent CSS classes in snapshot tests #40448

Closed
1 task done
mennorenkens opened this issue Jan 6, 2024 · 5 comments
Closed
1 task done

[system] Need for consistent CSS classes in snapshot tests #40448

mennorenkens opened this issue Jan 6, 2024 · 5 comments
Assignees
Labels
package: system Specific to @mui/system

Comments

@mennorenkens
Copy link

mennorenkens commented Jan 6, 2024

Duplicates

  • I have searched the existing issues

Summary

We have some snapshot tests in our test-suite. They simply compare HTML states between test runs, and fail if anything changed.
These do not work well with MUI unfortunately. When we generate the initial snapshots locally (or update them), the generated CSS class names in the HTML files are different from the ones that our testing-server generates and compares to.

This is due to the fact that our local environments have a different NODE_ENV than our CI server.
We found that the logic is in node_modules/@mui/system/modern/createStyled.js:

if (process.env.NODE_ENV !== 'production') {
    if (componentName) {
        label = `${componentName}-${lowercaseFirstLetter(componentSlot || 'Root')}`;
    }
}

Is it possible to determine this based on a runtime variable instead of a compile time one, like NODE_ENV? If not, do you have other suggestions?

Examples

No response

Motivation

No response

Search keywords:

@mennorenkens mennorenkens added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 6, 2024
@oliviertassinari oliviertassinari added the package: system Specific to @mui/system label Jan 7, 2024
@danilo-leal danilo-leal changed the title Need for consistent CSS classes in snapshot tests [system] Need for consistent CSS classes in snapshot tests Jan 9, 2024
@brijeshb42 brijeshb42 added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 9, 2024
@brijeshb42
Copy link
Contributor

Seems like an issue with the difference in your environments? Could you have the same env vars in your local tests that are on your CI ?

@mennorenkens
Copy link
Author

mennorenkens commented Jan 9, 2024

Yes, there indeed is a difference for the NODE_ENV between our environments! It's development locally (better debuggability) and production in on the server running our tests (to match the actual production environment 1-on-1).
It is just inconvenient / time-consuming if we locally have to re-compile (in production mode) every time we want to run a snapshot test. It would be easier if there is a runtime variable on which class name generation is based, so we our snapshot tests locally can also generate the same css-classnames as in the production environment. That way we can re-use our already existing local build, which is much faster.
Is that clear? Or still not?

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Jan 9, 2024
@brijeshb42
Copy link
Contributor

I don't see how a runtime variable can be introduced here unless you actually call createStyled in your codebase instead of styled from @mui/system or @mui/material, because they use a single instance of styled.
The best that can be done is add one more env var check, ie, process.env.NODE_ENV !== 'test', since test and production are 2 standard values used throughout the codebase. Then you an use test for your local tests.

@mennorenkens
Copy link
Author

We prefer to keep using 'production' as node_env. Thanks for the help anyways! 🙏

@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Apr 28, 2024

@mennorenkens @brijeshb42 So, what's remaining to do here?

@ZeeshanTamboli ZeeshanTamboli removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: system Specific to @mui/system
Projects
None yet
Development

No branches or pull requests

4 participants