Skip to content

Commit

Permalink
fix nondeterminism in OptionalMethodCallProcessor
Browse files Browse the repository at this point in the history
An optional method call (`__calls : [ setLocation (80%?): [40.689269,
-74.044737] ]`) would be resolved using an external source of
randomness, rather than the FakerGenerator instance used by all other
value generators, resulting in nondeterministic behavior.

We're making the generator an optional parameter to
OptionalMethodCallProcessor and falling back to random_int for
backwards-compatibility. This can be made non nullable in a next major
release.

(Also see OptionalValueResolver, this is handled in the same way there
https://github.com/nelmio/alice/blob/a7e15b08a64d4f6fb64b4c3b58bd6a710f71050e/src/Generator/Resolver/Value/Chainable/OptionalValueResolver.php#L101-L103)

Fixes gh-1230
  • Loading branch information
brainlock committed Dec 19, 2024
1 parent 5b87f64 commit 6ff527a
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 4 deletions.
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

0 comments on commit 6ff527a

Please sign in to comment.