"Calling many times CacheManager::getInstance()" during tests with dataProvider
r0b- opened this issue · 8 comments
Hi,
my controller test cases fail if they have a dataProvider with more than 4 items (using Symfony's WebTestCase).
Error is:
app.ERROR: [Files] Calling many times CacheManager::getInstance() for already instanced drivers is a bad practice and have a significant impact on performances....
The cache instance getter is called through smyfony's DI when the controller get's instantiated.
So is it possible to disable the $badPracticeOmeter somehow or increase the limit or...?
Also I tried to call CacheManager::clearInstances on test teardown but it seems to be buggy (the instances array itself is not cleared so the $badPracticeOmeter counter is always incremented) :-(
Thanks
Rob
Hi Rob,
You you show me your code implementation in private please (Ore publicly if you want too).
Thanks you.
This is the service wrapping phpfastcache:
class MyCacheService
{
/**
* @var ExtendedCacheItemPoolInterface
*/
private $cacheInstance;
public function __construct(Cache $cache)
{
$this->cacheInstance = $cache->get("my_cache");
}
...
This is the controller using the service:
public function indexAction(Request $request)
{
$myCacheService = $this->get("myproject.my_cache");
....
}
And this is the test:
class IndexControllerTest extends WebTestCase
{
/** @var Client */
private $client;
public function setUp()
{
$this->client = static::createClient(["debug" => true]);
}
public function routeProvider()
{
return [
["/a"], ["/b"], ["/c"], ["/d"], ["/e"]
];
}
/**
* @dataProvider routeProvider
*/
public function myTest($route)
{
$this->client->request("GET", $route);
$this->assertTrue($this->client->getResponse()->isOk());
}
For a reason that i ignore you call CacheManager multiple time and this is a bad practice.
Did you read this Wiki ?
https://github.com/PHPSocialNetwork/phpfastcache/wiki/%5BV5%5D-Why-calling-getInstance()-each-time-is-a-bad-practice-%3F
You must store and keep your cache instances :)
Also making a wrapper to make get/set syntax will considerably affect your application performances.
PhpFastCache is itself an abstract wrapper around drivers themselves. I strongly encourage you to use the Psr6 guideline.
hmmmm, of course I've read the wiki, but the controller lifecycle is controlled by symfony during the tests.
However, is it possible that you fix the CacheManager::clearInstances method so that it actually clears the instances array (just add a self::$instances = [];)? That would help me a lot!
Can you go inside the phpfastcache vendor and alter the ClearInstance method like this please ?
from:
foreach (self::$instances as &$instance) {
unset($instance);
}
gc_collect_cycles();
return !count(self::$instances);to:
foreach (self::$instances as $key => $instance) {
unset(self::$instances[$key]);
}
gc_collect_cycles();
return !count(self::$instances);If it work i'll push the fix asap.
Or you can Pr also if you know how to do it :)
Created a PR...
Thanks in advance for merging!
Merged, thanks you :)