Problem/Motivation

In #3353658: [PHPUnit 10] Provide a static alternative to randomMachineName() and implement in data providers, we introduced new static methods for random data generation in tests.

In this issue, we explore whether deprecating the non-static methods, contained in RandomGeneratorTrait, for later removal.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

mondrake created an issue. See original summary.

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: Unassigned » spokje

At first glance, this might become a META where we deprecate methods one by one, since especially $this->randomMachineName() is used a _lot_, but let's see where we end up if we go all in.

mondrake’s picture

mondrake
Location 🇮🇹
commented

To be honest, I'm not even sure this is worth doing. Having a trait in the base classes and calling the methods non-statically allows avoiding the use import.

spokje’s picture

spokje
Primary language Dutch
commented

Hmm, that makes sense, we're looking at ~2500 lines of changed code here.

Maybe we do a nice "Closed (won't fix)" and move on to bigger (or rather smaller in code lines) and better things?

mondrake’s picture

mondrake
Location 🇮🇹
commented

Maybe we could limit to deprecating the use of the magic __gets introduced by the parent (i.e. stop allowing accessing the property directly) and maybe RandomGeneratorTrait::randomStringValidate() which is just a callback that is no longer called since the entire implementation moved to the new static methods.

catch’s picture

catch
he/him
Primary language English
commented

The __get was the one from the original issue that I thought we might not want to keep around forever.

$this->randomMachineName() seems like it might be more trouble than it's worth to get rid of.

spokje’s picture

spokje
Primary language Dutch
commented

There are also \Drupal\Tests\UnitTestCase::randomMachineName and\Drupal\Tests\UnitTestCase::getRandomGenerator that are now basically copies of the methods in the Trait.

Do we want/need to do anything with those?

mondrake’s picture

mondrake
Location 🇮🇹
commented

Maybe in UnitTestCase we could use the trait and remove the direct implementations?

spokje’s picture

spokje
Primary language Dutch
commented

Opened #3358375: Fix incorrect usage of randomStringValidate in \Drupal\Tests\jsonapi\Kernel\Normalizer\JsonApiDocumentTopLevelNormalizerTest::setUp about some weird/incorrect usage of RandomGeneratorTrait::randomStringValidate()

spokje’s picture

spokje
Primary language Dutch
commented
Title: Deprecate non-static random generation methods from RandomGeneratorTrait » [META] Deprecate non-static random generation methods from RandomGeneratorTrait
Assigned: spokje » Unassigned
spokje’s picture

spokje
Primary language Dutch
commented

Turned this issue into a META to prevent child-issues from delaying up each other from being committed.

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.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Reviewed & tested by the community

@spokje I think this can be closed now?

spokje’s picture

spokje
Primary language Dutch
commented
Status: Reviewed & tested by the community » Fixed

@mondrake Agreed, thanks!

Status: Fixed » Closed (fixed)

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