Skip to content

[Messenger] Fix doctrine tests#31473

Merged
Tobion merged 1 commit intosymfony:4.3from
weaverryan:messenger-fix-doctrine-tests
May 11, 2019
Merged

[Messenger] Fix doctrine tests#31473
Tobion merged 1 commit intosymfony:4.3from
weaverryan:messenger-fix-doctrine-tests

Conversation

@weaverryan
Copy link
Copy Markdown
Contributor

@weaverryan weaverryan commented May 10, 2019

Q A
Branch? 4.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets none
License MIT
Doc PR not needed

Not sure why, but when passing in pdo, the Doctrine Connection::_conn property is a PDO instance and isn't wrapped. In the master branch of doctrine/dbal, they now check for this an throw an exception.

@nicolas-grekas has a PR in Doctrine for this (doctrine/dbal#3543), but I don't see any reason we shouldn't just avoid the pdo option entirely.

Copy link
Copy Markdown
Contributor

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Copy Markdown
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same :) Thanks

@Tobion
Copy link
Copy Markdown
Contributor

Tobion commented May 11, 2019

Good catch, thanks Ryan.

@Tobion Tobion merged commit a7cf3f9 into symfony:4.3 May 11, 2019
Tobion added a commit that referenced this pull request May 11, 2019
This PR was merged into the 4.3 branch.

Discussion
----------

[Messenger] Fix doctrine tests

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | none
| License       | MIT
| Doc PR        | not needed

Not sure why, but when passing in `pdo`, the Doctrine `Connection::_conn` property is a PDO instance and isn't wrapped. In the master branch of `doctrine/dbal`, they now check for this an throw an exception.

@nicolas-grekas has a PR in Doctrine for this (doctrine/dbal#3543), but I don't see any reason we shouldn't just avoid the `pdo` option entirely.

Commits
-------

a7cf3f9 Fixing tests - passing pdo is not wrapped for some reason in dbal
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants