Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix nondeterminism in OptionalMethodCallProcessor #1231

Merged
merged 1 commit into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/Generator/Caller/Chainable/OptionalMethodCallProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace Nelmio\Alice\Generator\Caller\Chainable;

use Faker\Generator as FakerGenerator;
use LogicException;
use Nelmio\Alice\Definition\MethodCall\OptionalMethodCall;
use Nelmio\Alice\Definition\MethodCallInterface;
Expand All @@ -33,14 +34,20 @@ final class OptionalMethodCallProcessor implements ChainableCallProcessorInterfa
*/
private $processor;

public function __construct(?CallProcessorInterface $processor = null)
/**
* @var ?FakerGenerator
*/
private $faker;

public function __construct(?CallProcessorInterface $processor = null, ?FakerGenerator $faker = null)
{
$this->processor = $processor;
$this->faker = $faker;
}

public function withProcessor(CallProcessorInterface $processor): self
{
return new self($processor);
return new self($processor, $this->faker);
}

public function canProcess(MethodCallInterface $methodCall): bool
Expand All @@ -65,7 +72,13 @@ public function process(
throw new LogicException('TODO');
}

if (random_int(0, 99) >= $methodCall->getPercentage()) {
// TODO: for BC purposes, let's fallback to random_int. The generator should be made
// non-nullable in 4.x and the random_int usage removed. (See OptionalValueResolver)
$random = null !== $this->faker
? $this->faker->numberBetween(0, 99)
: random_int(0, 99);

if ($random >= $methodCall->getPercentage()) {
return $fixtureSet;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Loader/NativeLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ protected function createCallProcessor(): CallProcessorInterface
return new CallProcessorRegistry([
new ConfiguratorMethodCallProcessor(),
new MethodCallWithReferenceProcessor(),
new OptionalMethodCallProcessor(),
new OptionalMethodCallProcessor(null, $this->getFakerGenerator()),
new SimpleMethodCallProcessor(),
]);
}
Expand Down
28 changes: 28 additions & 0 deletions tests/Loader/LoaderIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
use Nelmio\Alice\UserDetail;
use PHPUnit\Framework\TestCase;
use ReflectionClass;
use ReflectionObject;
use stdClass;
use Throwable;
use TypeError;
Expand Down Expand Up @@ -1575,6 +1576,33 @@ public function testNewlinesInIdentity(): void
);
}

public function testOptionalMethodCallsAreDeterministic(): void
{
$gen_sequence = function (NativeLoader $loader) {
$set = $loader->loadData([
FixtureEntity\Caller\Dummy::class => [
'dummy{0..9}' => [
'__construct' => false,
'__calls' => [
['setTitle (50%?)' => ['Foo']],
],
],
],
]);
$objs = array_values($set->getObjects());

return array_map(
fn ($obj) => (new ReflectionObject($obj))->getProperty('title')->getValue($obj),
$objs,
);
};

$seq_1 = $gen_sequence(new NativeLoader());
$seq_2 = $gen_sequence(new NativeLoader());

self::assertSame($seq_1, $seq_2);
}

public static function provideFixturesToInstantiate(): iterable
{
yield 'with default constructor – use default constructor' => [
Expand Down
Loading