Add ability to manually setup dataproviders

[Danack]

Hi,

This is a feature request to allow users to set which data-providers are used by tests through code rather than using annotations.

I prefer to run my unit-tests in an environment that matches as closely as possible the environment that the code will be run in production. However as I don't use annotations in my code I use the php.ini option:

opcache.save_comments=0

which is a small optimization for not loading comments. However this means that all annotations fail, as the comments just aren't available to PHPunit.

It seems wrong having to either not use data-providers, or having to test in an environment that doesn't match production.

I guess the syntax for coding which data providers are used, would be something like this:

self::setDataProvider('dataProviderFunctionName', 'testName');

in the setUpBeforeClass function for the test.

cheers
Dan

[Danack]

So I have an implementation for this - it doesn't go through setupBeforeClass as it appears that would be a large amount of work.

Instead it just allows you to define a static method getDataProvider which accepts one parameter, the method being tested. i.e.

    static function getDataProvider($testMethod) {

        $dataProviders = array(
            'testRoutesValid' =>  'provideRoutesValid'
        );

        if (isset($dataProviders[$testMethod]) == true) {
            return $dataProviders[$testMethod];
        }

        return null;
    }

will use the method provideRoutesValid for the test testRoutesValid.

https://github.com/Danack/phpunit/compare/master...dataProviderWithoutAnnotation?expand=1

Any feedback on function naming etc before I submit a PR?

[JasperHorn]

In my opinion, the proper fix would be to provide a way to define "annotations" in code rather than comments. This way all annotations would work and there would be no need to specifically fix this for data providers. I could imagine it looking something like this:

public function annotationsForTestBla()
{
    return array('@dataProvider' => 'blaProvider',
                 '@depends' => array('testA',
                                     'testB'));
}

public function testBla()
{
    $this->localAnnotation('@codeCoverageIgnoreStart', '');
    // ignored code
    $this->localAnnotation('@codeCoverageIgnoreEnd', '');
    // code
}

Of course, I don't know how hard it would be to implement either half of this, and I didn't plan on doing so, so you should feel free to ignore my opinions ;)

[Danack]

The second bit:

 $this->localAnnotation('@codeCoverageIgnoreStart', '');

I think would be impossible as the data in the settings is used before the test class is even instantiated.

For the first bit, I'll change my code so that it is possible to return other types of information as well. and then see how difficult it is to add support for other annotation types.

[JasperHorn]

(Just thinking out loud.)
Wouldn't it be possible to have the localAnnotation function to do nothing when called, but extract its contents in the pre-processing step in which standard comment-annotations are also extracted? It's not as simple and it means that $var = 'localAnnotation'; $this->$var(); doesn't work, but at least you'll have a way to define inline annotations when comments aren't stored.

[Danack]

Hi Jasper,

tbh I'm not sure what you're asking. There's no parsing of functions so there's no way to use them. However I think you can do what you want now with the changed method.

So now you could do:

static function getMetadata($testMethod) {

    $metadata = array();

    $dataProviders = array(
        'testRoutesValid' =>  'provideRoutesValid'
    );

    if (isset($dataProviders[$testMethod]) == true) {
        $metadata['dataProvider'] = $dataProviders[$testMethod];
    }

    return $metadata;
}

And return whatever annotations you like for a test, though currently only the dataProvider one is supported, as I don't want to go stomping through the code without any feedback on whether this would be accepted.

Actually - the codeCoverageIgnoreStart is not an annotation for a test is it? It seems to be an annotation for the source code being tested, rather than for the test ? Sorry I don't use that so am unsure.

[JasperHorn]

Yes, codeCoverageIgnoreStart is an "inline" annotation - it's in the middle of your code and its position matters, as it isn't about the entire function. I don't use it myself either, but I was just thinking about a way to let you use such annotations outside the comments.

My idea was: I assume there is a step where PHPUnit parses your comments and extracts the annotations. In this step, one might be able to parse the function as well (or instead) and extract all (straight-forward) calls to localAnnotation and store them as annotations like would happen when parsing comment-annotations. Now that I write it down like this, I do see that's probably a bad idea for a number of reasons. (I was just thinking out loud after all.)

Anyway, I do agree, it would be good to get some feedback before getting into the code too deeply.

[Danack]

The code coverage stuff appears to be in the module https://github.com/sebastianbergmann/php-code-coverage so it would almost certainly be separate from this proposed change.

[sebastianbergmann]

You will have to live with different settings of opcache.save_comments in your test / production environment. Or simply set opcache.save_comments=1 for production. I do not believe that opcache.save_comments=0 provides a significant performance gain anyway.

[staabm]

@sebastianbergmann would it make sense to throw a warning in phpunit when opcache.save_comments=0 is used, so the user gets aware of the fact that annotations won't work because of it....?