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 coverage failure in case of an invalid file #1794

Open
wants to merge 1 commit into
base: 0.26
Choose a base branch
from

Conversation

mabar
Copy link

@mabar mabar commented Dec 23, 2022

This PR:

My lib supports two incompatible versions of a dependency. Code for older version extends class that does not exist in newer one. Currently code coverage fails with Uncaught Error: Class "Latte\Macros\MacroSet" not found https://github.com/orisai/localization/actions/runs/3766779916/jobs/6403641877 (shown run uses only temporary commit, I also had to do the same fix in sebastianbergmann/php-code-coverage#973)

After applying the fix, coverage no longer fails. And file is correctly reported as uncovered.
orisai/localization@a0f7a0b
https://github.com/orisai/localization/actions/runs/3766885353

@mabar
Copy link
Author

mabar commented Feb 13, 2023

@maks-rafalko @theofidry Could you check that please? Thank you

@maks-rafalko
Copy link
Member

maks-rafalko commented Apr 2, 2023

Hello, sorry for the late reply. To be honest, I'm not sure I completely get the issue.

Could you please point to the code where "lib supports two incompatible versions of dependency?" I would like to clearly understand what is going on and why php-coverage/infection work with class that don't exist, as well as why it's needed in the lib (probably you should have to create a new major version instead of doing something like that).

Thank you.

@mabar
Copy link
Author

mabar commented Apr 3, 2023

Could you please point to the code where "lib supports two incompatible versions of dependency?"

Sure!
There was a breaking change in latte/latte package.
In v2 template extensions were created by extending MacroSet class https://github.com/orisai/localization/blob/7cf546c92320b7b8fffbff86895a65c162bfe616/src/Bridge/Latte/TranslationMacros.php#L13
In v3, MacroSet is removed and instead, Extension is extended https://github.com/orisai/localization/blob/7cf546c92320b7b8fffbff86895a65c162bfe616/src/Bridge/Latte/TranslationExtension.php#L21
When I run Infection, Latte 3 is installed and MacroSet class does not exist. Due to extending non-existent class, Infection breaks.

To verify my patch, run:

git clone git@github.com:orisai/localization.git
cd localization
composer update
make mutations

I have removed the patch in another branch to demonstrate failure with latest Infection:

git checkout drop-infection-patch
composer update
make mutations

probably you should have to create a new major version instead of doing something like that

I would usually agree, but with tens of dependencies, it makes upgrades so painful. Also maintenance-wise, I don't really want to maintain more major releases when I don't have to :)

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

2 participants