[code-infra] Make Argos upload script reusable #45883
Conversation
Netlify deploy previewhttps://deploy-preview-45883--material-ui.netlify.app/ Bundle size report |
Janpot
changed the title
| @@ -79,7 +79,7 @@ | |||
| "test:regressions-pigment-css:run": "nx run nx_test_regressions_pigment_css_run", | |||
| "test:regressions-pigment-css:server": "pnpm --filter @app/pigment-css-vite-app run preview --port 5001", | |||
| "test:unit": "nx run nx_test_unit", | |||
| "test:argos": "node ./scripts/pushArgos.mjs", | |||
| "test:argos": "node ./scripts/pushArgos.mjs test/regressions/screenshots/chrome", | |||
There was a problem hiding this comment.
WDYT about making this the default argument?
I imagine that most if not all repos/projects will use the same path. 🤔
There was a problem hiding this comment.
I don't really mind adding a default, but test/regressions/screenshots/chrome, while currently being in use, feels a bit nonsensical to me.
There was a problem hiding this comment.
I suggested it only because the migration would be seamless. 🙈
But yeah, I agree, we could omit the chrome part, since we do not create screenshots on other browsers anyway... 🤷
There was a problem hiding this comment.
The original CLI that we wrap also uses a positional argument with no default. imo it makes most sense to not let this script make assumptions about the rest of the repo, it feels leaky.
There was a problem hiding this comment.
Sure, I have no objections to this approach as well. 👍
It will be cleaner long term. 👌
We just need to remember that this is a breaking change that will need adjustment when @mui/monorepo is bumped. ℹ️
Janpot
changed the title
Janpot
changed the title
oliviertassinari
changed the title
Remove the pigment logic form argos push script, just generate the screenshots in the same screenshots dir as core, it's the same outcome. (which was not ideal in the first place because it may overwrite but ok, will be fixed once pigment regression tests are moved to the pigment project)
Make the folder for argos push script configurable so we can reuse it more easily