laravel / browser-kit-testing

Provides backwards compatibility for BrowserKit testing in the latest Laravel release.
MIT License
509 stars 75 forks source link

Outdated MocksApplicationServices.php implementation #142

Closed bytestream closed 4 years ago

bytestream commented 4 years ago

Description:

Comparison of:

withoutEvents() diff:

$mock = Mockery::mock('Illuminate\Contracts\Events\Dispatcher');

$mock->shouldReceive('fire', 'dispatch', 'getCommandHandler')->andReturnUsing(function ($called) {
    $this->firedEvents[] = $called;
});

$this->app->instance('events', $mock);

return $this;
$mock = Mockery::mock(EventsDispatcherContract::class)->shouldIgnoreMissing();

$mock->shouldReceive('dispatch', 'until')->andReturnUsing(function ($called) {
    $this->firedEvents[] = $called;
});

Event::clearResolvedInstances();

$this->app->instance('events', $mock);

return $this;

I noticed this having tried to use expectsEvents() and ran into a mockery error stating until was called but had not been mocked.

There are other differences https://www.diffchecker.com/HRM1BixO

Steps To Reproduce:

$this->expectsEvents([Foo::class]);
driesvints commented 4 years ago

We don't maintain this version anymore. Can you please upgrade to 6.x and try again? Feel free to open a new issue if it persists.

bytestream commented 4 years ago

You can see the same issue exists on the 6.x branch which I accidently linked in my original message:

Unfortunately 6.x requires Laravel 7 so I cannot upgrade.


Given Laravel 6 is LTS until Sept 2021 (https://laravel.com/docs/6.x/releases) I would think this package should also follow that schedule.

driesvints commented 4 years ago

@bytestream only the frameworks follows LTS, not its first party libraries. As per our release policy: https://laravel.com/docs/7.x/releases#support-policy

For all additional libraries, including Lumen, only the latest release receives bug fixes.

driesvints commented 4 years ago

Tbh, I don't immediately understand the problem here. If you can give specific steps to reproduce (also show what Foo is) then I'll try to reproduce it.

bytestream commented 4 years ago

@bytestream only the frameworks follows LTS, not its first party libraries. As per our release policy: https://laravel.com/docs/7.x/releases#support-policy

For all additional libraries, including Lumen, only the latest release receives bug fixes.

Ah thanks for the link. I was not aware of that.

Tbh, I don't immediately understand the problem here. If you can give specific steps to reproduce (also show what Foo is) then I'll try to reproduce it.

Should be replicable with the below:

public function testExpectsEvents()
{
    $this->expectsEvents([
        MessageSending::class,
        MessageSent::class,
    ]);

    app('mailer')->raw('foo', function ($message) {
        //
    });
}

/*
Mockery\Exception\BadMethodCallException : Received Mockery_0_Illuminate_Contracts_Events_Dispatcher::until(), but no expectations were specified
/vendor/laravel/framework/src/Illuminate/Mail/Mailer.php:508
/vendor/laravel/framework/src/Illuminate/Mail/Mailer.php:260
/vendor/laravel/framework/src/Illuminate/Mail/Mailer.php:185
*/
bytestream commented 4 years ago

The point I was making in my first message is that if you compare laravel/framework withoutEvents() with this packages withoutEvents() and play a game of spot the difference then you'll see issues:

Apologises if I was not clear earlier.

If shouldReceive is updated to match laravel/framework then I suspect the error in the above test case would be resolved.

bytestream commented 4 years ago

I am sure there are other outdated function implementations if you compare laravel/framework to this package.

driesvints commented 4 years ago

@bytestream I think at this point it's best that you attempt a PR to the 6.x branch.