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

Command icon 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:

Comments

Spokje created an issue. See original summary.

spokje’s picture

Status: Active » Needs review
mondrake’s picture

I agree it looks like Doug Adams striking back

spokje’s picture

Dropped the 42 as well.
As much as I'm a fan, we're making things more difficult to comprehend.

mondrake’s picture

Status: Needs review » Reviewed & tested by the community

The 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:

spokje’s picture

_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: ;)

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed ad0ffae266 to 11.x (10.2.x). Thanks!

  • longwave committed ad0ffae2 on 11.x
    Issue #3358375 by Spokje, mondrake: Fix incorrect usage of...

Status: Fixed » Closed (fixed)

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