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

Compatibility with PHPUnit 11 #61

Merged
merged 3 commits into from
Jul 16, 2024
Merged

Conversation

shulard
Copy link
Member

@shulard shulard commented Jul 15, 2024

This library depend on phpunit/php-code-coverage, this PR allow compatibility with v11 of that library.

It also requires a new dependency (which was already included before trough the phpunit/php-code-coverage). I think it’s mandatory because we are using something from this lib.

@shulard
Copy link
Member Author

shulard commented Jul 15, 2024

Hello @jaylinski !

This PR is to handle the PHPunit 11 update.

I added an exception to handle the deprecation for the blacklist options.

Let me know if it’s fine for you ^^

If we merge this one, we must tag a new minor release then we can create a new major after removing the blacklist options.

@shulard shulard force-pushed the chore/phpunit-php-code-coverage-11 branch 2 times, most recently from d6c8bb5 to fc0886d Compare July 15, 2024 19:51
@shulard
Copy link
Member Author

shulard commented Jul 15, 2024

It looks like I find a way to deal with the blacklist options. The FileIterator library provide the ability to exclude some file and folders so I refactored the code to use this.

We can think about dropping this option but it must be fine for now to keep it ^^

Copy link
Member

@jaylinski jaylinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Left some comments about a bunch of minor issues.

src/Listener/CodeCoverageListener.php Show resolved Hide resolved
src/Listener/CodeCoverageListener.php Outdated Show resolved Hide resolved
src/Listener/CodeCoverageListener.php Outdated Show resolved Hide resolved
@@ -35,7 +35,7 @@ public function let(ConsoleIO $io, Driver $driver)
public function it_can_process_all_directory_filtering_options(SuiteEvent $event)
{
$this->setOptions([
'blacklist' => [
'whitelist' => [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add a dedicated blacklist-test.

Otherwise our code-coverage will drop.

Sadly the CI pipeline didn't upload the code-coverage to Scrutinizer (https://scrutinizer-ci.com/g/friends-of-phpspec/phpspec-code-coverage/), maybe something is broken there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been pushed but you need to check the inspections. The dashboard page is only displaying master build.

Here is the result for the latest push: https://scrutinizer-ci.com/g/friends-of-phpspec/phpspec-code-coverage/inspections/8024bdbd-49db-4a89-91b3-d0a179fdf1e5

@shulard shulard force-pushed the chore/phpunit-php-code-coverage-11 branch from fc0886d to c914e43 Compare July 16, 2024 06:43
shulard added 3 commits July 16, 2024 08:46
Also require phpunit/php-file-iterator which is used to find the files to include.
Since the phpunit/php-code-coverage isn't able to exclude any folder from version 11, we must handle that logic differently.

We now compute a list of files and folder to be excluded and we give it to the filter.
@shulard shulard force-pushed the chore/phpunit-php-code-coverage-11 branch from c914e43 to 1dc3e64 Compare July 16, 2024 06:46
Copy link
Member

@jaylinski jaylinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, nice work!

@shulard shulard merged commit ff137d7 into master Jul 16, 2024
147 checks passed
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