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

Run tests for all supported versions defined for peerDependencies per component #1417

Merged

Conversation

evertharmeling
Copy link
Contributor

@evertharmeling evertharmeling commented Jan 24, 2024

Q A
Bug fix? no
New feature? yes
Issues Relates to #1389
License MIT

Refactored the run-vitest-all.sh script to loop over every component/workspace and check if there are peerDependencies who defined support for multiple versions (like https://github.com/symfony/ux/blob/2.x/src/Svelte/assets/package.json#L23). The script then runs the tests for every defined version for that library in that workspace.

The greatest benefit of this change is that this needs no extra administration in keeping a matrix value up-to-date in the GitHub workflow for every library in a workspace that supports more than 1 version as it checks every package.json automatically.

Tell me what you think about it! Cheers!

@weaverryan weaverryan force-pushed the multiple-js-lib-version-testsuite branch from 052f43a to 16fa514 Compare January 29, 2024 14:48
@weaverryan
Copy link
Member

Fantastic! The shell script is now a bit complex, but this is very pragmatic, so let's roll with it. Thank you Evert!

@weaverryan weaverryan merged commit b52bd37 into symfony:2.x Jan 29, 2024
16 checks passed
@evertharmeling
Copy link
Contributor Author

Great! Yeah I thought it was the most pragmatic way to fix it and since there's lots of documentation on shell scripting, it should be doable to keep working on it. Let's see what the future brings :)

Only thing to keep in mind that the script currently checks on || (double pipe) for finding out if multiple versions are defined. So the format ^3.41 || ^4.0 is kinda mandatory within the repo or we should add support for alternative syntax (like single pipe).

Cheers!

@smnandre
Copy link
Collaborator

Just a little thing @evertharmeling ... it does not work for me on macOS :/

I'll try to fix what i can... but i may need to change/adapt minor things (will poke you)

@evertharmeling
Copy link
Contributor Author

evertharmeling commented Jan 31, 2024

Alright, what error do you get? I've also opened up #1433 where the boundary range notation caused problems. Let me know if I can help

EDIT: we could also add a flag to run the 'full version'-suite and by default stick to the old way, so in CI we could leverage the 'full version'.

@smnandre
Copy link
Collaborator

No your changes are perfect... i just need to fix something (and some errors may come from my own zsh scripts so... )

Just poking you if someone else had the problem ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants