[FrameworkBundle] Simplify testing of protected resources #28065
Conversation
geoffrey-brier
force-pushed
the
simplify-testing-protected-resources
branch
from
07bb9c9 to
9c21266
Compare
geoffrey-brier
force-pushed
the
simplify-testing-protected-resources
branch
from
9c21266 to
7e0dce0
Compare
geoffrey-brier
force-pushed
the
simplify-testing-protected-resources
branch
from
7e0dce0 to
47d95ab
Compare
| $context = $firewallContext ?? $firewallName; | ||
| $session = $this->getContainer()->get('session'); | ||
|
|
||
| $token = new UsernamePasswordToken($user, $credentials, $firewallName, $roles); |
There was a problem hiding this comment.
I think creating the token should be put in a private method so we can override it, in case one needs to check different types of authentication modes that create different kinds of tokens.
There was a problem hiding this comment.
you do realize that a private method cannot be overridden, right ?
There was a problem hiding this comment.
Sorry, I meant protected, I should be a bit tired 😅
There was a problem hiding this comment.
but then, it should not be in the Client class either (which is not as easy to replace than a method in the TestCase).
And I'm not sure we really need to make it extendable, as everything it does here can be done from outside the client if you have different need.
There was a problem hiding this comment.
@Pierstoval I believe that if people need a more advanced way to login, they should not rely on this method. IMO, that's a reasonable limitation considering that it will ease most "login" use cases. WDYT ?
|
That's a method I've been using for years in my custom TestCase, but if it's present in the framework directly, I think it should be more customizable to cover more use cases 🤔 |
geoffrey-brier
force-pushed
the
simplify-testing-protected-resources
branch
from
47d95ab to
d81cdd4
Compare
geoffrey-brier
force-pushed
the
simplify-testing-protected-resources
branch
from
d81cdd4 to
8a41561
Compare
geoffrey-brier
force-pushed
the
simplify-testing-protected-resources
branch
from
8a41561 to
9ca23a4
Compare
|
@geoffrey-brier could you rebase ? @stof what is missing ? |
|
About this... After the insertion of the new WebTestCase assertions, I'm starting to think that this method should be in the The login process is dependent on the app and should be customizable (token-based login, no In my projects I'm using something like this: https://github.com/Pierstoval/PHPUnitWebTestCase, and it allows me to use WDYT? |
|
@Pierstoval would you like to take over this PR? |
|
If @geoffrey-brier is okay for me to take the PR, I'll be glad to attempt to finish it :) |
|
Sure, go ahead ;) |
|
@nicolas-grekas I started working on it, rebased everything, moved the methods to the Should I make a new PR? |
|
Sure, please open another PR. |
|
Continued in #35644 |
10 participants
I added two new shortcut methods to simplify testing of protected resources.
->login($user, $roles = array(), $firewallName = 'main')The
loginmethod helps to login users and has 1 mandatory parameters $user which can either be anobject or a string. There are also 2 optionnal parameters in case you'd like to use a custom firewall or add extra roles. If the user implements UserInterface we'll merge the roles.
->logout()The
logoutmethod simply invalidates the session