The message telling that the new user has been e-mailed is always displayed, even if it's not true.

I don't know if it's possible to create a test to reproduce the failure.

This second patch is shorter.

Tested and all seems to work fine.

@xjm, do yo mean a test that creates a new user, fails to send an e-mail to the new user, and checks the displayed message? Seems strange to do.

Anyways, I think it's quite obvious that the return value of _user_mail_notify() should be checked before printing a success message.

Here is the test patch (It should fail before applying patch at #2). I am also put test patch together with the patch in #2.

"Mal" in "MalPhpMail is same as "Mal" in word "Malfunction".

In order to let mail function fail intentionally, I put the MalPhpMail there. Would love the change the name if you have one for it.

Meanwhile, I will convert variable_get/variable_set function to CMI.

Thanks @wuinfo! That test should provide the needed coverage. I like the solution of defining a "broken" mail service.

First, one note about file locations and namespacing for classes that are used only for testing:

I'm with @disasm that "MalPHPMail" is a bit confusing--generally, we prefer whole words in class names, functions, etc. rather than abbreviations. I'd suggest TestPhpMailFailure or something like that.

Since this class is purely for testing purposes, we should instead move it to the test namespace (including creating a test module to house it if appropriate).

Since the bug we're testing is in the user module, the place to create a test module would be in something like:

We'd create an empty module file in that directory, and then move the test mail sender to the appropriate PSR-0 directory within that module. So, the namespace for the test class will then be:
Drupal\user_mail_failure_test; and we'll need to add a use statement for Drupal\Core\Mail\PhpMail.

Make sense? :)

Aside from that, there's just a couple minor cleanups to make:

1. +++ b/core/modules/user/lib/Drupal/user/Tests/UserCreateFailMailTest.phpundefined
@@ -0,0 +1,53 @@
+ * Definition of Drupal\user\Tests\UserCreateFailMailTest.

   Minor thing, but the standards for these docblocks since last summer are that they should start with "Contains" instead of "Definition of" and have a preceding slash on the namespace, so:

   Contains \Drupal\user\Tests\UserCreateFailMailTest.

   (A lot of core isn't actually updated to the standard yet, but we should use it here.)

2. +++ b/core/modules/user/lib/Drupal/user/Tests/UserCreateFailMailTest.phpundefined
@@ -0,0 +1,53 @@
+ * Test the create user administration page.
...
+   * Create a user through the administration interface and ensure that it
+   * displays proper message when mail does not work.

   Minor docblock style note: The docblocks should be a single line of 80 characters or fewer and start with an indicative verb, e.g. "Tests."

3. +++ b/core/modules/user/lib/Drupal/user/Tests/UserCreateFailMailTest.phpundefined
@@ -0,0 +1,53 @@
+    // We stop mail function
...
+    // We create a user, notify the created user and fail to send email.

   For inline comments, we normally use the imperative, e.g.:

   // Replace the mail functionality with a fake, malfunctioning service.
   // Create a user, but fail to send an email.

   (Also make sure that they are complete sentences with capitalization, periods, articles, etc.)

4. +++ b/core/modules/user/lib/Drupal/user/Tests/UserCreateFailMailTest.phpundefined
@@ -0,0 +1,53 @@
+      'notify' => true,

   TRUE should be in all caps except in JS.

5. +++ b/core/modules/user/lib/Drupal/user/Tests/UserCreateFailMailTest.phpundefined
@@ -0,0 +1,53 @@
+    $this->assertText(t('Unable to send e-mail. Contact the site administrator if the problem persists.'),'Error message for disfunctional mail.');
+    $this->assertNoText(t('A welcome message with further instructions has been e-mailed to the new user @name.', array('@name' => $edit['name'])), 'Do not show welcome message');

   We can probably remove the assertion messages from these two assertions--the default assertion will be even more specific.

6. +++ b/core/modules/user/lib/Drupal/user/Tests/UserCreateFailMailTest.phpundefined
@@ -0,0 +1,53 @@
+    // Set the old variable back.
+    config('system.mail')->set('interface.default', $mail_system_variable)->save();

   Each test method gets its own fresh test installation, so AFAIK we don't need to do this. (Might be worth testing just to make sure it doesn't leak into the parent environment -- remove the line, run the test, and then see if the parent site's setting is affected.)

Adding the novice tag; all these changes should be fairly simple to make. If you do update @wuinfo's patch, please be sure to upload the failing patch as well as the combined patch again, and also provide an interdiff showing your changes. Thanks!

The last submitted patch, contradictory-messages-creating-new-user-test_1843380_13.patch, failed testing.

Thanks @wuinfo! Note that if you put the test-only patch first and the passing patch second, the testbot won't mark it needs work. :)

I think this is pretty much ready. I just noticed a couple small things re-reading it:

1. +++ b/core/modules/user/lib/Drupal/user/Tests/UserCreateFailMailTest.phpundefined
@@ -0,0 +1,49 @@
+ * Test the create user administration page.

   Tests, not test. :) http://drupal.org/node/1354#functions

2. +++ b/core/modules/user/lib/Drupal/user/Tests/UserCreateFailMailTest.phpundefined
@@ -0,0 +1,49 @@
+  // Enable user_mail_failure_test module.
+  public static $modules = array('user_mail_failure_test');
...
+  // Test the create user administration page.
+  protected function testUserAdd() {

   Ah, sorry if I was unclear: These should be one line only, but they should still be docblocks: http://drupal.org/node/1354#functions

   Also, "Tests". :)

3. +++ b/core/modules/user/tests/modules/user_mail_failure_test/lib/Drupal/user_mail_failure_test/TestPhpMailFailure.phpundefined
@@ -0,0 +1,26 @@
+  public function mail(array $message) {
+    return FALSE;

   Maybe just add an inline comment here as well to explicitly say:
   

   // Instead of attempting to send a message, just return failure.

   (Above the return but inside the function.)

4. +++ b/core/modules/user/tests/modules/user_mail_failure_test/user_mail_failure_test.info.ymlundefined
@@ -0,0 +1,8 @@
diff --git a/core/modules/user/tests/modules/user_mail_failure_test/user_mail_failure_test.module b/core/modules/user/tests/modules/user_mail_failure_test/user_mail_failure_test.module

   Let's also add an opening <?php tag and an @file docblock for the module file: http://drupal.org/node/1354#file

Tagging novice for those cleanups--removing the "Needs tests" tag because the current tests are exactly what we need.

Oops, one more thing -- This still needs to be a docblock as well. (Otherwise api.drupal.org and IDEs won't be able to parse it to list it as a member for the class.)

I really want to RTBC this, so I just fixed this myself. :) (In general one wouldn't RTBC one's own patch, but this is super minor and doesn't change any functional code.)

Thanks @alexpott,

Since I have changed the location of the module, the interdiff.txt file is large. So, I think it is not necessarily to include it.

@wuinfo You don't need to re-upload the patch to call testbot. Just change the status to needs review, and it will run the tests on any patches that haven't had tests ran on them.

@disasm, yes, I realized it soon after I changed the status on second comment.

The last submitted patch, contradictory-messages-creating-new-user-include-fix-1843380-27.patch, failed testing.

#27: contradictory-messages-creating-new-user-include-fix-1843380-27.patch queued for re-testing.

Committed 42bfe92 and pushed to 8.x. Thanks!