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

Data providers / parameterized tests #208

Closed
ezzatron opened this issue Feb 6, 2017 · 4 comments
Closed

Data providers / parameterized tests #208

ezzatron opened this issue Feb 6, 2017 · 4 comments
Labels

Comments

@ezzatron
Copy link
Contributor

ezzatron commented Feb 6, 2017

I think the concept of data providers is something that lots of people will look for when switching from PHPUnit to Peridot. I think I brought it up with @brianium during our first get-together, and there's #183 and peridot-php/peridot-core#16 related to this issue already.

I had a conversation with my friend @koden-km about how to migrate some PHPUnit tests that use Data Providers over to Peridot.

We came up with a solution that utilized an associative array and a foreach loop. Here's some sample code:

describe('should produce a double double when there are double digit statistics for', function () {
    /**
     * Builds data for every combination of stats that can be a double double.
     */
    $doubleDoubleDataProvider = function () {
        $statistics = ['points', 'rebounds_defensive', 'assists', 'steals', 'blocks'];
        $statisticsCount = count($statistics);
        $data = [];

        for ($i = 0; $i < $statisticsCount; ++$i) {
            for ($j = $i + 1; $j < $statisticsCount; ++$j) {
                $subStatistics = [$statistics[$i], $statistics[$j]];
                $data[implode(', ', $subStatistics)] = $subStatistics;
            }
        }

        return $data;
    };

    foreach ($doubleDoubleDataProvider() as $label => $statistics) {
        it($label, function () use ($statistics) {
            $statistics = array_fill_keys($statistics, 10);
            $statistics['games_played'] = 1;
            $actual = $this->subject->calculateStatistics($statistics);

            expect($actual->doubleDoubles)->to->equal(1);
            expect($actual->tripleDoubles)->to->equal(0);
        });
    }
});

This produces output like this:

should produce a double double when there are double digit statistics for
  ✓ points, rebounds_defensive
  ✓ points, assists
  ✓ points, steals
  ✓ points, blocks
  ✓ rebounds_defensive, assists
  ✓ rebounds_defensive, steals
  ✓ rebounds_defensive, blocks
  ✓ assists, steals
  ✓ assists, blocks
  ✓ steals, blocks

This approach isn't too bad, but I think that's partly because this scenario only has one input to each test ($statistics). If each test required multiple inputs, either the inner closure would have to expand its list of use'd variables, or it would have to accept all inputs in a single use'd array/object, then access each by index/property.

I looked for some prior art to guide us in implementing this feature. It seems as though the typical JavaScript approach is similar to this:

[1,2,3].forEach(function (itemNumber) {
    describe("Test # " + itemNumber, function () {
        it("should be a number", function (done) {
            expect(itemNumber).to.be.a('number')
            expect(itemNumber).to.be(itemNumber) 
        });
    });
});

Unfortunately, this doesn't map to PHP very well because of the fact that PHP's closures don't automatically inherit variables from the parent scope. That is, at least until the Arrow Functions RFC is implemented.

Maybe this calls for a new Peridot feature that could somehow exploit setDefinitionArguments(). There's a definite challenge though, as plugins like peridot-phony also use this method for injecting other arguments.

Even if we make no code changes, I think having some solid documentation for how to achieve this kind of setup would help newcomers to justify the switch to Peridot.

@jails
Copy link

jails commented Apr 18, 2017

Just adding a comment here to get notifications about this issue since I'm also interested about this concern. Indeed we already had a debate around it but we didn't find any consensus yet.

@ezzatron
Copy link
Contributor Author

I'm still holding out hope that arrow functions will make for a nicer syntax for this use-case 🤞

@ragboyjr
Copy link
Contributor

@ezzatron have you checked out https://preprocess.io?

@ezzatron
Copy link
Contributor Author

Looks cool 👍

@ezzatron ezzatron closed this as not planned Won't fix, can't repro, duplicate, stale Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants