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

Normalize data provider names #7656

Merged

Conversation

paulbalandan
Copy link
Member

@paulbalandan paulbalandan commented Jul 5, 2023

Description
In php-cs-fixer v3.19, a new fixer php_unit_data_provider_name is released that aims to normalize data provider names used only ONCE.

Description of php_unit_data_provider_name
$ vendor/bin/php-cs-fixer describe php_unit_data_provider_name
Description of php_unit_data_provider_name rule.
Data provider names must match the name of the test.

Fixer applying this rule is risky.
Fixer could be risky if one is calling data provider by name as function.

Fixer is configurable using following options:
* prefix (string): prefix that replaces "test"; defaults to 'provide'
* suffix (string): suffix to be present at the end; defaults to 'Cases'

Fixing examples:
 * Example #1. Fixing with the default configuration.
   ---------- begin diff ----------
   --- Original
   +++ New
   @@ -1,8 +1,8 @@
    <?php
    class FooTest extends TestCase {
        /**
   -     * @dataProvider dataProvider
   +     * @dataProvider provideSomethingCases
         */
        public function testSomething($expected, $actual) {}
   -    public function dataProvider() {}
   +    public function provideSomethingCases() {}
    }

   ----------- end diff -----------

 * Example #2. Fixing with configuration: ['prefix' => 'data_', 'suffix' => ''].
   ---------- begin diff ----------
   --- Original
   +++ New
   @@ -1,8 +1,8 @@
    <?php
    class FooTest extends TestCase {
        /**
   -     * @dataProvider dt_prvdr_ftr
   +     * @dataProvider data_feature
         */
        public function test_feature($expected, $actual) {}
   -    public function dt_prvdr_ftr() {}
   +    public function data_feature() {}
    }

   ----------- end diff -----------

 * Example #3. Fixing with configuration: ['prefix' => 'provides', 'suffix' => 'Data'].
   ---------- begin diff ----------
   --- Original
   +++ New
   @@ -1,25 +1,25 @@
    <?php
    class FooTest extends TestCase {
        /**
         * @dataProvider dataProviderUsedInMultipleTests
         */
        public function testA($expected, $actual) {}
        /**
         * @dataProvider dataProviderUsedInMultipleTests
         */
        public function testB($expected, $actual) {}
        /**
   -     * @dataProvider dataProviderUsedInSingleTest
   +     * @dataProvider providesCData
         */
        public function testC($expected, $actual) {}
        /**
         * @dataProvider dataProviderUsedAsFirstInTest
         * @dataProvider dataProviderUsedAsSecondInTest
         */
        public function testD($expected, $actual) {}

        public function dataProviderUsedInMultipleTests() {}
   -    public function dataProviderUsedInSingleTest() {}
   +    public function providesCData() {}
        public function dataProviderUsedAsFirstInTest() {}
        public function dataProviderUsedAsSecondInTest() {}
    }

   ----------- end diff -----------

Based on the existing naming we have, I used the setting:

'php_unit_data_provider_name' => [
    'prefix' => 'provide',
    'suffix' => '',
],

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

.php-cs-fixer.dist.php Outdated Show resolved Hide resolved
@paulbalandan paulbalandan force-pushed the normalize-data-provider-names branch from e79ad20 to fe122c0 Compare July 6, 2023 03:47
Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

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

LGTM!

@MGatner
Copy link
Member

MGatner commented Jul 6, 2023

I like the consistency but don't like some of the resulting method names that got much more convoluted.

@paulbalandan
Copy link
Member Author

@MGatner may I know which ones are convoluted? So I can rename them?

@MGatner
Copy link
Member

MGatner commented Aug 2, 2023

Oh it was just a comment, not blocking or worth any effort. This makes it very clear which provider is being used so is plenty functional. Names like provideCheckControlStringWithLineBreakAndTabReturnsTheString are awkward but causing no harm.

@paulbalandan paulbalandan added the stale Pull requests with conflicts label Aug 2, 2023
@paulbalandan paulbalandan force-pushed the normalize-data-provider-names branch from 406c42d to 90e632a Compare August 3, 2023 04:06
@paulbalandan paulbalandan removed the stale Pull requests with conflicts label Aug 3, 2023
@paulbalandan paulbalandan force-pushed the normalize-data-provider-names branch from 90e632a to 0e7e31a Compare August 3, 2023 05:08
@paulbalandan paulbalandan merged commit b3fe701 into codeigniter4:develop Aug 3, 2023
@paulbalandan paulbalandan deleted the normalize-data-provider-names branch August 3, 2023 05:48
@kenjis kenjis mentioned this pull request Aug 4, 2023
2 tasks
@kenjis kenjis added refactor Pull requests that refactor code testing Pull requests that changes tests only labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Pull requests that refactor code testing Pull requests that changes tests only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants