Fixed iteration tests#303
Conversation
| * When one PULL yields RECORD(s) then FAILURE, {@see pull()} defers the {@see Neo4jException} to the next | ||
| * {@see BoltResult::fetchResults()} so records are delivered before the error (TestKit pull_2_end_error.script). | ||
| */ | ||
| private ?Neo4jException $deferredPullFailure = null; |
There was a problem hiding this comment.
Is this because of peek? Can you confirm peek is @nothrow against other drivers too? Mention it in the comment here
There was a problem hiding this comment.
Yes — updated the $deferredPullFailure docblock: it’s not peek-only (RECORD+FAILURE in one PULL); peek uses the same path via lazy current(); end-of-stream peek is non-throwing (null), matching official drivers.
| /** @var list<int> $ids */ | ||
| $ids = $path->ids; | ||
| /** @var list<int> $indices */ | ||
| $indices = $path->indices; |
There was a problem hiding this comment.
Is this backwards compatible? What happens in a Neo4j 4.4 setting?
Another option is that it is already handled in the bolt library; if so, we need to ensure the minimum version enforces that the Path class has actual indices. Otherwise, this package will break for users.
There was a problem hiding this comment.
It’s backwards compatible because our minimum dependency is stefanak-michal/bolt:^7.4, where Path has indices and ids is just a deprecated alias to the same array. So Neo4j 4.4 isn’t an issue here; the only risk would be someone running an older Bolt lib than our constraint. If you want extra safety, we can fall back to ids when indices isn’t present
…), PULL n=-1, and Bolt pull/disconnect fixes
p123-stack
force-pushed
the
stub-iteration-1
branch
from
79bf155 to
d496c9e
Compare
* feat: add temporal type support and UTC patch negotiation for Bolt 4.3–5.0
p123-stack
force-pushed
the
stub-iteration-1
branch
from
19f7235 to
9cf6d1c
Compare
| /** | ||
| * Maps driver exceptions to TestKit errorType strings, aligned with other drivers. | ||
| */ | ||
| final class DriverErrorTypeMapper |
There was a problem hiding this comment.
I don't see this class being used anywhere in testkit-backend. Is this safe to remove?
There was a problem hiding this comment.
It wasn't wired up anywhere. DriverErrorResponse still uses get_class($exception), so the mapper was dead code. I've removed DriverErrorTypeMapper and its unit test; no runtime behavior change.
| */ | ||
| private static function normalizeRows(array $rows): array | ||
| { | ||
| $normalized = []; |
There was a problem hiding this comment.
The results from the driver are very consistent, why do we need to normalise them?
There was a problem hiding this comment.
Removed normalizeRows() — it was only there for Psalm, not runtime. Rows are passed through as-is from $response->content, with types documented at the pull() boundary.
…t the pull boundary
2 participants
No description provided.