Problem/Motivation
A test that creates a user and logs in will issue 3 HTTP requests:
- GET to the login form
- POST to submit the login form
- GET to the user profile page
$this->drupalLogin is called thousands of times. This leads to a significant number of HTTP requests.
Instead, we can use the one-time login link, and reduce this to 2 HTTP requests per login:
- GET to the one-time login path
- GET to the user profile page
Steps to reproduce
Proposed resolution
- Implement #3469202: Add LoginTrait in core.
- Ensure we have separate coverage for the user login form.
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3469309
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
mstrelan commentedLocally I have
\Drupal\Tests\block_content\Functional\BlockContentListTestpassing but it looks like there are lots of fails in other tests.Comment #4
mstrelan commentedWell that was fun. There are three tests remaining that use the login form. One is the UserLoginTest, so we need to keep that. The other two seem to be oddities that could probably be fixed, perhaps in a followup.
It's hard to evaluate if this is overall faster than before because there is such variability in the test suite times. I did see one run that was under five minutes. In my local testing I found a saving of around 200ms per login. So that's roughly 5 mins total savings over 1500 logins.
Comment #5
smustgrave commentedFollow ups appear to be mentioned in the threads.
Comment #6
mstrelan commented::userPassResetUrlmethod and updated the existinguser_pass_reset_urlfunction to use current time as discussed with @catch in Slack.UserLoginTest::testLoginCacheTagsAndDestinationto explicitly submit the login form instead of relying on::drupalLoginComment #7
smustgrave commentedSo have re-ran twice and keep getting failures, but not entirely sure why.
Comment #8
mstrelan commentedHEAD was broken. Have rebased and it's green again.
Comment #9
smustgrave commentedNice. Believe feedback has been addressed. The one thread wasn't sure about
Comment #10
moshe weitzman commentedLGTM.
Comment #16
nod_Committed and pushed bf4ae81164 to 11.x and 39f2b9f40b to 11.0.x and b16714a919 to 10.4.x and 332f671c2d to 10.3.x. Thanks!
Comment #17
nod_LocaleTranslationUiTest fails on 11.0.x, 10.4.x, and 10.3.x
Comment #21
mstrelan commentedWe can probably add
to the failing test.
Comment #23
mstrelan commentedAdded MR for 11.0.x
Comment #24
smustgrave commented11.0 appears to be passing
Was confused for a second why 11.0 but see that 11.x wasn't reverted
Comment #28
nod_Committed and pushed 104f82857d to 11.0.x and b868899be1 to 10.4.x and 47614c4759 to 10.3.x. Thanks!
Comment #30
daffie commentedLocaleTranslationUiTest fails on 11.x with PostgreSQL!
Setting the priority to critical.
Comment #31
mstrelan commentedI won't get to this for a bit, but see #17 and 21 and apply this to 11.x, it was already applied to the other branches.
Comment #34
catchI've cherry-picked
104f8285forwards to 11.x, tentatively marking fixed but please re-open if there's still a problem.Comment #35
joachim commented