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 EventFake::assertListening() for asserting string-based observer listeners #42289

Merged
merged 3 commits into from
May 9, 2022

Conversation

simonhamp
Copy link
Contributor

@simonhamp simonhamp commented May 6, 2022

Follow-up to #42193

Widening the matching mechanism for string-based assertions (from Str::endsWith('@handle') to Str::contains('@')) allowed the underlying logic to unexpectedly handle Observer event assertions, transforming the $actualListener from a string to an array without also transforming the $expectedListener from a string to an array (where the assertion is a string).

(NB: This wasn't breaking any production code, only some tests that used string-based event listener assertion for observers instead of array-based assertion)

This was missed due to a lack of testing around Observer listener assertion in the EventFake test suite. So this PR contains new tests to cover this scenario.

It addresses the issue by explicitly parsing a string-based $expectedListener assertion when the $actualListener is also a string (now always converted to an array as of #42193), so that the $expectedListener correctly matches the parsed $actualListener.

// $actualListener         $expectedListener
Model::observe(new Observer)  'Observer@method'

// Before fix
['Observer', 'method'] === 'Observer@method'
// => false, failing test

// After fix
['Observer', 'method'] === ['Observer', 'method']
// true, pass

simonhamp added 3 commits May 6, 2022 20:19
The string-based assertion doesn't work. The array-based one does
# Conflicts:
#	src/Illuminate/Support/Testing/Fakes/EventFake.php
@taylorotwell taylorotwell merged commit eb9c371 into laravel:9.x May 9, 2022
@simonhamp simonhamp deleted the fix/eventfake-model-observers branch May 11, 2022 21:43
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.

2 participants