Replaced ArrayIterator with StatementIterator in Portability\Connection #3115
Conversation
8216edf to
577f6ab
Compare
|
|
||
| public static function statementProvider() | ||
| { | ||
| if (extension_loaded('ibm_db2')) { |
There was a problem hiding this comment.
Unlike MysqliStatement and SQLAnywhereStatement, DB2Statement and other driver statement class declarations depend on corresponding extension symbols, so they only can be loaded if extensions are loaded.
|
|
||
| class StatementIteratorTest extends \Doctrine\Tests\DbalTestCase | ||
| { | ||
| public function testGettingIteratorDoesNotCallFetch() | ||
| /** | ||
| * @dataProvider statementProvider() |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Yes, I've been always using them. A function name without parentheses looks awkward.
| $stmt->getIterator(); | ||
| } | ||
|
|
||
| public function testIteratorIterationCallsFetchOncePerStep() |
There was a problem hiding this comment.
Would it make sense to require a non-empty return type on test methods? It's void in 99% of cases, but if a test returns a dependency, it's in most cases of a known type.
There was a problem hiding this comment.
I don't understand the "non-empty return type" part. You mean making all tests return something specific, instead of void?
There was a problem hiding this comment.
No, I mean that the return type must be always explicitly specified and in most cases be void.
There was a problem hiding this comment.
Of course. We just have it disabled currently in DBAL because of BC:
Line 18 in fd0f0f3
There was a problem hiding this comment.
Not sure it could be a requirement project-wide. For instance, fetch() could return either a row (array) or one field (string|null) or false (end of set). You'll have to have a very strict API to enable this, can't see it happening in the near future.
| /** | ||
| * @dataProvider statementProvider() | ||
| */ | ||
| public function testStatementIterationCallsFetchOncePerStep(string $class) |
| $this->assertIterationCallsFetchOncePerStep($stmt, $calls); | ||
| } | ||
|
|
||
| private function configureStatement(MockObject $stmt, &$calls) : void |
There was a problem hiding this comment.
can haz int typehint?
I am pretty sure @Ocramius will hate the reference here. 😆
There was a problem hiding this comment.
We've been through that a couple of times. Sometimes they just make sense.
|
|
||
| $stmtIterator = new StatementIterator($stmt); | ||
| foreach ($stmtIterator as $i => $_) { | ||
| private function assertIterationCallsFetchOncePerStep(Traversable $iterator, &$calls) : void |
There was a problem hiding this comment.
Was lazy to initialize the variable :)
| $this->assertEquals($i + 1, $calls); | ||
| } | ||
| } | ||
|
|
||
| public static function statementProvider() |
There was a problem hiding this comment.
can haz iterable return type?
There was a problem hiding this comment.
Yeah, again, was lazy to add the element type hint which the CS forces.
|
This also fixes #2705 imho. |
|
Back-ported to |
Fixes #3114