symfony / panther

A browser testing and web crawling library for PHP and Symfony
MIT License
2.91k stars 213 forks source link

Add `loginUser()` helper method to Panther client #574

Closed plotbox-io closed 1 hour ago

plotbox-io commented 1 year ago

Please see here for discussion context

When doing E2E testing, it is common to need to login a user to perform certain actions. We often don't care about the login process for 99% of our tests, but we must still perform the steps each time. This can add unneccesary time and complexity onto the tests that could be avoided if only we had a way to simulate logins.

It would be nice to have a helper function named loginUser() similar to the Application Test client. However, as we are doing E2E testing, we know that there is an added complexity of the environment settings of the alternate webserver (specifically how it handles session data). So, to solve this, we can pass SessionStorageInterface and firewallContext as arguments (but with sensisble defaults so they could be omitted in most cases).

Example of calling code would be:

$panther->logInUser(
    $user,
    new MockFileSessionStorage('/app/var/cache/panther/sessions'),
    'main'
);

Or simple (for most cases)

$panther->logInUser($user);

Have tested this approach (in userland code) and is working well for me. Was suggested in the discussion that I raise a PR for integration to Panther library code so here it is for your consideration. Please let me know if you need anything more from me.

Richard.

shadydealer commented 1 year ago

@plotbox-io Some apps would require a token other than a UsernamePasswordToken, could you make it an optional parameter instead? The symfony KernelBrowser uses a TestBrowserToken which could also be an alternative, not sure whether using that would solve everybody's issues.

Also wondering if this PR is even being considered?

Bilge commented 1 year ago

The sheer number of assumptions baked into this is truly staggering.

arderyp commented 1 year ago

I'm interested in other approaches if people have clever ideas. Auth is an extremely common use case, so some sort of panther support for it would be cool and would likely boost panther usage across the board

dunglas commented 1 year ago

HTTP Basic?

shadydealer commented 1 year ago

We use form login, accross all of our apps, so I don't think HTTP Basic would be of much use to us, unfortunately.

I'm open to ideas on how to reduce the number of assumptions and how to decouple it from the Symfony framework. If you look at the discussion in https://github.com/symfony/panther/issues/283 you'll see that the code was directly taken from the symfony testing framework, with a little modification, because, the perfect situation for me would be to have to only change WebTestCase to PantherTestCase and then use static::createPantherClient() where needed, making the transition from a WebTestCase to a PantherTestCase as seemless as possible.

Maybe the loginUser method could detect whether the project is using the Symfony framework or not, and behave differently depending on the result? Like, HTTP Basic for projects which aren't using Symfony and something along the lines of the code in the PR? Or bake in the HTTP Basic login in the PantherClient and provide a separate package, dedicated to Symfony projects, which overrides the loginUser method?

dunglas commented 1 year ago

You could enable HTTP Basic only for the panther env. Or (or in addition), we could provide a helper to submit the standard Symfony login form using HTTP?

arderyp commented 1 year ago

I'm also not keen on basic auth. My actual site doesn't use it, and I want my tests to behave as closely to my live site as possible, including running through my real auth mechanism.

I think allowing basic auth is fine, letting the implementer decide. But forcing it doesn't seem appealing.

dunglas commented 1 year ago

@arderyp that's the plan. You can already plug your custom auth logic if needed.

antoniovj1 commented 4 days ago

@plotbox-io Do you need some help with this?

plotbox-io commented 1 hour ago

@antoniovj1 Sorry, we are no longer using Panther after dropping our experimental API work that was previously using this. Happy that it at least seemed to spark a few ideas within the panther team. Kind regards, Richard