-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add dedicated e2e login tests #17014
Add dedicated e2e login tests #17014
Conversation
@cedric-anne Regarding the two new behaviors that are only enabled while in The idea is to cover both |
bf73997
to
7ea908f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not like the idea of adding specific endpoints for testing. It means more code to maintain, potential security issues on them, ...
This seems the simplest strategy among those demonstrated in the link above. The alternative is to parse the login page to get the token, which IMO would generate more code to maintain compared to a simple endpoint with 3 lines of code. edit: also, the file could be placed in a special folder that is deleted when building the production archive. |
Agree. Plus, I don't think there is much reason to worry about having a fresh login for every test. If you are testing a session-reliant feature like user preferences, just make sure to clear the session in the beforeEach or before hook in that specific spec before calling the login command. |
7ea908f
to
b5fcb5c
Compare
I'm still not in agreement with some of the opinions of the Cypress team and some of their older opinions seem in contrast to some of the newer features. For example, a few years ago they were saying not to use login sessions between tests but then they added support for that later. Don't use sessions. Just fake it (2018): Use sessions to authenticate faster (2021): I'd also like to see how this compares when tests are grouped into spec files a little differently. For example, the form spec files all have 1 or 2 tests in them. If they were combined into a single spec file and the login was handled as it was before (actual login with sessions), would performance be equally as good? I'm not familiar enough with the forms feature to know how the session affects usability/test isolation. Maybe forms could be tested in different specs depending on which interface (simplified/standard) is used if it matters. Also, in the real world you aren't going to have a clean session every time a user performs an action. The session shouldn't be manipulated in any way directly by tests, so having a non-clean session may not be an issue. If an issue occurs, we are probably using the session to store data when we shouldn't be (like the parameters for an LDAP import). |
I think the main issue here is that Grouping more tests into the same specs just to benefit from session sharing doesn't seem ideal to me as it hurt readability of those tests. However, there is an option in the This would be a good middle ground to simplify this issue. |
Why do we need a dedicated login test ?
Our first implementation of e2e tests uses GLPI UI to login before a given test.
As per cypress official recommandations, this should be avoided: https://docs.cypress.io/guides/references/best-practices#Organizing-Tests-Logging-In-Controlling-State.
More details can be found in the cypress presentation from the assertjs 2018 conference: https://www.youtube.com/watch?v=5XQOK0v_YRE
To sum it up, the login process should be validated from the UI once in a dedicated test.
Then, all others tests should programmatically log into the application using some kind of direct HTTP requests.
Thus, I have moved the current login implementation into a dedicated test and added a new programmatic login command that can be used for all others tests.
Programmatic login
To implements programmatic login, we have two barriers:
Randomized inputs
To fix the randomized names issue, I've added some changes to the login front file.
It will now use predetermined names (username, password, remember_me) if it receives a request from a
testing
environnements that doesn't come from the UI.CSRF
For the CSRF tokens, cypress give some example of the possibles strategies here: https://github.com/cypress-io/cypress-example-recipes/blob/master/examples/logging-in__csrf-tokens/cypress/e2e/logging-in-csrf-tokens-spec.cy.js
I've chosen the 3rd strategy "expose CSRF via a route when not in production", which should bring the most performances while being easy to maintain.
The CSRF token will thus be retrieved by doing a
GET
request onfront/csrf.php
, a special page that does not return anything if the environnements is nottesting
.