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

[6.x] Replace Event Dispatcher in resolved cache repositories when `Event::fake()` is used #31119

Merged

Conversation

@colindecarlo
Copy link
Contributor

colindecarlo commented Jan 14, 2020

The fake and fakeFor method of the Event facade currently fails to replace the existing Event Dispatcher with the faked dispatcher in resolved cache repositories.

I've found that making assertions on cache events is a pretty useful way of testing code that uses the cache under the hood. For instance:

class Calculator {

    public function add($operand1, $operand2)
    {
        $cacheKey = sprintf("%s+%s", $operand1, $operand2);

        if (Cache::has($cacheKey)) {
            return Cache::get($cacheKey);
        }

        $result = $operand1 + $operand2;

        Cache::put($cacheKey, $result);

        return $result;
    }
}

class CalculatorTest {

    public function test_it_returns_calculations_from_cache()
    {
        Cache::put('1+1', 3);
        Event::fake();

        $calculator = new Calculator;

        $this->assertEquals(3, $calculator->add(1, 1));

        Event::assertDispatched(CacheHit::class, function ($hit) {
            return $hit->key == '1+1' && $hit->value == 3;
        });
    }
}

This PR adds a public method to the CacheManager used to refresh each cache repositories to which it holds a reference to with the event dispatcher that is currently bound in the service container.

The method is called in the Event::fake and Event::fakeFor methods.

A getEventDispatcher method was added to the cache Repository class to make testing a little easier.

@colindecarlo colindecarlo changed the title Replace Event Dispatcher in resolved repositories when `Event::fake()` is used Replace Event Dispatcher in resolved cache repositories when `Event::fake()` is used Jan 14, 2020
@GrahamCampbell GrahamCampbell changed the title Replace Event Dispatcher in resolved cache repositories when `Event::fake()` is used [6.x] Replace Event Dispatcher in resolved cache repositories when `Event::fake()` is used Jan 14, 2020
@taylorotwell

This comment has been minimized.

Copy link
Member

taylorotwell commented Jan 15, 2020

So does it not work already if you call Event::fake() first instead of after Cache::put?

@colindecarlo

This comment has been minimized.

Copy link
Contributor Author

colindecarlo commented Jan 16, 2020

In some cases yes, all that is required is to put the call to Event::fake before accessing the cache. However, after some debugging, it looks like if you include the RefreshDatabase trait then bootstrapping the app includes the ArtisanServiceProvider which makes all the commands, some of which require the cache (for instance RestartQueueCommand). In these cases the cache is created during the test setup so it gets the real event dispatcher.

@taylorotwell taylorotwell merged commit 381ee1f into laravel:6.x Jan 16, 2020
2 checks passed
2 checks passed
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.