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

TestCase: data provider supports \Traversable #272

Merged
merged 1 commit into from Nov 14, 2015

Conversation

@jiripudil
Copy link
Contributor

jiripudil commented Nov 13, 2015

I've found that if you want to return some more complex structures from a data provider or even do some other logic to it inbetween, generators seem to be a more convenient approach than arrays.

In my case, I need to test data validation of a third-party API, generating a valid request and breaking it in a lot of slightly different ways, and generators just seem to be much more expressive:

public function dataProvider()
{
    $request = $this->createValidRequest();
    unset($request['title']);
    yield [$request, 'Missing parameter title'];

    $request = $this->createValidRequest();
    unset($request['description']);
    yield [$request, 'Missing parameter description'];

    // ...
}
@@ -87,8 +87,8 @@ public function runTest($method, array $args = NULL)

foreach ((array) $info['dataprovider'] as $provider) {
$res = $this->getData($provider);
if (!is_array($res)) {
throw new TestCaseException("Data provider $provider() doesn't return array.");
if (!is_array($res) && (PHP_VERSION_ID < 50500 || !$res instanceof \Generator)) {

This comment has been minimized.

Copy link
@dg

dg Nov 13, 2015

Member

IMHO version check is not needed

This comment has been minimized.

Copy link
@jiripudil

jiripudil Nov 13, 2015

Author Contributor

ok, I'll get rid of it

require __DIR__ . '/../bootstrap.php';


if (PHP_VERSION_ID < 50500) {

This comment has been minimized.

Copy link
@dg

dg Nov 13, 2015

Member

Better is to use anotation @phpversion 5.5, now Travis have failed.

This comment has been minimized.

Copy link
@jiripudil

jiripudil Nov 13, 2015

Author Contributor

Thanks! I didn't recall there is such annotation

@fprochazka

This comment has been minimized.

Copy link
Contributor

fprochazka commented Nov 13, 2015

👍

@milo

This comment has been minimized.

Copy link
Member

milo commented Nov 13, 2015

What about to support \Traversable instead of \Generator?

@jiripudil

This comment has been minimized.

Copy link
Contributor Author

jiripudil commented Nov 13, 2015

@milo I thought about it, but didn't find any use case for Traversables other than Generators. But yeah, why not, while I'm at it :)

@jiripudil jiripudil force-pushed the jiripudil:feature/dataprovider-generator branch from 870b790 to 027fb44 Nov 13, 2015
@jiripudil jiripudil changed the title TestCase: data provider supports generators TestCase: data provider supports \Traversable Nov 13, 2015
@jiripudil

This comment has been minimized.

Copy link
Contributor Author

jiripudil commented Nov 13, 2015

I've added support for Traversable and squashed the branch into a single commit.

if (!is_array($res)) {
throw new TestCaseException("Data provider $provider() doesn't return array.");
if (!is_array($res) && !$res instanceof \Traversable) {
throw new TestCaseException("Data provider $provider() doesn't return array or \\Traversable.");

This comment has been minimized.

Copy link
@dg

dg Nov 13, 2015

Member

Why leading slash?

This comment has been minimized.

Copy link
@jiripudil

jiripudil Nov 13, 2015

Author Contributor

removed

@@ -0,0 +1,37 @@
<?php

/** @phpversion 5.5 */

This comment has been minimized.

Copy link
@milo

milo Nov 13, 2015

Member

Split it to three lines please. To be consistent.

}
}

/** @dataProvider dataProvider*/

This comment has been minimized.

Copy link
@milo

milo Nov 13, 2015

Member

Missing space before */

array('MyTest::testDataProviderGenerator', array(1)),
array('MyTest::testDataProviderGenerator', array(2)),
array('MyTest::testDataProviderGenerator', array(3)),
), $test->order);

This comment has been minimized.

Copy link
@milo

milo Nov 13, 2015

Member

Use short array notation in tests 5.4+.

@@ -33,6 +42,12 @@ class MyTest extends Tester\TestCase
$this->order[] = array(__METHOD__, func_get_args());
}

/** @dataProvider dataProviderIterator*/

This comment has been minimized.

Copy link
@milo

milo Nov 13, 2015

Member

Missing space before */. Please, fix it in whole test, it's typo.

@milo

This comment has been minimized.

Copy link
Member

milo commented Nov 13, 2015

Imho ready to merge. I left few CS hint comments.

@jiripudil

This comment has been minimized.

Copy link
Contributor Author

jiripudil commented Nov 14, 2015

@milo I've fixed the CS compliance. Should I squash it now?

@milo

This comment has been minimized.

Copy link
Member

milo commented Nov 14, 2015

Yes please.

@jiripudil jiripudil force-pushed the jiripudil:feature/dataprovider-generator branch from 98b0db6 to 7066509 Nov 14, 2015
@jiripudil

This comment has been minimized.

Copy link
Contributor Author

jiripudil commented Nov 14, 2015

Squashed.

milo added a commit that referenced this pull request Nov 14, 2015
TestCase: data provider supports \Traversable
@milo milo merged commit fea9659 into nette:master Nov 14, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@milo

This comment has been minimized.

Copy link
Member

milo commented Nov 14, 2015

Thank you.

@jiripudil jiripudil deleted the jiripudil:feature/dataprovider-generator branch Nov 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.