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

DX: Use "yield from" in tests #5905

Merged
merged 1 commit into from
Aug 28, 2021
Merged

DX: Use "yield from" in tests #5905

merged 1 commit into from
Aug 28, 2021

Conversation

kubawerlos
Copy link
Contributor

The scenario is almost always the same, huge array $tests and the foreach with yield and some if after:

        $tests = [/** ... */];

        foreach ($tests as $index => $test) {
            yield $index => $test;
        }

        if (\PHP_VERSION_ID < 80000) {

Copy link
Member

@julienfalque julienfalque left a comment

Choose a reason for hiding this comment

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

I would argue we should extract conditional cases into dedicated providers with @require annotations, but this can be done later.

@kubawerlos kubawerlos added the RTM Ready To Merge label Aug 28, 2021
@kubawerlos kubawerlos added this to the 3.0.3 milestone Aug 28, 2021
@kubawerlos
Copy link
Contributor Author

I would argue we should extract conditional cases into dedicated providers with @require annotations, but this can be done later.

I'd like to discuss it, so we have some order in tests/providers, so when bumping lowest PHP version we have this, not this.

@keradus
Copy link
Member

keradus commented Aug 28, 2021

i'm open to discuss how to have conditional tests existing, but IMHO it's good value to see some tests were skipped due to missing requirements.

it;s different to say " 100 / 100 tests passed" vs "100 / 150 tests passed and 50 could not be executed due to not fulfilled requirements"

@keradus keradus changed the title Use "yield from" in tests DX: Use "yield from" in tests Aug 28, 2021
Copy link
Member

@keradus keradus left a comment

Choose a reason for hiding this comment

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

anyway, good cleanup

@keradus
Copy link
Member

keradus commented Aug 28, 2021

Thank you @kubawerlos.

@keradus keradus merged commit a536b4c into PHP-CS-Fixer:3.0 Aug 28, 2021
@kubawerlos
Copy link
Contributor Author

it;s different to say " 100 / 100 tests passed" vs "100 / 150 tests passed and 50 could not be executed due to not fulfilled requirements"

what I would REALLY like is to have a way to get reported tests that are skipped in every PHP version we support (like the removed one with @requires PHP <7.0 from #5902).

@kubawerlos kubawerlos deleted the use_yield_from_in_tests branch August 28, 2021 13:58
@coveralls
Copy link

coveralls commented Aug 28, 2021

Coverage Status

Coverage remained the same at 92.292% when pulling bc656ec on kubawerlos:use_yield_from_in_tests into 0a4f6b8 on FriendsOfPHP:3.0.

@keradus keradus removed the RTM Ready To Merge label Aug 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants