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

protected dataProvider method reported as required to be public #157

Closed
janedbal opened this issue Dec 14, 2022 · 8 comments
Closed

protected dataProvider method reported as required to be public #157

janedbal opened this issue Dec 14, 2022 · 8 comments

Comments

@janedbal
Copy link
Contributor

There is new check about dataProviders being public introduced in #150, but protected providers work just fine. I just tested private one and that also works (invocation over method reflection used behaind the scenes).

  • Used phpunit/phpunit version: 9.5.26
  • Used phpstan/phpstan-phpunit version: 1.3.2
@ondrejmirtes
Copy link
Member

/cc @villfa

@villfa
Copy link
Contributor

villfa commented Dec 14, 2022

Hi @janedbal,

May you provide a repro?
On my side I tried to reproduce what you describe this way:

<?php

use PHPUnit\Framework\TestCase;

class DummyTest extends TestCase
{
    /** @dataProvider provideValue */
    public function test($arg): void
    {
        self::assertTrue($arg);
    }

    protected static function provideValue(): iterable
    {
        return [
            [true],
        ];
    }
}

But I got this error:

# vendor/bin/phpunit tests/DummyTest.php 
PHPUnit 9.5.26 by Sebastian Bergmann and contributors.

E                                                                   1 / 1 (100%)

Time: 00:00.035, Memory: 6.00 MB

There was 1 error:

1) Error
The data provider specified for DummyTest::test is invalid.
ReflectionException: Trying to invoke protected method DummyTest::provideValue() from scope ReflectionMethod

ERRORS!
Tests: 1, Assertions: 0, Errors: 1.

I've also checked the documentation:

A data provider method must be public and either return an array of arrays or an object that implements the Iterator interface and yields an array for each iteration step.

EDIT:
Here the PHP version used to run the test:

# php -v
PHP 8.0.25 (cli) (built: Nov 15 2022 03:22:16) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.25, Copyright (c) Zend Technologies
    with Xdebug v3.1.6, Copyright (c) 2002-2022, by Derick Rethans

@janedbal
Copy link
Contributor Author

I'm using PHP 8.1 where you dont need to call setAccessible. That might be the reason.

@janedbal
Copy link
Contributor Author

janedbal commented Dec 14, 2022

Confirmed, your example works on PHP 8.1 and does not on PHP 8.0.

composer require phpunit/phpunit
composer require phpstan/phpstan-phpunit
nano DummyTest.php
# paste your contents
docker run --rm -it -v "$PWD":/usr/src/myapp -w /usr/src/myapp php:8.0-cli php vendor/bin/phpunit DummyTest.php # fails
docker run --rm -it -v "$PWD":/usr/src/myapp -w /usr/src/myapp php:8.1-cli php vendor/bin/phpunit DummyTest.php # works

@villfa
Copy link
Contributor

villfa commented Dec 14, 2022

I confirm it too (tried with PHP 8.1.13).

I don't know if it is possible, nor if it's aligned with the PHPStan's way of doing things, but here a proposal:

  • if the minimum PHP version required by the project is lower than 8.1, the rule is always enabled. If there is no version specified I guess we'd fallback on the version used to execute PHPStan.
  • If the version is >= 8.1, the rule would be enabled depending on a parameter (false by default, true if phpstan-strict-rules is installed).

@ondrejmirtes WDYT?

@ondrejmirtes
Copy link
Member

Oh god, what a mess. Well, first I'd ask over at PHPUnit to see whether this is intended or not. Documentation still states that dataProvider must be public. So I did: sebastianbergmann/phpunit#5115

@ondrejmirtes
Copy link
Member

In the future PHPUnit is gonan require dataProviders to be public again: sebastianbergmann/phpunit#5115 (comment)

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants