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

@dataProvider using iterator_to_array() #2034

Closed
bwoebi opened this issue Jan 12, 2016 · 12 comments
Closed

@dataProvider using iterator_to_array() #2034

bwoebi opened this issue Jan 12, 2016 · 12 comments
Labels
type/refactoring A refactoring that should be applied to make the code easier to understand and maintain

Comments

@bwoebi
Copy link
Contributor

bwoebi commented Jan 12, 2016

Can we please iterate one-by-one over an array?
In my concrete example, I have 288 testcases needing each 2-10MB of data. I'm yielding them, one-by-one in my dataprovider, but phpunit is applying iterator_to_array() to it.

What's happening? An out of memory (more than 1GB of memory is needed, not acceptable on many systems). ... I thought that were the precise point of using an iterator dataprovider in order to have them processed one-by-one?

@sebastianbergmann
Copy link
Owner

The current architecture of PHPUnit's instantiates all test objects ahead of time. So using iterator_to_array() or not does not make a difference.

@bwoebi
Copy link
Contributor Author

bwoebi commented Jan 13, 2016

Hmm, and I guess it'd be a superior amount of work to refactor it in a way to incrementally fetch tests?
The only disadvantage I see is not having a proper counter how much are left or are there other problems with it?

@sebastianbergmann
Copy link
Owner

The count is obvious, of course. However, I fear that there are many hidden assumptions based on the current architecture.

@bwoebi
Copy link
Contributor Author

bwoebi commented Jan 13, 2016

Don't know. One has to go ahead and see what breaks.

@ScreamingDev
Copy link

ScreamingDev commented Aug 15, 2016

I looked at PHPUnit 5.5
This is not impossible but needs refactoring at some points.

For example:

if ($dataProviderMethod->getNumberOfParameters() == 0) {
                $data = $dataProviderMethod->invoke($object);
            } else {
                $data = $dataProviderMethod->invoke($object, $methodName);
            }

should be

if ($dataProviderMethod->getNumberOfParameters() == 0) {
                yield $dataProviderMethod->invoke($object);
            } else {
                yield $dataProviderMethod->invoke($object, $methodName);
            }

And the return below needs to be a yield too.
After that follow additional changes to \PHPUnit_Util_Test::getProvidedData which should yield everything out. Especially lines like this

if ($dataProviderData = self::getDataFromDataProviderAnnotation($docComment, $className, $methodName)) {

Are a problem. But still solvable (wait for it).
And so on until you reach vendor/phpunit/phpunit/src/Framework/TestSuite.php:485.
There is a mess in \PHPUnit_Framework_TestSuite::createTest and seems to be the hardest part.

But the overall solution and shortest way will be using/adding a hasProvidedData instead of getProvidedData. Assertions on the data itself should be made while it flows and not when it is done.

I had a huge amount of data and like to see the dots flow every 150ms.
Instead I also have just one dot that comes up after a little less than 2 minutes.

Hope to see that soon ;)

Cheers!

@ScreamingDev
Copy link

ScreamingDev commented Aug 15, 2016

I invested an hour and came this far:

https://github.com/sourcerer-mike/phpunit/commit/c2b8a0ae0377d3cb99e9eb147ea0961e6bc4022f

There is a point in PHPUnit 5.5 src/Framework/TestSuite.php:584 where you stack all tests.
This is the next point that needs changes and yield out the tests instead of stacking them.

I can imagine something like src/Framework/TestSuite.php:584:

yield $groups => $_test

and in src/Framework/TestSuite.php:619:

yield $test;

Which needs \PHPUnit_Framework_TestSuite::addTestMethod to respect ( src/Framework/TestSuite.php:886 ).

Hope you will continue from here and publish it as a patch or any other way very soon @sebastianbergmann ;)

@sebastianbergmann
Copy link
Owner

I think this is blocked by #10.

@sebastianbergmann sebastianbergmann added status/blocked Blocked by a dependency, or similar type/refactoring A refactoring that should be applied to make the code easier to understand and maintain labels Oct 14, 2017
@stale stale bot added the stale label Jul 2, 2019
Repository owner deleted a comment from stale bot Jul 2, 2019
@danon
Copy link

danon commented Jul 18, 2019

I'm using a @dataProvider which loads and parses data from a file. Sometimes, parsing fails - ulimately failing every dataProvider case.

I would very much like if the failing entry would only fail one dataProvider, and let other pass. It would also be great to use a deubgger to run that.

Of course I could try/catch that one entry, but then every test would pass (even the one with broken entry).

And to acheive that, I would also need dataProvider to be run one after another. I can even supply an iterator or a callback, should you need to have these references earlier. Any way to do that would be great.

@epdenouden epdenouden added PHPUnit 8 and removed status/blocked Blocked by a dependency, or similar labels Jul 19, 2019
@epdenouden
Copy link
Contributor

epdenouden commented Jul 19, 2019

Hello @bwoebi, @danon and everybody else still reading this: this will be solved by the improved data provider handling.

@ScreamingDev exactly! You were looking in the right direction. Why did you stop, though?? I mean, it would only have been a few more weeks of work.

ping @pcrov @mariofischer @jdufresne as you 👍'd this

@epdenouden
Copy link
Contributor

Superseded by #3736

@ScreamingDev
Copy link

@epdenouden you asked and answered it at once xD n1.

@epdenouden
Copy link
Contributor

@epdenouden you asked and answered it at once xD n1.

So sad, I thought this one hour 'developer relations workshop' would teach me the tricks to get other devs to do all the work 😞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/refactoring A refactoring that should be applied to make the code easier to understand and maintain
Projects
None yet
Development

No branches or pull requests

5 participants