spinen / laravel-mail-assertions

PHPUnit mail assertions for testing email in Laravel
https://spinen.com
32 stars 18 forks source link

Laravel 5.5 Issue #36

Closed pdbreen closed 6 years ago

pdbreen commented 7 years ago

PhpUnit 6 changed the order for @before vs setUp invocations. As a result, there is no app / no facades available when setUpMailTracking is invoked. From this thread - https://github.com/tightenco/mailthief/pull/77 - it looks like you could use the same afterApplicationCreated solution:

/** @before */
public function setUpMailTracking()
{
    $this->afterApplicationCreated(function() {
        Mail::getSwiftMailer()
            ->registerPlugin(new MailRecorder($this));
    });
}

I'm not using your package at the moment (local variation), otherwise I'd test to confirm and send you the PR.

jimmypuckett commented 7 years ago

@pdbreen Thanks for the heads up. We will look into this.

/cc @ssfinney

ssfinney commented 7 years ago

Hi @pdbreen! Thanks for taking the time to let us know about this.

@jimmypuckett In order to use that afterApplicationCreated solution, I think we need a new tagged version of this package just for Laravel 5.5+. Or I suppose we could try to detect the version.

Thoughts?

jimmypuckett commented 7 years ago

@ssfinney we could tag a version or maybe do some magic to determine which version is being used? Either way, with the Mail::fake stuff in L5.5, I am not 100% sure that this package adds value?

Penthious commented 7 years ago

For those that need this fix for laravel 5.5 put this in your composer.json, all my tests are passing again

"require": {
       "spinen/laravel-mail-assertions": "dev-develop"
},
"repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/chamav/laravel-mail-assertions"
        }
    ]
jimmypuckett commented 7 years ago

Seems like someone could just make a quick PR?

gemal commented 6 years ago

any update on this. What is missing for this one to be fixed?

gemal commented 6 years ago

What is missing here for a merge?

pablorsk commented 6 years ago

Temporal fix for packages with stable minimum-stability.

    "require-dev": {
        "pablorsk/laravel5-mail-assertions": "0.3.2"
    },
ssfinney commented 6 years ago

Hey everyone, thanks for your patience with this. 0.3.2 has just been released with changes to resolve this.

gemal commented 6 years ago

TypeError: Argument 1 passed to Spinen\MailAssertions\MailRecorder::__construct() must be an instance of PHPUnit_Framework_TestCase, instance of Tests\Feature\MultiLanguageEmailTest given, called in /vendor/spinen/laravel-mail-assertions/src/MailTracking.php on line 53

test looks pretty standard:

        $this->seeEmailWasSent()
            ->seeEmailTo($email)
            ->seeEmailSubject($subject)

any clue ?

pablorsk commented 6 years ago

@gemal, if you check my fork you can see the fix:

https://github.com/spinen/laravel-mail-assertions/compare/develop...pablorsk:develop#diff-6722694af772ec10f77c6620a4d37558

If you are using this library, please, wait for 0.3.2 release.

gemal commented 6 years ago

I use v0.3.2. I just updated :(

pablorsk commented 6 years ago

We are using on multinexo.com my fork:

"pablorsk/laravel5-mail-assertions": "0.3.2"

while the problem is solved in this library.

ssfinney commented 6 years ago

Thanks everyone! Re-opening. #42 should resolve @gemal's issue.

ssfinney commented 6 years ago

Guys, we've spent some more time on this and tried to nail down these compatibility issues.

Version 0.3.3 has been released! Bottom line: PHPUnit 4 - 7 is now supported, along with Laravel 5.1 - 5.6, and PHP is still supported from 5.5 through 7.1.

Thanks for hanging in there with us!