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

Allow for Mocking Mail and Queue #152

Closed
SebastianS90 opened this issue Feb 23, 2017 · 28 comments
Closed

Allow for Mocking Mail and Queue #152

SebastianS90 opened this issue Feb 23, 2017 · 28 comments

Comments

@SebastianS90
Copy link
Contributor

There is some confusion around regarding mocks together with Laravel Dusk. It is not possible to mock Facades the normal way, because HTTP requests are sent to an actual webserver (or artisan serve command) that runs in a different process instead of emulating the request in the test process.

But there is still a way how this could be done:

  • Implement new drivers that can be set as MAIL_DRIVER and QUEUE_DRIVER
  • In the .env.dusk file, these mock drivers will be set such that the Laravel instance in our webserver knows it should use them.
  • When the driver is instructed to send a mail or to queue a task, then it stores that information into a file instead of actually sending the mail
  • Inside the test case, we can use a mock interface like before, but it takes the information from the stored file instead of its internal state
  • After each test we should erase the file to start with a fresh setting in the next test case

This works as long as the test case runs on a machine that has access to the same file system as the webserver. You could even run your application on a webserver in your staging environment, mount a folder from there with sshfs onto your local development machine and then start Dusk with APP_URL pointing to your staging environment.

@georaldc
Copy link

Mailtrap could probably work too for emails.

One issue I see with the dusk .env file though is if an error were to happen during testing, I don't think cleanup occurs and your .env backup is restored. If you try running dusk again, then you lose your original .env file/backup since it gets replaced by the dusk file. Not related to this issue I know, but just a scenario I thought about now.

@SebastianS90
Copy link
Contributor Author

SebastianS90 commented Feb 23, 2017

Yes, I'm aware of the mailtrap API. But that would run over network and does not work for queues. And we don't have a nice interface for that either.

Regarding your comment on overriding .env.backup: Good catch, I created a separate issue for it and fixed it in #154

@JVMartin
Copy link

JVMartin commented Feb 26, 2017

Thank you, @SebastianS90 - we really, really need this.

It would be (frankly) insane to hit a network API in a testbed - incredibly wasteful and resource intensive.

This shouldn't be limited to just the mailer and queue - I would like to be able to mock any class and attach it to the application container just like we used to be able to do in 5.3, and perform assertions against the application state, etc.

Using files and other intermediate caches between the application and the testbed feels clumsy.

@sethetter
Copy link

@JVMartin agreed on files and intermediate caches feeling clumsy. The only other option would be to somehow expose the application context that's handling requests to the test runner context. I'm not entirely sure how that would be possible, but it's entirely possible that there's a way that I'm not aware of :)

Definitely a +1 on this issue though. Currently have to keep all the Notification, Event, and Mail mocking we're doing commented out in browser tests :(

ogf-bot pushed a commit to opengovfoundation/madison that referenced this issue Mar 8, 2017
Summary:
Commented out until [the dusk issue with mocking](laravel/dusk#152) is resolved

Resolves T211

Test Plan: * `make test`

Reviewers: doshitan

Reviewed By: doshitan

Maniphest Tasks: T206, T211

Differential Revision: https://phabricator.opengovfoundation.org/D124
@NoelDeMartin
Copy link

I think there is a better way to handle this, using an approach already being used in dusk. In order to login a user into the browser, the same problem appears, since it's not possible to access the session from the test code. However, as you may know from the documentation, it is possible to do the following:

$browser->loginAs($user)

Internally, this works because DuskServiceProvider registers some new routes in the application starting with _dusk, for example /_dusk/login/{userId}/{guard?}. Using this approach, something like the following could be implemented:

$browser->fakeMail()
        // perform actions to send an email using the browser
        ->assertMailSent('Mail Subject', '[email protected]');

With this approach, anything that is possible with normal tests can be achieved, communicating with the browser through a custom api defined for tests.

I am thinking on doing this myself, so if I can get it clean enough and nobody tells me anything bad from this approach, I may provide a pull request in the future.

@deleugpn
Copy link
Contributor

@NoelDeMartin I tried to do that, but failed miserably. It may require a more extensive time to achieve it.

My thought process was a little bit like this:

Queueable will store a serialized object in the database to be called by artisan work. With that in mind, if you could provide a serialized object (JSON) to an endpoint registered by DuskServiceProvider and have it be executed by the application, it would be possible to have any snippet of code being executed in the server-side of the test.
One step-back is that you cannot serialize a closure, which means doing an easy anonymous function is not an option.

@NoelDeMartin
Copy link

@deleugpn Yeah, my approach would be to implement all the mail testing mechanism behind those dusk api routes. I hadn't considered to have any snippet of code executed through the API since I agree that it's too complex.

@browner12
Copy link
Contributor

any progress on this? @DojoGeekRA had been discussing this on the Slack, and came across this issue.

I didn't realize the Fakes were only for Feature tests, and not Browser tests, until after a lot of frustration.

@NoelDeMartin
Copy link

@browner12 I have implemented an approach to solve this in one of my projects, I still haven't prepared it to be a decent pull request but I could share the "raw" implementation so that anyone can use it on their projects for the time being. If you're interested let me know and I'll post it somewhere and share the link.

@roberto-aguilar
Copy link

@NoelDeMartin i'm interested, if you can share it, that will be awesome 😃

@browner12
Copy link
Contributor

yah, i'm definitely curious. was thinking about different ways to implement, so it'd be great to see yours.

@deleugpn
Copy link
Contributor

@browner12 @NoelDeMartin @DojoGeekRA I'm open to feedback on https://hackernoon.com/asserting-jobs-in-queue-with-laravel-dusk-adb03654ca43

@browner12
Copy link
Contributor

thanks for the link @deleugpn . I'm hoping we can come up with a solution that does not require a queue database, and also works for other things, like asserting that notifications were sent, or an event was fired.

I was toying with the idea of creating some type of JSON object in the storage directory

@NoelDeMartin
Copy link

NoelDeMartin commented Nov 30, 2017

@browner12 @DojoGeekRA I have published a gist with the code I am using so far, along with a README explaining some points of my proposal. Check it out! https://gist.github.com/NoelDeMartin/60b70e81b483e9393aaf06cafb72e8ec

@deleugpn As @browner12 mentioned, I think the right solution shouldn't require a database since it's more setup. I also think the JSON in storage is not a solution either, since it would generate some files that are not necessary. In my solution, fake data is stored in browser cookies, so everything should go away once the test is finished.

@browner12
Copy link
Contributor

thanks for sharing your approach @NoelDeMartin.

When I look at a Dusk test, I see assertions at 2 different levels. Client assertions and server assertions. Lets take a simple test that confirms a user can signup.

public function testUserCanSignup()
{
    $user = factory(User::class)->make();

    $this->browse(function (Browser $browser) use ($user) {
        $browser->visitRoute('signup')
                ->type('email', $user->email)
                ->click('#submit')
                ->assertRouteIs('dashboard');
        });

    $this->assertDatabaseHas('users', [
        'email' => $user->email,
    ]);
}

We see assertRouteIs() which is making an assertion about something on the client side. We also see assertDatabaseHas() which is making an assertion about something on the server side.

Based on your proposal, you're putting the Mail faking and Mail assertions on the Browser (client side), which seems incorrect to me. Every assertion on the Browser is currently looking at some state of the client side. Sending mail is not a function of the client side, but rather the application (server side).

If I were to design what I want the test to look like it would be something like:

public function testUserCanSignup()
{
    $user = factory(User::class)->make();

    $this->browse(function (Browser $browser) use ($user) {
        $browser->visitRoute('signup')
                ->type('email', $user->email)
                ->click('#submit')
                ->assertRouteIs('dashboard');
        });

    $this->assertDatabaseHas('users', [
        'email' => $user->email,
    ]);

    $this->assertMailSent($user, SignupEmail::class);

    $this->assertNotificationSent($user, SignupNotification::class);  //as an example of testing notifications

    $this->assertEventFired(UserSignedUp::class);  //as an example of testing events
}

This is why I suggested the JSON in storage approach, as we need some kind of (semi)permanent storage to communicate between the application being run by the browser, and the application being run by our test. Obviously the server has no access to the cookie once the Request is gone. And we've already said that using the database (while it works) is a little too heavy-handed. So JSON in the /storage seems like a good solution.

I imagine it working by having some kind of environment/config variable that tells the specific service providers (NotificationServiceProvider, MailServiceProvider, etc) to register the Fakes, and the Fakes will write the JSON. Then the assertions would simply read off the JSON.

Let me know if I'm misunderstanding yours, or if you have ideas on this approach.

Thanks!

@NoelDeMartin
Copy link

NoelDeMartin commented Dec 1, 2017

@browner12 I see your reasoning, but I think there is a different way to look at it. There is actually 3 levels of tests:

  • Client assertions. As you mentioned, this are the ones related to the browser, things like which is the current URL or what I can see in the rendered HTML.
  • Server assertions. Also like you mentioned, things like the state of the database.
  • Test or "In-memory" assertions. This is actually what happens with mocking with laravel tests. You can see how it is used in this page of the documentation: https://laravel.com/docs/5.5/mocking

This 3rd category is something that is neither in server state (database, files) nor browser state (headless chrome), but in the test environment (memory of the process running phpunit). This is actually how mocking works in normal HTTP testing in Laravel, so that's what I am trying to achieve at the end. To be clear about this, here is the same example in Laravel docs about mocking Mail in HTTP tests, but how this could work based in my proposal for dusk:

public function testOrderShipping()
{
    $fake = Mail::duskFake();

    // Perform order shipping...
    $this->browse(function (Browser $browser) use ($user) {
        $browser->fakeService('mail', $fake)
                ->visitRoute('/ship-order');
    });

    // Assert a message was sent to the given users...
    Mail::assertSent(OrderShipped::class, function ($mail) use ($user) {
        return $mail->hasTo($user->email) &&
               $mail->hasCc('...') &&
               $mail->hasBcc('...');
    });

    // Assert a mailable was sent twice...
    Mail::assertSent(OrderShipped::class, 2);

    // Assert a mailable was not sent...
    Mail::assertNotSent(AnotherMailable::class);
}

I understand what you are saying, and the key problem here is the line where we are calling $browser->fakeService('mail') (and I know this is not the same syntax from my gist, as I said this is a WIP and these are just different implementations of the same concept). But Dusk is already doing something like this, when you want to login with the Browser you don't have to always fill in a login form, you can do $brower->loginAs(User::first()) which is comunicating with the server in the same way (laravel session id is stored in cookies, even if the data is then in the /storage). The implementation can be seen here: https://github.com/laravel/dusk/blob/2.0/src/DuskServiceProvider.php#L18..L31

I see how the syntax can be confusing, and that's why I am still working on this before doing a PR. What you mention about having the data in /storage, I don't think it's completely contrary to my approach, we could have in a cookie only the "faking id" and actually store all the fake data in a file, similar to what session is doing in Laravel. But since this will be used only for tests and we don't need persistence after the test has been run, I actually think it's better to keep everything in cookies because they are cleared when the test is finished.

PS: I am confident that doing some things behind the scenes in the Browser and DuskTestCase classes, this could even have the same syntax as normal HTTP tests, like so:

public function testOrderShipping()
{
    Mail::fake();

    // Perform order shipping...
    $this->browse(function (Browser $browser) use ($user) {
        $browser->visitRoute('/ship-order');
    });

    // Assert a mailable was sent twice...
    Mail::assertSent(OrderShipped::class, 2);
}

NoelDeMartin added a commit to NoelDeMartin/dusk that referenced this issue Feb 18, 2018
NoelDeMartin added a commit to NoelDeMartin/dusk that referenced this issue Feb 20, 2018
@NoelDeMartin
Copy link

I submitted a PR some days ago and Taylor does not want to include this right now, so the best I could do is create an external package. It can be found here: https://github.com/NoelDeMartin/laravel-dusk-mocking

Until we don't have any feedback from the maintainers, this is the best we will get. I'm open to any feedback, so if anyone has anything to improve I'm open to PRs and issues.

@jcrawford
Copy link

@NoelDeMartin I have integrated this package into Laravel 5.6 as suggested however when running my tests using your example code I get an error as such.

Exception: Unable to retrieve mock for [Illuminate\Support\Facades\Mail].

This is my test code

$this->browse(function ($browser) {
            
            $mail = $browser->fake(Mail::class);

            $browser->visit('/')
                ->type('full_name', 'Joseph Crawford')
                ->type('email', '[email protected]')
                ->type('message', 'Phone Is Not Required')
                ->press('submit')
                ->assertSee('Your contact request has been sent!');

            $mail->assertNotSent(ContactEmail::class);
        });       

Any thoughts as to why it is unable to retreive a mock for Mail?

@NoelDeMartin
Copy link

@jcrawford That works for me, so could you please open an issue in my repository with the full error trace and more details so that I can take a closer look?

@jcrawford
Copy link

@NoelDeMartin Will do.

@jontroncoso
Copy link


composer require --dev noeldemartin/laravel-dusk-mocking

gets me this:


Using version ^1.0 for noeldemartin/laravel-dusk-mocking
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Can only install one of: laravel/dusk[3.0.x-dev, v4.0.0].
    - Can only install one of: laravel/dusk[v4.0.0, 3.0.x-dev].
    - Can only install one of: laravel/dusk[3.0.x-dev, v4.0.0].
    - noeldemartin/laravel-dusk-mocking v1.0.0 requires laravel/dusk ~3.0 -> satisfiable by laravel/dusk[3.0.x-dev].
    - Installation request for noeldemartin/laravel-dusk-mocking ^1.0 -> satisfiable by noeldemartin/laravel-dusk-mocking[v1.0.0].
    - Installation request for laravel/dusk (locked at v4.0.0, required as ^4.0) -> satisfiable by laravel/dusk[v4.0.0].


Installation failed, reverting ./composer.json to its original content.

anybody know if there is a better alternative to this? or can we get similar functionality into the codebase? (or is it already there?)

@NoelDeMartin
Copy link

@jontroncoso please, open an issue in my repository since this is not a problem related with dusk. It's probably just a versions issue, since dusk 4.0 was released recently.

@NoelDeMartin
Copy link

@jontroncoso I just released a coupled of versions with the same naming as dusk (v3.0.0 and v4.0.0), so it should be fixed now.

@carcinocron
Copy link

carcinocron commented Oct 27, 2018

I'm currently just grepping the log file (and remembering to clear it overtime) but maybe the solution would be a mail driver that targets a database table (similar to a queue database table I suppose)?

@NoelDeMartin
Copy link

@InstanceOfMichael Have you seen my package? I use the same approach as what's being done to login/logout in dusk: https://github.com/NoelDeMartin/laravel-dusk-mocking

@driesvints
Copy link
Member

As this has been rejected in the past by Taylor I'm going to close this. You can use @NoelDeMartin package if you want to. Thanks @NoelDeMartin!

@naneri
Copy link

naneri commented Dec 21, 2018

Guys, you can install Mailhog and set it as SMTP driver, same usecase as mailtrap, but it works as a package and you won't be dependent on on remote service and it also has a docker container by the way.

@mshamaseen
Copy link

It would be great if we could mock in Laravel dusk, that will cover both the frontend and the backend in one test!

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

No branches or pull requests