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

Argument #1 ($manager) must be of type Illuminate\Mail\MailManager #46180

Closed
alexanderkroneis opened this issue Feb 20, 2023 · 25 comments
Closed

Comments

@alexanderkroneis
Copy link

alexanderkroneis commented Feb 20, 2023

  • Laravel Version: 10.0.3
  • PHP Version: 8.2.0
  • Database Driver & Version:

Description:

We are using Mail::fake in a test and since we upgraded to Laravel 10 it's failing with following exception:

Illuminate\Support\Testing\Fakes\MailFake::__construct(): Argument #1 ($manager) must be of type Illuminate\Mail\MailManager, Illuminate\Support\Testing\Fakes\MailFake given, called in /var/www/vendor/laravel/framework/src/Illuminate/Support/Facades/Mail.php on line 68
Mail::fake();
Mail::assertNothingSent();
expect(fn () => $votingCard->approveRequest())->not()->toThrow(\Symfony\Component\Mime\Exception\LogicException::class);

Steps To Reproduce:

@driesvints
Copy link
Member

This was changed here: #46055 because of #45988

@joelbutcher @stevebauman do you have any ideas here on how to fix this?

@maloun96
Copy link

maloun96 commented Feb 20, 2023

I have the same issue!

@joelbutcher
Copy link
Contributor

@driesvints I can't replicate this... I've written a test that that should cover this in the framework

@driesvints
Copy link
Member

Thanks @joelbutcher. @alexgaal can you check this? ^

@joelbutcher
Copy link
Contributor

joelbutcher commented Feb 20, 2023

I've raised a PR covering this, too

@joelbutcher
Copy link
Contributor

@alexgaal @maloun96 if you run the following in tinker, what do you get?

> app('mail.manager')

The only thing I can think of is an incorrectly configured service provider binding that's causing app('mail.manager') to resolve to the MailFake class instead of MailManager.

@maloun96
Copy link

I believe it's because we declared multiple times:

    protected function setUp(): void
    {
        parent::setUp();
        Mail::fake();
    }

    public function test_123(): void
    {
        Mail::fake();

Before it worked. Now we should declare only once

@driesvints
Copy link
Member

@maloun96 that doesn't seems like valid code to me (even if it was working before).

@maloun96
Copy link

@driesvints indeed, legacy code...

@driesvints
Copy link
Member

@alexgaal are you faking emails in a similar way?

@maloun96
Copy link

maloun96 commented Feb 20, 2023

Tested with a fresh laravel app and still get errors.
image

@joelbutcher
Copy link
Contributor

joelbutcher commented Feb 20, 2023

@maloun96 this is because your test uses PHPUnit\Framework\TestCase instead of Tests\TestCase – the application doesn't get bootstrapped when using this base test case.

@maloun96
Copy link

True, thanks to all!

@joelbutcher
Copy link
Contributor

joelbutcher commented Feb 20, 2023

@driesvints this issue raises a good point about testing with facades though. If a facade implements a fake method that then builds an object based on that facade's root and returns it, any test suit that calls Facade::fake() more than once (like the example @maloun96 provided above within setUp and then testItDoesX) will break.

@taylorotwell Might it be worth covering this use-case in the testing docs?

@maloun96
Copy link

It can be nice to have the ability to fake multiple times.

@driesvints
Copy link
Member

@joelbutcher I don't think we document using faking like that anywhere?

@alexanderkroneis
Copy link
Author

alexanderkroneis commented Feb 20, 2023

@alexgaal @maloun96 if you run the following in tinker, what do you get?

> app('mail.manager')

The only thing I can think of is an incorrectly configured service provider binding that's causing app('mail.manager') to resolve to the MailFake class instead of MailManager.

Psy Shell v0.11.12 (PHP 8.2.0 — cli) by Justin Hileman
app('mail.manager')
= Illuminate\Mail\MailManager {#5302}

@alexgaal are you faking emails in a similar way?

We are using Mail::fake twice in different tests:
Bildschirm­foto 2023-02-20 um 16 08 12

@driesvints
Copy link
Member

@alexgaal can you share the full test please?

@alexanderkroneis
Copy link
Author

alexanderkroneis commented Feb 20, 2023

@driesvints Sure!

test('bwk can edit voting card requests from voters', function () {
    $votingCard = VotingCard::factory()->for(Voter::factory()->create())->create();
    $count = VotingCard::count();

    /** @var \Illuminate\Pagination\LengthAwarePaginator $paginator */
    $paginator = Livewire::test(VotingCardList::class)
        ->viewData('votingCards');

    expect($paginator->total())->toBe($count);

    // sees if a matching voter was found to the information provided inside the request
    collect($paginator->items())->filter(fn(VotingCard $votingCard) => null === $votingCard->voter)->each(
        function (VotingCard $votingCard) {
            $component = Livewire::test(VotingCardListItem::class, ['votingCard' => $votingCard])
                ->assertSee($votingCard->firstname)
                ->assertSee($votingCard->lastname);

            $suggestions = $component->get('suggestions');

            expect($suggestions->count())->toBeGreaterThan(0);

            $component->call('selectVoter', $voter = Voter::first())
                ->assertSee($voter->firstname)
                ->assertSee($voter->lastname);

            expect($votingCard->status)->toBe(VotingCardStatus::Requested);

            $mail = Mail::fake();
            Lottery::odds(1, 2)
                ->winner(function () use ($component, $mail, $votingCard) {
                    $component->call('approveRequest');

                    expect($votingCard->refresh()->status)->toBe(VotingCardStatus::RequestApproved);
                    $mail->assertSent(M2ConfirmDenyVotingCardApplication::class);
                })
                ->winner(function () use ($component, $mail, $votingCard) {
                    $component->call('denyRequest');

                    expect($votingCard->refresh()->status)->toBe(VotingCardStatus::RequestDenied);
                    $mail->assertSent(M2ConfirmDenyVotingCardApplication::class);
                })
                ->choose();
        }
    );
    // --> TODO: either sends the voting card per postal mail or hands them out manually
})->group('ewas', 'votings-cards');

...

test('missing emails do not cause a http 500', function () {
    /** @var VotingCard $votingCard */
    $votingCard = VotingCard::factory()
        ->for(
            Voter::factory()
                ->has(
                    Study::factory(['email' => ''])->for(
                        EducationalInstitutionFieldOfStudy::factory()
                            ->for(EducationalInstitution::factory())
                            ->for(FieldOfStudy::factory())
                    )
                )
        )->create();

    Mail::fake();
    Mail::assertNothingSent();
    expect(fn () => $votingCard->approveRequest())->not()->toThrow(\Symfony\Component\Mime\Exception\LogicException::class);
})->group('ewas', 'votings-cards');

@driesvints
Copy link
Member

You're calling Mail::fake(); multiple times in a single test because of the collect->each. That's not supported afaik.

@alexanderkroneis
Copy link
Author

Oh, thank you Dries! I fixed that, it's working now. Thanks to all of you.

@joelbutcher
Copy link
Contributor

joelbutcher commented Feb 20, 2023

@alexgaal @driesvints I'm working on a PR to fix improve this

@driesvints
Copy link
Member

Thanks all

@schonhoff
Copy link
Contributor

Hi @joelbutcher,
I'm not into the Facade fake awareness but could this PR fix the problem with laravel-helo and ide-helper?
beyondcode/helo-laravel#37
Maybe you have could have a look? Would be much appreciated :-) Just if you have the time to have a look into the issue. If not I will wait for the PR to be tagged and will try it myself.

Thanks and have a nice day/night.

@joelbutcher
Copy link
Contributor

joelbutcher commented Feb 22, 2023

@driesvints @taylorotwell I think we need to consider documenting using Facade::fake() only once in tests, as since #46188 was merged quite a few issues with packages upgrading to Laravel v10 have risen?

E.g. barryvdh/laravel-ide-helper#1422

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants