t-regx / phpunit-data-provider Goto Github PK

@danon Is the archive section in the composer.json still needed, or are the entries in the .gitattributes sufficient? (Then I can take care of removing the archive section.)

Blocked by #18.

The term "master" is associated with racism. So many open source projects are switching their default branch to something else (most often to "main"). Here's how to do it: https://stevenmortimer.com/5-steps-to-change-github-default-branch-from-master-to-main/

Let's improve code quality by checking the code with tools for common flaws and against established standards. This should also make contributing to this project easier, and reduce the amount of work required to do PR reviews.

I'll take care of this.

• add Composer script for PHP linting
• make all PHP files strict
• add PHP_CodeSniffer and Composer scripts for this
• add PHP_CodeSniffer to the CI pipeline
• enforce that all files are strict via PHP_CodeSniffer
• autoformat all files with PhpStorm
• make the code PSR-12-compliant
• enforce PSR-12 in the CI pipeline
• add PHPStan (including Composer scripts and checks in the CI pipeline)
• fix all level-0 PHPStan-reported flaws
• fix all level-1 PHPStan-reported flaws
• fix all level-2 PHPStan-reported flaws
• fix all level-3 PHPStan-reported flaws
• fix all level-4 PHPStan-reported flaws
• fix all level-5 PHPStan-reported flaws
• fix all level-6 PHPStan-reported flaws
• fix all level-7 PHPStan-reported flaws
• fix all level-8 PHPStan-reported flaws
• fix all level-9 PHPStan-reported flaws

To use the latest version of PHPStan, we'll need at least PHP 7.1 in order to be able to do a composer install.

@danon Would it be okay for you if we raised the version requirement to PHP >= 7.1?

As this will be a kind-of-breaking change, this would warrant a new major release as the next release (i.e., 3.0.0). Maybe a bug-fix version release just before the change of the PHP requirements would make sense so that users with PHP 7.0 still can benefit from the cleanup that has happened since the last release.

@danon What do you think?

Blocked by #18.

This is required for PHP-4 compliance, which in turn is required for PHP-12 compliance.

Blocked by #36.

I'd like to flatten the directory structure like this:

• src/TRegx/DataProvider/*.php -> src/*.php
• test/TRegx/DataProvider/*.php -> tests/*.php

So basically, I'd like to drop the empty in-between structure TRegx/DataProvider (without changing the PHP namespaces of the classes) and rename test/ to tests/ to be more in sync what most other PHP projects use.

@danon Would you be okay with this?

Currently, when new instances of the same values are necessary, it can't be done in the library.

For example, with this code:

    /**
     * @test
     * @dataProvider methodsAndIterators
     */
    public function shouldCallExpectedIteratorMethods(Method $method, HistoryIterator $history, iterable $dataProvider)
    {
        // given
        // when
        $this->execute($method->crossed($dataProvider));
        // then
        $expected = [
            'rewind', // start iteration
            'valid', 'current', 'key', 'next', // first iteration
            'valid', 'current', 'key', 'next', // second iteration
            'valid', 'current', 'key', 'next', // third iteration
            'valid', // iteration finished
        ];
        $this->assertSame($expected, $history->history);
    }

    public function methodsAndIterators(): DataProvider
    {
        return DataProviders::cross(
            $this->methods(),
            $this->iterators([
                [['one']],
                [['two']],
                [['three']]
            ])
        );
    }

    private function iterators(array $values): array
    {
        $iterator1 = new HistoryIterator($values);
        $iterator2 = new HistoryIterator($values);
        return [
            Iterator::class           => [$iterator1, $iterator1],
            \IteratorAggregate::class => [$iterator2, new IdentityAggregate($iterator2)],
        ];
    }

Instances $iterator1 and $iterator2 are being reused, when crossing on methods. They're different on iterators() provider, but shared on other crossed values.

Currently, there's literally no way to sidestep this issue of reused instances. The provided instance will be crossed with many other values no matter what we do.

Possible ways to handle it:

• Add Builder.afterEach() and Builder.beforeEach() - callbacks that can be used to "reset" the state of the instances
• Add Builder.addComputed(callable) which can be used to supply new instances each time provider requests value

Nice package! I tried running it as documented, with:

/**
 * @test
 * @dataProviders getSearchParameters
 */
public function users_can_search_for_booking($role, $field)
{
    dump($role, $field);
}

public function getSearchParameters()
{
    return DataProviders::cross(
        [ ['admin'], ['partner'], ['user'], ],
        [ ['title'], ['email'], ['city'], ['state'], ['description'], ]
    );
}}

That did not work for me. What did work was using the CrossDataProviders directly, like this:

/**
 * @test
 * @dataProvider getSearchParameters    // NOTE: using @dataProvider here, not @dataProviders
 */
public function users_can_search_for_booking($role, $field)
{
    dump($role, $field);
}

public function getSearchParameters()
{
    return CrossDataProviders::cross(
        [ ['admin'], ['partner'], ['user'], ],
        [ ['title'], ['email'], ['city'], ['state'], ['description'], ]
    );
}