Problem/Motivation

(From #1659682: Consider renaming randomString() and/or adding additional, context-appropriate helpers for random string generation; see also #1672764: Improve documentation of randomString() and randomName())

  • DrupalTestBase::randomName() and UnitTestCase::randomName() does not entirely clarify the function's purpose: to generate a random machine name, for use generating already-validated machine names.

Proposed resolution

  • Rename randomName() to randomMachineName().

Remaining tasks

API changes

  • DrupalTestBase::randomName() is renamed to randomMachineName().
  • UnitTestCase::randomName() is renamed to randomMachineName().
CommentFileSizeAuthor
#19 1676910-randomMachineName-19.patch673.26 KBpenyaskito
#17 drupal-rename_randomName-1676910-17.patch674.71 KBl0ke
#14 drupal-rename_randomName-1676910-14.patch675.77 KBcs_shadow
#10 drupal-rename_randomName-1676910-10.patch646.55 KBcs_shadow
#8 drupal-rename_randomName-1676910-8.patch627.55 KBcs_shadow
#6 drupal-rename_randomName-1676910-6.patch627.54 KBcs_shadow
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Component: documentation » simpletest.module

I don't think this is quite a documentation issue. :)

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Issue summary: View changes

Updated issue summary.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Issue summary: View changes
Issue tags: +d8dx, +API change

Is it too late for this to land on D8?

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Issue tags: -d8dx +DX (Developer Experience), +DrupalWTF
cs_shadow’s picture

cs_shadow CreditAttribution: cs_shadow commented

Apart from DrupalTestBase::randomName(), there's also a UnitTestCase::randomName(). Does this issue implies to change both or just the first one (that's if its not too late). Someone please clarify.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Apart from DrupalTestBase::randomName(), there's also a UnitTestCase::randomName(). Does this issue implies to change both or just the first one (that's if its not too late). Someone please clarify.

You are 100% right, we should sync those 2 code snippets.

cs_shadow’s picture

cs_shadow CreditAttribution: cs_shadow commented
Status: Active » Needs review
FileSize
drupal-rename_randomName-1676910-6.patch627.54 KB

Attaching a patch where I replaced every occurrence of randomName to randomMachineName. Hope it doesn't breaks anything.

Status: Needs review » Needs work

The last submitted patch, 6: drupal-rename_randomName-1676910-6.patch, failed testing.

cs_shadow’s picture

cs_shadow CreditAttribution: cs_shadow commented
Status: Needs work » Needs review
FileSize
drupal-rename_randomName-1676910-8.patch627.55 KB

Sorry, patch in #6 was not against latest HEAD.

Status: Needs review » Needs work

The last submitted patch, 8: drupal-rename_randomName-1676910-8.patch, failed testing.

cs_shadow’s picture

cs_shadow CreditAttribution: cs_shadow commented
FileSize
drupal-rename_randomName-1676910-10.patch646.55 KB

This one should work.

cs_shadow’s picture

cs_shadow CreditAttribution: cs_shadow commented
Status: Needs work » Needs review
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Doesn't apply anymore.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Issue tags: +Novice
cs_shadow’s picture

cs_shadow CreditAttribution: cs_shadow commented
Status: Needs work » Needs review
FileSize
drupal-rename_randomName-1676910-14.patch675.77 KB

Quick reroll.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Issue tags: -Needs reroll +Needs change record

I guess we will need a change record draft.

Status: Needs review » Needs work

The last submitted patch, 14: drupal-rename_randomName-1676910-14.patch, failed testing.

l0ke’s picture

l0ke
Primary language English
Location Ukraine
CreditAttribution: l0ke commented
Status: Needs work » Needs review
FileSize
drupal-rename_randomName-1676910-17.patch674.71 KB

Reroll against latest HEAD.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Does not apply anymore.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1676910-randomMachineName-19.patch673.26 KB

Used rename with phpstorm.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Issue summary: View changes
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented

Created draft change notice: https://www.drupal.org/node/2315865

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Issue summary: View changes
cs_shadow’s picture

cs_shadow CreditAttribution: cs_shadow commented
Status: Needs review » Reviewed & tested by the community

Tweaked the Change Recoed a bit.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs change record

Committed 9eac446 and pushed to 8.0.x. Thanks!

  • alexpott committed 9eac446 on 8.0.x 
    Issue #1676910 by cs_shadow, penyaskito, lokeoke | xjm: Rename...
m1r1k’s picture

m1r1k CreditAttribution: m1r1k commented
Issue tags: +#ams2014contest

Status: Fixed » Closed (fixed)

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