feat: inject the http session into use() - #213
Open
Amoifr wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #149
This is the API you sketched in the issue:
It is on
KernelBrowseronly, not onBrowser, since it needs the app's container and the test process's cookie jar. Panther and Playwright drive a real server, so the same trick does not apply there.The one change outside
KernelBrowseris a hook:use()now calls a no-opprotected afterUse()after the callback. It is needed because the session has to be saved after the callback has mutated it, andCallback::invokeAll()gives no point to do that. The alternative was to hand out a save-through decorator, which I dropped: it would not be a realSessionany more, sogetFlashBag()andinstanceof Sessionwould break. Handing out the real object seemed worth the five line seam, but say the word if you would rather have it another way.Things I measured rather than assumed:
CookieJar::allValues()skips the domain check entirely when the cookie has none (if ($domain)), so it is sent whatever host is visited. That avoids guessing a host, which cannot be done anyway before the first request:getCurrentUrl()throwsUnable to access the request before visiting a pageon a fresh browser. A test covers a non-localhosthost.using_the_session_keeps_the_existing_session_cookieasserts the id is the same before and after.set()after asave()works and persists, which is what happens when a test uses the session in two separate->use()calls.The round trip goes both ways in the tests: the app reads what the test wrote (new
/read-sessionroute on the test kernel), and the test reads what the app wrote (the existing/page1?start-session=1).When sessions are not enabled,
httpSession()throwsSessions are not available/enabled., matching the wording of the existingsecurityToken()guard.280 tests green, PHPStan clean. I could not run php-cs-fixer locally, it is not in the dev deps, so the bot may still have something to fix.