Update RestRegisterUserTest to use the ResourceTestBase base class instead of the deprecated RESTTestBase

I'll give this a shot.

@wim-leers, I'm trying to work through rewriting this test but running into some issues with how to structure my first request.

I'm unclear how to structure the request body and where to go to find out. Can you point me to any API Documentation or examples? I'm extending ResourceTestBase and getting the first `testRegisterUser` function to fail, so trying to get to green...

Any direction is greatly appreciated.

Thanks @justinlevi for looking into this!

It would help to post a patch instead of posting the code in the comment (even if you know the patch isn't quite working yet). This will help for two reasons:

• It will let us inspect the diff between the old test (that uses the deprecated base class) and the new one that uses the correct base class.
• It will run on the automated testing infrastructure, so that everyone on the issue can see the exact failure and discuss how to fix it.

I had a few issues going on that was leading to the error above, mainly that the email address wasn't be created in the body correctly using this syntax:

"mail" => [["value" => "$name@example.com"]],

and that I wasn't sending the password due to an oversight on my part.

I also noticed that the previous tests had a couple of redundant tests??

As an example:

// Attempt to register with a password when e-mail verification is on.
    $this->registerRequest('Estraven', TRUE);
    $this->assertResponse('422', 'A Password cannot be specified. It will be generated on login.');

 // Attempt to register with a password when e-mail verification is on.
    $this->registerRequest('Ursula', TRUE);
    $this->assertResponse('422', 'A Password cannot be specified. It will be generated on login.');

And

  // Attempt to register without sending a password.
    $this->registerRequest('Rick.Deckard', FALSE);
    $this->assertResponse('422', 'No password provided');

    // Attempt to register without sending a password.
    $this->registerRequest('Tibe', FALSE);
    $this->assertResponse('422', 'No password provided');

I don't know if I'm missing something, but I've gone ahead and removed those in my attached patch.

I also don't yet understand how to properly `assertMailString` as I've tried to include the AssertMailTrait as it was included in WebTestBase, but now that we're using ResourceTestBase which inherits from BrowserTestBase, we no longer have access to that method.

Any suggestions on how to handle mail tests using ResourceTestBase?

Alright, I may have reached a point where I need some additional direction/help. Please see the attached patch.

Note that all tests pass except for the ones that check email. I am not sure why this is. I've tried to include the `AssertMailTrait` in the same way that \Drupal\Tests\user\Functional\UserAdminTest and \Drupal\Tests\simpletest\Functional\MailCaptureTest use that trait.

I can even confirm that including the `testNotificationEmailAddress` method runs as expected in this test class. I've narrowed the issue down to the difference between

$this->drupalPostForm('user/register', $edit, t('Create new account'));

and

$this->registerUser($edit['name']);

Seems that Posting the form at user/register is doing something differently than making a POST request via the user.register route, at least in the way I'm attempting to do so here.

Any help is appreciated.

Updating with PHPCS coding standard fixes.

ugh - forgot to grant permissions to the anonymous user in this last refactoring.

Hmm... I can verify that

docroot/core/lib/Drupal/Core/Mail/Plugin/Mail/TestMailCollector.php

Drupal\Core\Mail\Plugin\Mail\TestMailCollector
`\Drupal::state()->set('system.test_mail_collector', $captured_emails);` is in fact getting saved in the test key_value database table.
At least when it originates from UserRegistrationResource > sendEmailNotifications

No idea why AsserMailString > getMails is not returning any emails then??

Apologies for the barrage of messages.

Could it be that we're somehow using the wrong state or context due to a sub request triggered from ResourceTestBase->request() and this new context no longer has access to the original Drupal state object??

Just shooting in the dark.

I've created a screen capture video demonstrating the issue here:
http://justinleviwinter.s3.amazonaws.com/container-state-different-from-...

The last patch uploaded has all test passing locally. I implemented a really really awful workaround to go to the key_value table directly and pull out the stored emails from 'system.test_mail_collector'. Would love some help as far as how to actually get the TestMailCollector plugin to work correctly here.

This issue can be marked as duplicated of #2887869: Convert web tests to browser tests for user module part-2 ?

Shouldn't we remove \Drupal\user\Tests\RestRegisterUserTest::testRegisterUser now?

@dawehner looks like its been already removed

I have updated the patch to remove the file mentioned in #24.

I hope this it's correct.

Thanks

Changing the Status.

I think this looks good, but because this is a removal + addition of a file instead of the file being moved - this is hard to review. Using git diff -M git might find this change as a move.

Since this is a change of the base-class I'm not sure if that is going to help to make it easier to review this though. Git might be confused about this because there is too much change in the file.

In any case, this looks good.

+++ b/core/modules/user/tests/src/Functional/RestRegisterUserTest.php
@@ -0,0 +1,321 @@
+    $prefix = $GLOBALS['databases']['default']['default']['prefix'];
+    $table = $prefix . 'key_value';
+    $db = \Drupal::database();
+    $query = "SELECT value FROM $table  WHERE name='system.test_mail_collector'";
+    $result = $db->query($query);
+    $captured_emails = unserialize($result->fetch()->value);

I'm confused, why can't we use the AssertMailTrait?

The AssertMailTrait wasn't working because of some caching/container sync issue it looks like. Adding ->resetAll() fixes this. Bit heavy handed, but *shrug*

Crediting @Wim Leers and @dawehner for code review comments.

Committed and pushed 478177541c to 8.7.x and c0465bbfc2 to 8.6.x. Thanks!

Backported to 8.6.x after running the test locally. Having the same test coverage in both branches is a good thing.

diff --git a/core/modules/user/tests/src/Functional/RestRegisterUserTest.php b/core/modules/user/tests/src/Functional/RestRegisterUserTest.php
index 9cca5abcb7..32e875cc31 100644
--- a/core/modules/user/tests/src/Functional/RestRegisterUserTest.php
+++ b/core/modules/user/tests/src/Functional/RestRegisterUserTest.php
@@ -126,7 +126,7 @@ public function testRegisterUser() {
     $this->assertTrue($user->isBlocked());
 
     $this->assertMailString('body', 'Your application for an account is', 2);
-    $this->assertMailString('body','Bob.Arctor has applied for an account', 2);
+    $this->assertMailString('body', 'Bob.Arctor has applied for an account', 2);
 
     // Verify that an authenticated user cannot register a new user, despite
     // being granted permission to do so because only anonymous users can

Fixed coding standards on commit.