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
clear EventDispatcher cache in tests #17620
Conversation
@@ -92,6 +93,7 @@ public function setUp(): void | |||
|
|||
PiwikCache::getEagerCache()->flushAll(); | |||
PiwikCache::getTransientCache()->flushAll(); | |||
EventDispatcher::getInstance()->clearCache(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put this in Fixture::clearInMemoryCaches()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see we call that in tearDown only for some reason... seems like a mistake, and we should clear all of those caches here too...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@diosmosis updated the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@diosmosis seems clearing all caches at this point lets various integration tests start failing. Seems clearing all static instances is a problem at that point as some fixtures are manipulating those before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted it so it only adds the additional cache clear, but left a note so it's clear why not all caches are cleared
4481fe3
to
6b7c677
Compare
Description:
Was writing some tests that require the ActivityLog plugin but failed to get them working. The reason actually was, that the ActivityLog plugin dynamically tries to register hooks. To get that working in tests it's required to clear the registered hooks that might have been cached previously.
Review