Problem/Motivation

Follow-up to #2870465-33: Convert web tests to browser tests for user module.

When converting tests, we accidentally detected code, which may no longer be relevant and may be removed.

+++ b/core/modules/user/tests/src/Functional/UserCancelTest.php
@@ -99,23 +99,6 @@ public function testUserCancelUid1() {
-    // Update uid 1's name and password to we know it.
-    $password = user_password();
-    $account = [
-      'name' => 'user1',
-      'pass' => $this->container->get('password')->hash(trim($password)),
-    ];
-    // We cannot use $account->save() here, because this would result in the
-    // password being hashed again.
-    db_update('users_field_data')
-      ->fields($account)
-      ->condition('uid', 1)
-      ->execute();
-
-    // Reload and log in uid 1.
-    $user_storage->resetCache([1]);
-    $user1 = $user_storage->load(1);
-    $user1->pass_raw = $password;

@@ -127,7 +110,6 @@ public function testUserCancelUid1() {
-    $user_storage->resetCache([1]);

Proposed resolution

  1. Make sure that it really is not needed
  2. Remove it.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#6 interdiff-2-6.txt660 bytesAnonymous (not verified)
#6 2926090-6.patch1.16 KBAnonymous (not verified)
#2 2926090-2.patch1.51 KBAnonymous (not verified)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

vaplas created an issue. See original summary.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Active » Needs review
FileSize
2926090-2.patch1.51 KB

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer commented
Status: Needs review » Reviewed & tested by the community

I tested this, and it still applies.

I ran ./vendor/bin/phpunit -c core/ core/modules/user/ --filter "testUserCancelUid1" before applying the patch and got this: OK (1 test, 10 assertions). After applying the patch, there are still 10 assertions and the test is still green.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/user/tests/src/Functional/UserCancelTest.php
@@ -127,7 +110,6 @@ public function testUserCancelUid1() {
-    $user_storage->resetCache([1]);

This feels like a bad idea since we're testing something that has not happened on the child site.

The rest of the patch looks fine.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs work » Needs review
FileSize
2926090-6.patch1.16 KB
interdiff-2-6.txt660 bytes

Fair point! Done.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer commented
Status: Needs review » Reviewed & tested by the community

#6 fixes #5. Great work @vaplas!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 07df228247 to 8.6.x and a96f30b9d0 to 8.5.x. Thanks!

Backported to 8.5.x since this is test only code. Credited myself because #5 influenced the patch.

  • alexpott committed 07df228 on 8.6.x 
    Issue #2926090 by vaplas, alexpott: Remove dead code from UserCancelTest...

  • alexpott committed a96f30b on 8.5.x 
    Issue #2926090 by vaplas, alexpott: Remove dead code from UserCancelTest...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.