-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Session, 5.4.0-RC] Can't use session before an HTTP request is made in functional tests #44253
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
Comments
Hello, thank you for your report and all the details on how to reproduce the issue. I found out that https://github.com/symfony/symfony/pull/41390/files#diff-2e73c153ff851124cb2a93c1dde4e21be01feeb234ad37a4fdfe4a537726f6dbR72 broke your case and indeed causes a BC break. The session you use in the test is not considered as started because of symfony/src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php Line 118 in 4e7e429
At the moment, I don't know what the correct fix is however. |
After having thought about it, it should technically be fine to have such behavior, especially in Symfony 6 (because BC are fine in majors). After all, Session is supposed to be a Request-based object, and IIRC there's ongoing work to detach the Session from the container (just like the Request was removed from the container in Symfony 2.4). Updating the Session before making an HTTP request shouldn't technically happen, because it doesn't make sense. If at some point this break is fixed in 5.4, I'd suggest to keep it for 6.0, for consistency with how Session should be handled (maybe this is another issue then) |
Could be related to: #44553 @Pierstoval can you give a try: The reproducer seems to require a heavy load of docker build. I could not get it to work that it could connect to the database. There are currently also some other fixes about sessions which could fixes the problem:
but think #44634 would from my point of view make the most sense that is effect. |
I have run into the same problem, wanting to run a functional test for controller behavior that's dependent on session state. As a workaround, I have created a trait for functional tests that allows to inject a session with the right values into the request: trait PrepareSessionValuesTrait
{
public function prepareSessionValues( array $values ): void {
static::getContainer()->get( 'event_dispatcher' )->addListener(
KernelEvents::REQUEST,
function ( RequestEvent $event ) use ( $values ) {
/** @var Session $session */
$session = static::getContainer()->get( 'session.factory' )->createSession();
foreach( $values as $k => $v ) {
$session->set( $k, $v );
}
$event->getRequest()->setSession( $session );
} );
}
} A test would look like this: $client = self::createClient();
$this->prepareSessionValues( ['my_value' => 'switcheroo' ] );
$client->request( /* request here */ ); For me, this workaround is perfect and I could contribute a PR that adds the trait (and some documentation) if you like. I'd be also interested in criticism and improvements to my approach. |
Remove all instances where we access the session through the container, replace with access through the request object. For E2E tests, use the PrepareSessionValuesTrait to get around symfony/symfony#44253 Remove session key for donor address change - that was needed for the Twig-based skins that rendered HTML, now the data has moved to the client side. Adapt our custom AccessDeniedException to have default empty string message.
Remove all instances where we access the session through the container, replace with access through the request object. For E2E tests, use the PrepareSessionValuesTrait to get around symfony/symfony#44253 Remove session key for donor address change - that was needed for the Twig-based skins that rendered HTML, now the data has moved to the client side. Adapt our custom AccessDeniedException to have default empty string message.
Remove all instances where we access the session through the container, replace with access through the request object. For E2E tests, use the PrepareSessionValuesTrait to get around symfony/symfony#44253 Remove session key for donor address change - that was needed for the Twig-based skins that rendered HTML, now the data has moved to the client side. Adapt our custom AccessDeniedException to have default empty string message.
Remove all instances where we access the session through the container, replace with access through the request object. For E2E tests, use the PrepareSessionValuesTrait to get around symfony/symfony#44253 Remove session key for donor address change - that was needed for the Twig-based skins that rendered HTML, now the data has moved to the client side. Adapt our custom AccessDeniedException to have default empty string message.
Remove all instances where we access the session through the container, replace with access through the request object. For E2E tests, use the PrepareSessionValuesTrait to get around symfony/symfony#44253 Remove session key for donor address change - that was needed for the Twig-based skins that rendered HTML, now the data has moved to the client side. Adapt our custom AccessDeniedException to have default empty string message.
Does this issue still appear or can it close? |
Hey, thanks for your report! |
Symfony version(s) affected
5.4.0-RC1
Description
It seems that there were some changes in the Session that change the initialization process in the KernelBrowser or other related services. Not sure if this is a BC break, a bug, or anything else, so reporting it anyway just in case.
How to reproduce
$session->get('some_key')
should suffice.WebTestCase
$client = self::createClient()
$client->getContainer()->get('session')->set('some_key');
$client->getContainer()->get('session')->save()
$client->request(...)
on the route that's defined by the aforementioned Controllerdump()
statements, you'll notice that the value ofsome_key
is not set in the SessionAdditionnally, if you want to reproduce it with the project that made me discover the "issue" (not sure if it's a BC break or not), you can clone this project: https://github.com/Pierstoval/CorahnRin/blob/5.4-reproducer/tests/CorahnRin/Step/AbstractStepTest.php#L33-L35
To reproduce using the project, here are the steps:
make install
.Tests\CorahnRin\Step\AbstractStepTest
classTests\CorahnRin\Step\Step07SetbacksTest::testNoSetback
testThe test will fail because the
GeneratorController
(coming from thePierstovalCharacterManagerBundle
) checks the session to get the values from$session->get('character.corahn_rin')
, which is supposed to be already set in theTests\CorahnRin\Step\AbstractStepTest::submitAction()
method beforehand.Possible Solution
The current workaround I'm using is to make an HTTP request to any URL of the project that sets the session, but does not need to get elements from it. Basically, a GET request on most pages that check if user is logged in should do the trick.
Additional Context
The same happens if you make an HTTP request and execute
$client->restart()
, it acts just like the Client was recreated, maybe because of the lack of Cookie HTTP header.The text was updated successfully, but these errors were encountered: