We rely on RandomGeneratorTrait::randomString and ::randomMachineName in a couple of places in our tests. With the plan to #2571183: Deprecate random() usage in tests to avoid random test failures we should remove that.

I discussed this with @drunken_monkey in irc and we dislike random strings in tests anyway.

We should remove all the calls to random strings / machine names and replace them with hardcodes strings. Probably best if we postpone this on #2642728: Move tests away from EntityUnitTestBase? though.

Comments

borisson_ created an issue. See original summary.

borisson_’s picture

Assigned: Unassigned » borisson_

With #2642728: Move tests away from EntityUnitTestBase? in, I'm tackling this issue as well.

borisson_’s picture

I can't find any more calls to randomString or randomMachineName. I changed all the calls to normal text variant; in \Drupal\Tests\search_api\Plugin\Processor\TransliterationTest::testTransliterationWithString I added a new @dataProvider. Locally all tests still pass.

drunken monkey’s picture

Looks great, thanks! And really seems to be all of them.
I don't think the data provider for that one test method is really needed. Important is the PHP data type, so testing different strings doesn't really add anything, I'd say.

Also, have you taken care to provide sufficiently unique strings in all cases where they are looked for in other places (e.g., the page content) later, to avoid false positives in those places? It looks pretty good, but just wanting to make sure you took that into account. (And easier to ask then to check all of them again.)

borisson_’s picture

I removed the data provider. I'm quite sure that we use unique strings.

drunken monkey’s picture

Status: Needs review » Fixed

OK, excellent, thanks!
Then: committed.
Thanks again!

Status: Fixed » Closed (fixed)

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