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] Set the URL to better match real environments #34449

Merged

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Sep 23, 2022

This logic was added in #31273. I'm testing that we no longer need it. I suspect that it was added to solve #33853. Removing it makes the test environment better capture the complexity of the production environments, it can also be seen as a test for #34027.

@mui-bot
Copy link

mui-bot commented Sep 23, 2022

No bundle size changes

Generated by 🚫 dangerJS against efe3aee

@oliviertassinari oliviertassinari marked this pull request as ready for review September 23, 2022 20:50
@siriwatknp
Copy link
Member

I added it because of the error I got in jsdom/jsdom#2383 when I was writing tests for CssVarsProvider #29418.

Happy to remove it if it does not produce the error anymore.

Signed-off-by: Olivier Tassinari <olivier.tassinari@gmail.com>
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Sep 26, 2022

@siriwatknp Thanks for the context, so I assume that #34027 solved the issue hence the comment that points to jsdom/jsdom#2383 is misleading, it's a case that we need to support 😁.

Now, in hindsight, it's because you have added the URL that we could surface a bug in the tests of MUI X, which lead to mui/mui-x#6273. I think that it does make more sense to have the URL equal http://localhost than about:blank. I think that we are better off finding bugs like mui/mui-x#6273 than #34027 (more likely to be eventually found by the community than the first one).

@oliviertassinari oliviertassinari changed the title [test] Remove localStorage from the test env [test] Set the URL to better match real environments Sep 26, 2022
@oliviertassinari oliviertassinari merged commit e3b4512 into mui:master Sep 26, 2022
@oliviertassinari oliviertassinari deleted the harden-the-test-environment branch September 26, 2022 12:19
alexfauquette pushed a commit to alexfauquette/material-ui that referenced this pull request Oct 14, 2022
Signed-off-by: Olivier Tassinari <olivier.tassinari@gmail.com>
daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
Signed-off-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants