The user password reset needs a test, quoting dmitrig01 'can use mail.inc to intercept the mail'.

Files: 
CommentFileSizeAuthor
#29 293487-29-password-reset-test.patch5.67 KBdcam
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 293487-29-password-reset-test.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#23 reset_tests-293487-23.patch5.71 KBZenDoodles
PASSED: [[SimpleTest]]: [MySQL] 48,853 pass(es).
[ View ]
#16 issue-293487.patch4.54 KBlilou
Unable to apply patch issue-293487.patch
[ View ]
#14 password_reset_test_4.patch4.28 KBmr.baileys
Unable to apply patch password_reset_test_4.patch
[ View ]
#12 password_reset_test_3.patch4.31 KBmr.baileys
Failed: Failed to install HEAD.
[ View ]
#4 password_reset_test_2.patch4.35 KBmr.baileys
Failed: Failed to apply patch.
[ View ]
#1 password_reset_test_1.patch4.31 KBmr.baileys
Failed: 11445 passes, 21 fails, 2 exceptions
[ View ]

Comments

Component:tests» user system
Assigned:Unassigned» mr.baileys
Status:Active» Needs work
StatusFileSize
new4.31 KB
Failed: 11445 passes, 21 fails, 2 exceptions
[ View ]

First stab at this. This patch relies on the new assertions and API introduced in #296001: Capture e-mails sent during tests and add e-mail assertions / API.

This tests:

  1. the password reset functionality by requesting a new password by username
  2. the password reset functionality by requesting a new password by e-mail
  3. requesting a password for a user that does not exist
  4. it verifies that the e-mails go out (and don't go out for #3)
  5. extracts the password link from those e-mails.
  6. "clicks" the one-time login link
  7. verifies that the user is granted access and can change his/her password.
  8. Also ensures that the one-time link works only once.

Some of the logic for this test was already present in the existing user registration test, but a work-around was used to create the one-time login link (direct call to user_pass_reset_url instead of capturing the e-mail).

Leaving at NW until #296001: Capture e-mails sent during tests and add e-mail assertions / API is commited, and because the tests sometimes fail for reasons unknown to me.

Status:Needs work» Needs review

#296001: Capture e-mails sent during tests and add e-mail assertions / API has been commited, let's take this one for a spin...

Status:Needs review» Needs work

The last submitted patch failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.35 KB
Failed: Failed to apply patch.
[ View ]

The original code to test the password reset URL had the following code:

<?php
sleep
(1); // TODO Find better way.
?>

When I leave this out, tests succeed on my local computer about 25% of the time. If I put it back in, tests consistently pass, so putting it back in here to see what the testbot says. I browsed the repository but since this was part of the initial commit, I couldn't find any further reasoning as to why it's in there.

Status:Needs review» Needs work

The last submitted patch failed testing.

Status:Needs work» Needs review

Note to reviewers: I haven't figured out why, but the test fails 1 out of 5 runs. The other 4 test runs succeed without any problem...

That sleep(1) is also confusing me.

I think there's some validation which checks that the request time for the password request is < the request time for the time it's used, and on fast systems this can cause a failure. But that's half-remembered from something else similar rather than looking at the actual code it tests.

@catch: thanks for pointing me in the right direction. The current implementation for the one-time login link compares timestamps in seconds, and uses < and >. This means it might take up to one second before the link actually becomes active.

Since I think these links should be active immediately, I've created a separate issue for this: #481794: One-time link should be active immediately. Once this lands, the sleep(1) hack can be removed.

Status:Needs review» Needs work

The last submitted patch failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.31 KB
Failed: Failed to install HEAD.
[ View ]

re-rolled.

The sleep(1) is still in patch #12.

StatusFileSize
new4.28 KB
Unable to apply patch password_reset_test_4.patch
[ View ]

Oops... sorry about that.

Re-rolled without the sleep(1)...

Status:Needs review» Needs work

+    // Verify that the user received an e-mail.

missing a period

Not changed by the patch but could add one here too.

     // Check password type validation

Status:Needs work» Needs review
StatusFileSize
new4.54 KB
Unable to apply patch issue-293487.patch
[ View ]

Add two missing periods.

Verified that sleep() is not in the patch. Tests installed and reviewed.
User tests pass with no fails - 699 passes, 0 fails, and 0 exceptions

Status:Needs review» Needs work

The last submitted patch failed testing.

Priority:Critical» Normal

Tests don't qualify as critical.

Status:Needs work» Needs review

Re-test of issue-293487.patch from comment #16 was requested by kathyh.

Status:Needs review» Needs work

The last submitted patch, issue-293487.patch, failed testing.

Patch no longer applies after #46149: Prevent account cancellation for uid 1 was commited.

Version:7.x-dev» 8.x-dev
Assigned:mr.baileys» ZenDoodles
Status:Needs work» Needs review
Issue tags:+needs backport to D7
StatusFileSize
new5.71 KB
PASSED: [[SimpleTest]]: [MySQL] 48,853 pass(es).
[ View ]

Fix in D8 first, then backport.

Patch looks good and I was able to apply it. I'll leave this issue in "needs review" for others to test.

I've:
- applied the patch
- run tests few times (php core/scripts/run-tests.sh --file core/modules/user/lib/Drupal/user/Tests/UserPasswordResetTest.php)
- review the code of the test
Everything works fine. I would apply the patch.

I applied the patch, ran the test and got this message:

An AJAX HTTP error occurred. HTTP Result Code: 200 Debugging information follows. Path: /batch?id=5&op=do_nojs&op=do StatusText: OK ResponseText: Exception: The configuration directory type <em class="placeholder">staging</em> does not exist. in config_get_config_directory() (line 526 of /Users/lukehodge/Sites/d8/www/core/includes/bootstrap.inc).

Status:Needs review» Reviewed & tested by the community

Test works perfectly for me - marking RTBC

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Only took four years :)

Committed/pushed to 8.x, moving to 7.x for backport.

Status:Patch (to be ported)» Needs review
StatusFileSize
new5.67 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 293487-29-password-reset-test.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Backported #23 to D7.

Assigned:ZenDoodles» Unassigned
Issue summary:View changes

un assigning since it had been a while and someone else has posted patch anyway. :)

Status:Needs review» Needs work

The last submitted patch, 29: 293487-29-password-reset-test.patch, failed testing.