The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
Every time we need a lowercase machine name in tests we have to type Unicode::strtolower($this->randomMachineName())
Proposed resolution
Add a new parameter to RandomGeneratorTrait::randomMachineName():
protected function randomMachineName($length = 8, $lowercase = FALSE) {
...
}
OR
Add a new method to RandomGeneratorTrait:
protected function randomLowercaseMachineName($length = 8) {
...
}
Remaining tasks
None.
User interface changes
None.
API changes
None.
Data model changes
None.
Beta phase evaluation
| Issue category | Feature because adds some DX shorthand when writing tests. |
|---|---|
| Issue priority | Not critical because without this tests are not affected. Unicode::strtolower($this->randomMachineName()) can be used instead. |
| Disruption | No disruption. |
| Comment | File | Size | Author |
|---|---|---|---|
| #40 | 2556711-40.patch | 3.07 KB | Anonymous (not verified) |
| #7 | 2556711-7.patch | 11.99 KB | claudiu.cristea |
| #7 | interdiff.txt | 9.22 KB | claudiu.cristea |
Comments
Comment #2
claudiu.cristeaPostponing on #2499199: Use RandomGeneratorTrait in BrowserTestBase.
Comment #3
claudiu.cristeaI implemented both.
Comment #4
claudiu.cristeaThis is minor but adds a DX improvement. Added beta evaluation.
Comment #5
dawehnerDo you mind converting over at least some of the usecases?
Comment #6
claudiu.cristeaYes, of course.
Comment #7
claudiu.cristeaAdded more cases (but not all).
Comment #8
claudiu.cristeaI can change all occurrences. Should I?
Comment #9
dawehnerI think its enough for now
Comment #10
claudiu.cristeaAdded CR draft at https://www.drupal.org/node/2563579.
Comment #11
jibranIt is a test only change and a non disruptive one why not make it a normal task?
Comment #13
claudiu.cristeaThat was a random failure.
Comment #15
claudiu.cristeaSetting back to RTBC as per #9.
Comment #18
claudiu.cristeaIt was a random bot failure. Back to RTBC as per #9.
Comment #22
klausiNope, we should not add yet another random() method. Instead, we should deprecate all random values in tests and convert to hard coded strings, see #2571183: Deprecate random() usage in tests to avoid random test failures.
Comment #24
claudiu.cristeaMoving this back to RTBC based on #9 till #22 will get an approval.
Comment #25
klausiWe have agreement in the other issue and I talked to alexpott at Drupalcon who agrees to not use random functions where we can.
Adding another method means we need to deprecate another method later, so I would not do that.
Also, since you should not use random*() methods in tests nobody is supposed to use this new method.
Leaving status unchanged for other opinions, if you agree with me let's close this as "won't fix".
Comment #26
dawehnerI agree with the idea to not longer use random strings, given the problems, especially in the debugging way of things. Given that this particular issue would be certainly won't fix.
Comment #27
claudiu.cristeaIMO, we need random, arbitrary strings. I'm against deprecating this the use of random*(). As I see in #2571183: Deprecate random() usage in tests to avoid random test failures there's no a final agreement on removing random* stuff. @dawehner expresses his agreement partially but he's arguing also to maintain the random strings. If you look at the random name generator you can find that is safe enough, there's no risk of overlapping in the way it's designed.
This patch adds only a DX short-hand. If we gonna drop TOTALLY the random stiff it would be easy to drop this too. If not, we keep also this short-hand to help developers write tests more easy.
Comment #28
claudiu.cristeaOuch! Cross posting.
Comment #29
pwolanin commentedCan we just fix the original method to return lower case instead of adding another one?
Comment #30
claudiu.cristeaOf course we can but what if a test needs case variation? I don't know a use-case right now.
Comment #31
pwolanin commentedIf they need case variation they can use one of the other random methods
Comment #32
claudiu.cristea@pwolanin, but what if there are tests in place? How do we know which of them need case variation?
Comment #38
Anonymous (not verified) commentedI closed #2972573: randomMachineName() should conform to processMachineName() pattern, because its purpose is to solve the same problem. The issue describes few cases when
randomMachineName()should contains only lower case, instead of use newrandomLowercaseMachineName(). Otherwise we potentially have many places with random fails + inconvenient testing expected/actual asserts.But we also cann't use
strtolowerinside ofrandomMachineName(). BecauserandomMachineName()useRandom::name(..., unique = TRUE). Butname()check unique value with case-sensitive.So, what about next variant: implement
id(), and use it inside ofrandomMachineName()?Comment #40
Anonymous (not verified) commentedComment #41
dawehnerComment #45
jonathanshawAdding a parameter to the existing method, rather than a new method, seems harmless, even if #2571183: Deprecate random() usage in tests to avoid random test failures ever went ahead which is highly uncertain.
Comment #46
krzysztof domański1.
Unicode::strtoloweris deprecated. Use mb_* functions instead of Unicode::* methodsCurrent:
strtolower($this->randomMachineName())Clear message "it returns lowercase". Helpful when viewing the patch in the browser. When you see it, you know what's going on.
Proposed:
$this->randomMachineName(8, TRUE)Less understandable code. You need to know the parameters of the method to know what this method returns.
In this case, the first argument must be set.
Comment #47
jonathanshawThis is also superseded by #2972573: randomMachineName() should conform to processMachineName() pattern.