-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Scheduler] Fix optional $count
variable in testGetNextRunDates
#59125
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
Conversation
Hey! Thanks for your PR. You are targeting branch "7.3" but it seems your PR description refers to branch "6.4". Cheers! Carsonbot |
859e07a
to
88e6b36
Compare
@@ -108,7 +108,7 @@ public static function provideForToString() | |||
/** | |||
* @dataProvider providerGetNextRunDates | |||
*/ | |||
public function testGetNextRunDates(\DateTimeImmutable $from, TriggerInterface $trigger, array $expected, int $count = 0) | |||
public function testGetNextRunDates(\DateTimeImmutable $from, TriggerInterface $trigger, array $expected, ?int $count = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we never pass null either indeed:
public function testGetNextRunDates(\DateTimeImmutable $from, TriggerInterface $trigger, array $expected, ?int $count = null) | |
public function testGetNextRunDates(\DateTimeImmutable $from, TriggerInterface $trigger, array $expected, int $count) |
and let's remove the ?? count below
$count
variable in testGetNextRunDates
Thanks Nicolas. Changed made! |
88e6b36
to
d8fa076
Compare
Good catch, thanks @codedmonkey. |
Fixes a weird side effect of the optional
$count
variable intestGetNextRunDates
, because it uses the Null Coalescing Operator it was always expecting 0 iterations if not passing a value from the provider.It doesn't need to be optional because it's always passed from the provider so we could just make it a required variable.