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

Test fails due to changes in v8.48.1 → v8.48.2 #22

Closed
mpyw opened this issue Jun 28, 2021 · 4 comments · Fixed by #23
Closed

Test fails due to changes in v8.48.1 → v8.48.2 #22

mpyw opened this issue Jun 28, 2021 · 4 comments · Fixed by #23
Labels
help wanted Extra attention is needed

Comments

@mpyw
Copy link
Owner

mpyw commented Jun 28, 2021

In contrary v8.48.1 → v8.48.2 breaks my test...

    public function testInitializationForMailables(): void
    {
        $this->mock(Mailer::class)->shouldReceive('send');

        // [7.x] Multiple Mailers Per App
        // https://github.com/laravel/framework/pull/31073
        if (interface_exists(MailerFactory::class)) {
            $this->mock(MailerFactory::class)
                ->shouldReceive('mailer')
                ->andReturn($this->app->make(Mailer::class));
        }

        DB::connection();

        $this->assertFalse($this->getRecordsModifiedViaReflection());

        Mail::send(new GeneralMailable());

        // ...
}
@mpyw mpyw added the help wanted Extra attention is needed label Jun 28, 2021
@mpyw
Copy link
Owner Author

mpyw commented Jun 28, 2021

@Nacoma 🥺

@mpyw
Copy link
Owner Author

mpyw commented Jun 28, 2021

Abandoning Laravel's mock() wrapper works well but it still seems to be a "workaround".

$this->app->instance(Mailer::class, \Mockery::mock(Mailer::class, function (MockInterface $mock) {
    $mock->shouldReceive('send');
}));

// [7.x] Multiple Mailers Per App
// https://github.com/laravel/framework/pull/31073
if (interface_exists(MailerFactory::class)) {
    $this->app->instance(MailerFactory::class, \Mockery::mock(MailerFactory::class, function (MockInterface $mock) {
        $mock
            ->shouldReceive('mailer')
            ->andReturn($this->app->make(Mailer::class));
    }));
}

@mpyw
Copy link
Owner Author

mpyw commented Jun 28, 2021

Mail::getFacadeRoot()

  • Previously returns Illuminate\Mail\MailManager
  • Now returns Mockery_2_Illuminate_Contracts_Mail_Factory

@mpyw
Copy link
Owner Author

mpyw commented Jun 28, 2021

laravel/framework#37789 (comment)

I got it!

  • @Nacoma's fix is still correct.
  • We can't mock drivers delegated by manager's magic methods and macros.
    • In my previous test, MailerFactory was not correctly mocked.
// Without Mocking
Mail::send($mailable);
↓
$mailManager = Mailer::getFacadeRoot();
$mailManager->send($mailable);
↓
$mailManager = Mailer::getFacadeRoot();
$mailManager->__call('send', [$mailable]);
↓
$mailManager = Mailer::getFacadeRoot();
$mailManager->mailer()->send($mailable);
// With Mocking
Mail::send($mailable);
↓
$mailManagerMock = Mailer::getFacadeRoot();
$mailManagerMock->send($mailable);
↓
// Mockery can't find `Mailer::send()` method under `MailManager::__call()` delegation.

How do we get correct approach (not workarounds) for this problem...? @taylorotwell

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant