[11.x] Chore: Decouple Str::random() from Validator

[michaeldyrynda]

The use of Str::random() within the Validator can lead to non-deterministic test outcomes when using random string mocking such as Str::createRandomStringsUsing() or Str::createRandomStringsUsingSequence().

For example, with a test file that contains a set of tests as follows:

test('the thing is authorised', function () {
    postJson(...)->assertForbidden();
});

test('the thing was successful', function () {
    Str::createRandomStringsUsingSequence([
        'dot_placeholder', // Account for the form request being resolved via ValidatesWhenResolved
        'some_random_string',
        'another_random_string',
    ]);

    postJson(...)
        ->assertCreated()
        ->assertStreamedContent()
        ->assertJson(fn ($json) => $json
            ->where('data.0.id', 'some_random_string') // True in isolation, dot_placeholder otherwise
            ->where('data.1.id', 'another_random_string') // True in isolation, some_random_string otherwise
            ->etc()
        );
});

When accounting for the call to Str::random() within the Validator class' constructor, the test will pass in isolation i.e. filtering to only run the thing was successful.

If, however, the test file or the full test is run, the thing was successful fails making assertions against the matching random strings due to the Validator::$placeholderHash already being set in a preceding test.

As the $placeholderHash is a static property, the state persists through a single test process, which is why it impacts file, folder, and suite, but not a single test.

This PR adds a flushState method to the Validator class and calls it from the InteractsWithTestCaseLifecycpe trait, inline with similar behaviour elsewhere in the framework, per @timacdonald's suggestion.

[timacdonald]

Instead of changing this behaviour, I wonder if we should follow the existing pattern of introducing a static flushState method that then clears the value. We would then update InteractsWithTestCaseLifecycle to call this alongside all the others?

framework/src/Illuminate/Foundation/Testing/Concerns/InteractsWithTestCaseLifecycle.php

Lines 169 to 190 in c1b1628

	AboutCommand::flushState();
	Artisan::forgetBootstrappers();
	Component::flushCache();
	Component::forgetComponentsResolver();
	Component::forgetFactory();
	ConvertEmptyStringsToNull::flushState();
	Factory::flushState();
	EncodedHtmlString::flushState();
	EncryptCookies::flushState();
	HandleExceptions::flushState($this);
	Markdown::flushState();
	Migrator::withoutMigrations([]);
	Once::flush();
	PreventRequestsDuringMaintenance::flushState();
	Queue::createPayloadUsing(null);
	RegisterProviders::flushState();
	Sleep::fake(false);
	TrimStrings::flushState();
	TrustProxies::flushState();
	TrustHosts::flushState();
	ValidateCsrfToken::flushState();
	WorkCommand::flushState();

[michaeldyrynda]

Instead of changing this behaviour

I prefer your approach, but that (selfishly) won't save me for 11.x as it doesn't exist until 12.x. I suppose it's debatable whether #54845 is a bug to be fixed within the bug fix window or not.

It could be that the inline random is used and merged-forward to 12.x and the InteractsWithTestCaseLifecycle.php adds flushState later, because it probably should do that irrespective of how the $dotPlaceholder is generated.

Moot point but the str_shuffle is probably "faster" than using random_bytes in a while loop.

[timacdonald]

11.x as it doesn't exist until 12.x

Do you meant the trait or something else? The trait has been there since 10x: https://github.com/laravel/framework/blob/10.x/src/Illuminate/Foundation/Testing/Concerns/InteractsWithTestCaseLifecycle.php#L157-L166

[michaeldyrynda]

Oh, I am blind. I was looking in Illuminate\Testing not Illuminate\Foundation\Testing