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
Fix menu in long-running process (RoadRunner, ReactPHP, Amphp) #7885
Changes from 6 commits
6a1c337
af31e54
6dbaa2b
4e55191
9ad44a1
bfe0054
37a1a39
3f152c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
/* | ||
* This file is part of the Sonata Project package. | ||
* | ||
* (c) Thomas Rabaix <thomas.rabaix@sonata-project.org> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Sonata\AdminBundle\Tests\App\EventListener; | ||
|
||
use Sonata\AdminBundle\Event\ConfigureMenuEvent; | ||
use Symfony\Component\EventDispatcher\EventSubscriberInterface; | ||
|
||
final class ConfigureMenu implements EventSubscriberInterface | ||
{ | ||
private int $counter = 0; | ||
|
||
public static function getSubscribedEvents(): array | ||
{ | ||
return [ | ||
ConfigureMenuEvent::SIDEBAR => 'configureMenu', | ||
]; | ||
} | ||
|
||
public function configureMenu(ConfigureMenuEvent $configureMenuEvent): void | ||
{ | ||
$configureMenuEvent->getMenu()->addChild(sprintf('Dynamic Menu %s', ++$this->counter))->setAttribute('class', 'dynamic-menu'); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
/* | ||
* This file is part of the Sonata Project package. | ||
* | ||
* (c) Thomas Rabaix <thomas.rabaix@sonata-project.org> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Sonata\AdminBundle\Tests\Functional\Controller; | ||
|
||
use Sonata\AdminBundle\Tests\App\AppKernel; | ||
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\Response; | ||
|
||
final class MenuTest extends WebTestCase | ||
{ | ||
public function testDynamicMenuInLongRunningProcess(): void | ||
{ | ||
$client = static::createClient(); | ||
$client->disableReboot(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is what makes the client share the same container? If yes, can you add a comment here to explain this line (ex: this ensures the container is reused across multiple request to simulate the behavior of ...)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure, adding |
||
|
||
for ($i = 1; $i < 5; ++$i) { | ||
$client->request(Request::METHOD_GET, '/admin/dashboard'); | ||
|
||
static::assertSame(Response::HTTP_OK, $client->getResponse()->getStatusCode()); | ||
|
||
$crawler = $client->getCrawler(); | ||
|
||
$menu = $crawler->filter('.sidebar-menu .dynamic-menu a'); | ||
|
||
static::assertCount(1, $menu); | ||
static::assertSame(sprintf('Dynamic Menu %s', $i), trim($menu->text())); | ||
} | ||
} | ||
|
||
protected static function getKernelClass(): string | ||
{ | ||
return AppKernel::class; | ||
} | ||
} |
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.
if we cant add a test, we should at least leave a comment about why we do this, imho
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.
Sure. Adding it atm.
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.
Added. :)