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

Fix CI #73

Merged
merged 5 commits into from
Oct 10, 2024
Merged

Fix CI #73

merged 5 commits into from
Oct 10, 2024

Conversation

bocharsky-bw
Copy link
Member

@bocharsky-bw bocharsky-bw commented Oct 7, 2024

Fixing CI errors discovered in #72

@bocharsky-bw bocharsky-bw merged commit befe180 into SymfonyCasts:main Oct 10, 2024
12 checks passed
@bocharsky-bw bocharsky-bw deleted the fix-ci branch October 10, 2024 12:58
@justim
Copy link

justim commented Oct 16, 2024

Adding the conflict for phpunit/phpunit >= 10 prevents installing this package for projects that have PHPUnit 10 and/or 11 installed.

The logs from CI aren't helpful to see why it fails, the only thing I see is "Process completed with exit code 1.".

Since PHPUnit is a dev dependency, is there another way to solve this? Anything I can do here to help?

@bocharsky-bw
Copy link
Member Author

@justim hm I see, ideally you should use PHPUnit bridge instead of having PHPUnit dependency in your project, but having it is a valid option too.

I tried to unblock PHPUnit in #78 but seems there's an incompatibility issue that causes tests fail, if you have any ideas how to fix it - please, feel free to help

@bocharsky-bw
Copy link
Member Author

bocharsky-bw commented Oct 16, 2024

@justim IIRC Symfony is not ready for PHPUnit 10+ yet. Do you use PHPUnit 10 with the installed PHPUnit Bridge in your project?

@bocharsky-bw
Copy link
Member Author

bocharsky-bw commented Oct 16, 2024

The only way I see for now - either temporarily add "phpunit/phpunit": "^9.6" to the require-dev section or add "phpunit/phpunit": ">=10" into conflict section. IMO Conflict section fits better in this specific case because this package isn't related on phpunit/phpunit directly.

Also, see some related symfony/symfony#49069

If you have any ideas how to fix it in a different way - please, feel free to share your thoughts or create a PR.

@justim
Copy link

justim commented Oct 17, 2024

Adding the "phpunit/phpunit": "^9.6" makes the most sense here, I think.

Running the tests does not work with newer versions, so this package depends on that version of PHPUnit (9.6). And, I would say, if you use classes from a package (PHPUnit\Framework\TestCase), then it should be in your composer.json to prevent surprises when one of the dependencies all of a sudden upgrades their dependencies.

This also makes sure this constraint is only there when cloning this repo and running composer install, this has no impact on users that run composer require symfonycasts/sass-bundle to add this package to their project. Using "phpunit/phpunit": ">=10" in the "conflict" section blocks users from users installing this package if they have PHPUnit 10/11 installed. And when users install this they don't install the packages in "require-dev", and will never run the tests in this package.

@justim IIRC Symfony is not ready for PHPUnit 10+ yet. Do you use PHPUnit 10 with the installed PHPUnit Bridge in your project?

Running composer require --dev -W phpunit/phpunit on a fresh Symfony project installs PHPUnit 11. We did not use the bridge.

Want me to do a PR?

@bocharsky-bw
Copy link
Member Author

@justim yeah, I think you're right, thanks for pointing into it. I missed the fact that conflict works for all while require-dev - only for dev. I made the change in my PR #78

Once again, thanks for reporting it and discuss the possible solutions ❤️

@justim
Copy link

justim commented Oct 17, 2024

You're welcome 👍

Thanks for picking it up so quickly.

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.

2 participants