8000 [Security][Testing] Documented KernelBrowser::loginUser() by wouterj · Pull Request #13362 · symfony/symfony-docs · GitHub
[go: up one dir, main page]
Skip to content

[Security][Testing] Documented KernelBrowser::loginUser() #13362

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 23, 2020
Merged

[Security][Testing] Documented KernelBrowser::loginUser() #13362

merged 1 commit into from
Mar 23, 2020

Conversation

wouterj
Copy link
Member
@wouterj wouterj commented Mar 16, 2020

Fixes #13329

I think this is common enough to be put in the main article :)

javiereguiluz
javiereguiluz reviewed Mar 17, 2020
View reviewed changes
Copy link
Member
@javiereguiluz javiereguiluz left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution. I like this ... but I'm missing some details about what can I pass to the loginUser() method. We may consider detailing that it must be a UserInterface object.

@wouterj wouterj force-pushed the issue-13329/login-user-test branch from dae2ff7 to e4ad22b Compare March 17, 2020 16:08
@wouterj
Copy link
Member Author
wouterj commented Mar 17, 2020

Thanks Javier. I've updated the PR following your suggestions

@wouterj wouterj force-pushed the issue-13329/login-user-test branch from e4ad22b to 4c169b4 Compare March 17, 2020 16:10
@wouterj wouterj mentioned this pull request Mar 17, 2020
Closed
@wouterj
Copy link
Member Author
wouterj commented Mar 17, 2020

Note: I've put this on hold, let's first wait if symfony/symfony#36112 gets merged

@wouterj wouterj added On hold and removed On hold labels Mar 17, 2020
@wouterj
Copy link
Member Author
wouterj commented Mar 19, 2020

PR closed, this ready to be merged imho

@wouterj wouterj force-pushed the issue-13329/login-user-test branch from 4c169b4 to 2c6d6d4 Compare March 21, 2020 11:23
@javiereguiluz javiereguiluz merged commit 8649d07 into symfony:master Mar 23, 2020
@javiereguiluz
Copy link
Member

Wouter, while merging this, I decided to not delete the testing/http_authentication.rst.

This is an important article and many people rely on it. Redirecting to the main testing.rst article feels "wrong" because we can't redirect them to the exact section explaining the loginUser() method. That's why I kept the article, but added this CAUTION message at the beginning:

.. caution::

    Starting from Symfony 5.1, the methods explained in this article are no
    longer recommended to logging in users in your tests. Instead, use
    :ref:`the loginUser() method <testing_logging_in_users>`.

In some time, when most people are aware of this, we can remove the article and redirect. Thanks!

@wouterj wouterj deleted the issue-13329/login-user-test branch March 23, 2020 13:21
@wouterj
Copy link
Member Author
wouterj commented Mar 23, 2020

@javiereguiluz the redirection_map is parsed in a Symfony controller that catches all 404 pages right? Can't we add section-redirection, by doing e.g. testing/http_authentication testing#logging-in-users?

@javiereguiluz
Copy link
Member
8870

Yes, I think we could do that in the future ... not sure if it would work today. I'd need to double check it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@javiereguiluz javiereguiluz javiereguiluz left review comments

Assignees
No one assigned
Labels
Security Status: Needs Review Testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[DX][Testing] Added a loginUser() method to test protected resources
3 participants
@wouterj @javiereguiluz @carsonbot
0