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 BC break for Log feature tests #42987

Merged
merged 4 commits into from
Jun 28, 2022
Merged

[9.x] Fix BC break for Log feature tests #42987

merged 4 commits into from
Jun 28, 2022

Conversation

michael-rubel
Copy link
Contributor

About

PR #41925 introduced a breaking change for Log feature tests. This PR reverts Handler class behavior to call log levels on logger instead of always relying on log method. If the original log level doesn't exist in the Logger class as a method, we're directing it to the log method.

Steps to reproduce

Use Log facade to access logger:

Log::debug('Message');

Then we expect debug to be called in feature tests:

Log::shouldReceive('debug')
    ->withArgs(fn ($message) => $message === 'Message')
    ->once();

But the test fails, because:

Received Mockery_0_Illuminate_Log_LogManager::log(), but no expectations were specified

@michael-rubel michael-rubel changed the title [9.x] Fix Log feature tests [9.x] Fix BC break for Log feature tests Jun 28, 2022
@driesvints
Copy link
Member

Ping @Gummibeer

@taylorotwell taylorotwell merged commit 8f1d548 into laravel:9.x Jun 28, 2022
@ankurk91
Copy link
Contributor

This breaking change was introduced in April, and we updated our tests accordingly.

But after this PR, we need to revert our tests. :(

This PR has created more work for us (again) , I think it is too late to revert this PR.

@driesvints
Copy link
Member

@ankurk91 why didn't you report the breaking change?

@olsza
Copy link

olsza commented Jun 29, 2022

This breaking change was introduced in April, and we updated our tests accordingly.

But after this PR, we need to revert our tests. :(

This PR has created more work for us (again) , I think it is too late to revert this PR.

@ankurk91 that's what tests are for, at @michael-rubel they blew the basic tests and he immediately checked and corrected ;)
@ankurk91 before you change tests, ask the community ;)

@ankurk91
Copy link
Contributor

@olsza
I agree with you, i thought It was just me. Will take care next time.

ankurk91 added a commit to ankurk91/laravel-stripe-exceptions that referenced this pull request Jun 29, 2022
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.

5 participants