[code-infra] Improve test setup #18228
Conversation
LukasTy
added
test
type: enhancement
|
Deploy preview: https://deploy-preview-18228--material-ui-x.netlify.app/ Bundle size reportTotal Size Change: 0B(0.00%) - Total Gzip Change: 0B(0.00%) |
LukasTy
changed the title
| orchestratorScripts: [ | ||
| { | ||
| id: 'vitest-reload-on-error', | ||
| content: `window.addEventListener('vite:preloadError', (event) => { window.location.reload(); });`, |
There was a problem hiding this comment.
|
I suspect these issues are due to OOM/Process/crash of the server. I've created https://github.com/mui/mui-x/pull/18230/files to try to alleviate the problem by using a single browser server to run all tests, rather than the current approach which creates an instance for every package. In the errors you linked, you see this at the start Which makes it at least somewhat clear that the server died, and the failure to find the Which can be seen more towards the end, and it mentions it didn't find it on the server (localhost:xxxx) |
| test: { | ||
| name: getTestName(import.meta.url), | ||
| environment: 'browser', | ||
| setupFiles: [new URL('../../test/utils/setupPickers.js', import.meta.url).pathname], |
There was a problem hiding this comment.
I'm not 100% sure, but I'd expect this could be broken on windows. Perhaps you want fileURLToPath?
There was a problem hiding this comment.
We were already using this pattern new URL() in other places. 🤔
Maybe @noraleonte could try running pnpm test:unit:browser to check this assumption? 🙏
There was a problem hiding this comment.
I'd say you always want to use fileURLToPath, even if it works for @noraleonte. Maybe not on your systems, but .pathname is going to give incorrect results. e.g. when someone has their project in a folder with a space or something.
Different case, but I remember someone having their projects in a folder with a dot and this breaking a script that assumes only paths for files can contain a dot. Naturally most people won't have this dot, so the issue remains invisible for a long time and will be hard to debug once it appears. In this case it even affected publishing of stable releases. Conclusion: always use the built-in functions for manipulating urls and paths, even if it looks trivial and seems to work. This is something you can be religious about in my experience.
oliviertassinari
changed the title
4 participants
Attempt to address flaky failures like:
It looks like the setup refactor might have reduced the runtime of browser tests.
Not saying that these will fix flaky tests, but in any case, I don't see why they couldn't be merged as general improvements. 🤔