Consider renaming randomString() and/or adding additional, context-appropriate helpers for random string generation

Instances of randomString():

core/modules/block/lib/Drupal/block/Tests/BlockLanguageTest.php:    $info_name = $this->randomString(10);
core/modules/block/lib/Drupal/block/Tests/BlockLanguageTest.php:    $info_name = $this->randomString(10);
core/modules/block/lib/Drupal/block/Tests/BlockLanguageTest.php:    $info_name = $this->randomString(10);
core/modules/comment/lib/Drupal/comment/Tests/CommentContentRebuildTest.php:    $comment_loaded->content['test_property'] = array('#value' => $this->randomString());
core/modules/contact/lib/Drupal/contact/Tests/ContactSitewideTest.php:    $this->submitContact($this->randomName(16), $email, $subject, 2, $this->randomString(128));
core/modules/contact/lib/Drupal/contact/Tests/ContactSitewideTest.php:    $this->submitContact($this->randomName(16), $email, $this->randomString(64), 3, $this->randomString(128));
core/modules/contact/lib/Drupal/contact/Tests/ContactSitewideTest.php:    $this->submitContact($this->randomName(16), $email, $this->randomString(64), 4, $this->randomString(128));
core/modules/entity/lib/Drupal/entity/Tests/EntityTranslationTest.php:        'name' => $this->randomString(),
core/modules/field/lib/Drupal/field/Tests/TranslationTest.php:        'name' => $this->randomString(),
core/modules/menu/lib/Drupal/menu/Tests/MenuNodeTest.php:      "body[$langcode][0][value]" => $this->randomString(),
core/modules/node/lib/Drupal/node/Tests/NodeBuildContentTest.php:    $node->content['test_content_property'] = array('#value' => $this->randomString());
core/modules/poll/lib/Drupal/poll/Tests/PollTestBase.php:    $edit['choice[new:0][chvotes]'] = $this->randomString(7);
core/modules/simpletest/lib/Drupal/simpletest/TestBase.php:  public static function randomString($length = 8) {
core/modules/simpletest/lib/Drupal/simpletest/Tests/MailCaptureTest.php:    $subject = $this->randomString(64);
core/modules/simpletest/lib/Drupal/simpletest/Tests/MailCaptureTest.php:    $body = $this->randomString(128);
core/modules/simpletest/lib/Drupal/simpletest/Tests/MailCaptureTest.php:        'subject' => $this->randomString(64),
core/modules/simpletest/lib/Drupal/simpletest/Tests/MailCaptureTest.php:        'body' => $this->randomString(512),
core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php:      $name = $this->randomString(8);
core/modules/system/lib/Drupal/system/Tests/Form/FormTest.php:      'textfield' => $this->randomString(),
core/modules/system/lib/Drupal/system/Tests/Pager/PagerTest.php:      watchdog('pager_test', $this->randomString(), NULL, WATCHDOG_DEBUG);
core/modules/user/lib/Drupal/user/Tests/UserCreateTest.php:        'pass[pass1]' => $pass = $this->randomString(),
core/modules/user/lib/Drupal/user/Tests/UserRolesAssignmentTest.php:      'pass[pass1]' => $pass = $this->randomString(),

First pass.

[lorentz:drupal | Tue 10:25:05] $ grep -r "randomString" *
[lorentz:drupal | Tue 10:25:12] $ 

+++ b/core/modules/simpletest/lib/Drupal/simpletest/TestBase.phpundefined
@@ -805,6 +818,25 @@ abstract class TestBase {
+  public static function randomName($length = 8) {
+    return $this->randomSafeString($length);
+  }

Oh right, can't do this because it's static.

It might just be better to rename rather than providing the wrapper anyway.

Replacing both randomString() and randomName(). This is a way bigger patch. ;)

@@ -763,16 +763,23 @@ abstract class TestBase {
   /**
    * Generates a random string of ASCII characters of codes 32 to 126.
    *
-   * The generated string includes alpha-numeric characters and common misc
-   * characters. Use this method when testing general input where the content
-   * is not restricted.
+   * The generated string includes alpha-numeric characters and common
+   * miscellaneous characters. Use this method when testing general input
+   * where the content is not restricted.
+   *
+   * Do not use this method when special characters are not allowed (e.g., in
+   * machine or file names); instead, use
+   * Drupal\simpletest\TestBase::randomSafeString().
    *
    * @param $length
    *   Length of random string to generate.
+   *
    * @return
    *   Randomly generated string.
+   *
+   * @see Drupal\simpletest\TestBase::randomSafeString()
    */
-  public static function randomString($length = 8) {
+  public static function randomUnsafeString($length = 8) {
     $str = '';
     for ($i = 0; $i < $length; $i++) {
       $str .= chr(mt_rand(32, 126));

@@ -789,12 +796,18 @@ abstract class TestBase {
    * require machine readable values (i.e. without spaces and non-standard
    * characters) this method is best.
    *
+   * Do not use this method when testing unvalidated user input. Instead, use
+   * Drupal\simpletest\TestBase::randomUnsafeString().
+   *
    * @param $length
    *   Length of random string to generate.
+   *
    * @return
    *   Randomly generated string.
+   *
+   * @see Drupal\simpletest\TestBase::randomUnsafeString()
    */
-  public static function randomName($length = 8) {
+  public static function randomSafeString($length = 8) {
     $values = array_merge(range(65, 90), range(97, 122), range(48, 57));
     $max = count($values) - 1;
     $str = chr(mt_rand(97, 122));

For reviewers, this is the actual API change. The rest is string replace.

@sun is correct in #8 that the problem is carbon- and not silicon-based. However:

1. Giving developers more obvious method names is good DX. In a perfect world devs would always consider the implications of using any random character vs. alphanumeric. However, people are only human, and experience shows us that patches with misuse of randomString() slip past reviewers all the time and break QA all the time. Adding the word "unsafe" improves DX by making it abundantly clear that they need to take something else into consideration.

2. The current method documentation is sadly lacking, so it's not just people being stupid. I'd like to backport the docs improvements I have here.

I'm open to only renaming randomString(), but I think a docs fix by itself would not remove this headache.

Sorry, what I meant only was that the documentation fixes included in my patch can and should be backported. Not the renaming. I'll factor that out into its own issue.

Moved the docs improvements into #1672764: Improve documentation of randomString() and randomName().

randomName() could/should be renamed to randomMachineName() to resolve that confusion

Duh. Let's do this. :)

I'm still of the opinion that we should also rename randomString() to randomUnsanitizedString() as well. I think that name is more neutral than randomUnsafeString() and reminds the developer exactly what they're supposed to keep in mind: whether or not the string has been sanitized. #1672764: Improve documentation of randomString() and randomName() is in as well, so hopefully the renames will prompt people to read those docs and think about sanitization when they write their tests.

Updated issue summary.

Summary updated.

Updated issue summary.

Updated issue summary.

Updated issue summary.

Updated issue summary.

#21 is a much bigger scope than I'd originally thought but it sounds like a good direction, and we could add these helpers one at a time without difficulty. randomMachineName() is then actually the first specific helper. The bigger task would be converting existing tests so that core provides good examples (instead of using machine names all over the place to sidestep the question of prior validation by Drupal, the filesystem, or whatever).

Aside, our current randomString() is actually randomBasePrintableASCIIString(), I think. :)

I spun #1676910: Rename randomName() to randomMachineName() off into its own issue since it seems non-controversial and is easy to do by itself. Updating the summary.

Updated issue summary.

Also, it occurs to me that new helpers (and the conversion of existing tests to them) could be backportable.

Updated issue summary.

Between #1676910: Rename randomName() to randomMachineName(), #1672764: Improve documentation of randomString() and randomName(), and #1860594: Ensure that randomString() always returns a character that needs to be escaped for HTML, I think the original goal of this issue has been met. We could have other context-specific methods, but that's really a separate issue scope.