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

[9.x] Fix expectsDatabaseQueryCount() $connection parameter #46228

Merged
merged 2 commits into from
Feb 23, 2023
Merged

[9.x] Fix expectsDatabaseQueryCount() $connection parameter #46228

merged 2 commits into from
Feb 23, 2023

Conversation

giggsey
Copy link
Contributor

@giggsey giggsey commented Feb 22, 2023

The listen() function was assuming that the event was being dispatched for just that connection, rather than globally.

Instead, we now check the connection name matches if it was provided.

Fixes #45932

The listen() function was assuming that the event was being dispatched for just that connection, rather than globally.

Instead, we now check the connection name matches if it was provided.

Fixes #45932
@giggsey giggsey changed the base branch from 10.x to 9.x February 22, 2023 21:17
@giggsey
Copy link
Contributor Author

giggsey commented Feb 22, 2023

I think the checks failed because I created this PR against 10.x instead of 9.x. I'm not sure which version I should have targeted. My feeling was 9.x because the feature was only added 3 weeks ago, and didn't work as expected.

@timacdonald
Copy link
Member

@giggsey I personally think this is a bug with the connection, rather than the test helper. Going to open a different PR to address the bug.

@timacdonald
Copy link
Member

See #46231

@giggsey
Copy link
Contributor Author

giggsey commented Feb 23, 2023

@giggsey I personally think this is a bug with the connection, rather than the test helper. Going to open a different PR to address the bug.

Whilst I agree, I feel that's a bigger backwards incompatible change that has more potential to break user's applications.

At the very least, should the additional test in this PR be added after your new PR?

@taylorotwell taylorotwell merged commit 344c0d8 into laravel:9.x Feb 23, 2023
@giggsey giggsey deleted the fix-expectsDatabaseQueryCount branch February 23, 2023 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants