Problem/Motivation
Discovered whilst working on #3358186: [META] Deprecate non-static random generation methods from RandomGeneratorTrait.
In \Drupal\Tests\jsonapi\Kernel\Normalizer\JsonApiDocumentTopLevelNormalizerTest::setUp there's the following code:
$this->node = Node::create([
'title' => 'dummy_title',
'type' => 'article',
'uid' => $this->user,
'body' => [
'format' => 'plain_text',
'value' => $this->randomStringValidate(42),
],
[snipped]
]);
\Drupal\Tests\RandomGeneratorTrait::randomStringValidate() validates if a given string is valid (no consecutive spaces and no starting/ending spaces) and returns a boolean.
I can see no use in a body that is TRUE or FALSE, besides 42 not being a string in the first place, and think $this->randomStringValidate(42) is a typo/auto-extend error and should have been $this->randomString(42), where it's even debatable if 42 is needed/wanted and not a clever Hitch-hiker Guide reference.
Steps to reproduce
Proposed resolution
Replace $this->randomStringValidate(42) with $this->randomString(42)
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3358375
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3358375-fix-incorrect-usage
changes, plain diff MR !3936
Comments
Comment #2
spokjeComment #4
spokjeComment #5
mondrakeI agree it looks like Doug Adams striking back
Comment #6
spokjeDropped the 42 as well.
As much as I'm a fan, we're making things more difficult to comprehend.
Comment #7
mondrakeThe default length of the string will be 8 characters, is that enough for this test? (Just question, I have no clue)
However, in HEAD now the string will probably be just a '0' or a '1' given the fact that is calling the validator callback that returns a bool and not the random string generator. So :shrug:
Comment #8
spokje_joins the shrugging_
Well, as you said, the new 8 character string is longer than the current 1.
Don't see any specific node body length related tests, so :shrug: ;)
Comment #10
longwaveCommitted and pushed ad0ffae266 to 11.x (10.2.x). Thanks!