E2E Test Utils: Add getCurrentUser(), and use it for user switching #33050
Conversation
This file contains 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
|
Size Change: -1.08 kB (0%) Total Size: 1.05 MB
ℹ️ View Unchanged
|
youknowriad
approved these changes
Jun 29, 2021
Yeah, agree that it's a bit weird. I was looking for some way that WP exposed the currently logged-in user already (on the client side), and this was the best I could find. I was considering some alternatives (see PR desc) but ultimately thought that the cookie-based approach was more performant, and less dependent on a separate test plugin (which would've had other drawbacks). |
|
e2e failures are unrelated and being investigated. |
|
Nice one, thank you for working on it 👍🏻 |
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.
Description
Spun off from #32868.
A lot of our E2E test utils use
switchUserToAdmin()andswitchUserToUser(). Typically,switchUserToAdmin()is used at the beginning to be able to change a site-wide setting in wp-admin; andswitchUserToUser()is used at the end to switch back to 'normal' test privileges.The credentials used for
switchUserToAdmin()are statically stored here:gutenberg/packages/e2e-test-utils/src/shared/config.js
Lines 1 to 4 in 37d6024
The credentials for
switchUserToUser()are informed by theWP_USERNAMEandWP_PASSWORDenv vars, which in turn can be set through the--wordpress-usernameand--wordpress-passwordargs for the e2e test command, as documented here.If those args/env vars aren't set, we fall back to the admin credentials:
gutenberg/packages/e2e-test-utils/src/shared/config.js
Lines 6 to 10 in 37d6024
Due to the implementation of
switchUserToAdmin()andswitchUserToUser(), this means that in the default case (with no custom username/password combination set), both functions are pretty much no-ops, as they both bail early in that case:gutenberg/packages/e2e-test-utils/src/switch-user-to-admin.js
Lines 11 to 16 in 37d6024
gutenberg/packages/e2e-test-utils/src/switch-user-to-test.js
Lines 11 to 16 in 37d6024
This means that those functions cannot scale to a scenario where we want to use a different user programmatically, i.e. as part of an e2e test, rather than statically setting a different user for all tests. One example for such a scenario would be #32868 -- or really any e2e test where we'd like to use a user with lesser-than-admin privileges.
For this reason, I'd like to change the checks in
switchUserToAdmin()(and, for symmetry reasons, inswitchUserToUser()), to compare the currently logged-in user to the admin or custom user, respectively.(This means that for a scenario like #32868, we still won't be able to use
switchUserToUser(), as that just falls back to the custom user set via e2e tests args or env vars; but it's easy enough to useloginUser()directly).Note that
switchUserToAdmin()andswitchUserToUser()are present in a lot of our e2e test utils, so performance is crucial: we want to continue to bail early if the admin is already logged in, and we want to do it quickly.Alternatives considered
Evaluate
.display-name(in the masterbar, top right)Downside: Returns the display name, not the username
Add a test plugin that injects the username as a global JS var
Downside: Requires a plugin to be active, which can present a bit of a catch-22: e2e test setup needs to log in as admin (via
switchUserToAdmin()) -- butswitchUserToAdmin()determines whether to log in based on the global var provided by the plugin.How has this been tested?
Run the e2e tests locally (
npm run test-e2e), or observe them in CI on this PR.