What steps will reproduce the problem?
1. DateTimeTest and TimeTest test the Date rule instead of their own rule.
Three methods in tests/Rule/Date/DateTimeTest.php and tests/Rule/Date/TimeTest.php create new Date() instead of new DateTime() / new Time():
public function testGetName(): void
{
$rule = new Date(); // should be new DateTime() / new Time()
$this->assertSame('date', $rule->getName());
}
public function testSkipOnError(): void
{
$this->testSkipOnErrorInternal(new Date(), new Date(skipOnError: true));
}
public function testWhen(): void
{
$this->testWhenInternal(
new Date(),
new Date(when: static fn(mixed $value): bool => $value !== null),
);
}
So getName(), skipOnError and when are never tested for DateTime and Time. DateTest already covers Date, so these checks only run twice more on the same rule.
2. The Integer rule has no tests for getName(), skipOnError and when.
Integer has no test class of its own. NumberTest checks how Integer validates values (dataValidationPassed()/dataValidationFailed()), but testGetName(), testSkipOnError() and testWhen() there only use new Number(). AbstractNumber::getName() returns static::class, so Integer returns its own name, and no test checks that.
What is the expected result?
Every rule has these contract checks run on an instance of that rule.
What do you get instead?
DateTime, Time and Integer are not covered. A regression in their getName(), skipOnError() or when() would go unnoticed. For example: an override in the class, a missing trait after a refactoring, or a wrong default in the constructor.
Suggested fix
-
In DateTimeTest/TimeTest, replace Date with DateTime/Time in the three methods above. getName() returns 'date' for all of them (from BaseDate), so the expected value stays the same.
-
Add testIntegerGetName(), testIntegerSkipOnError() and testIntegerWhen() to NumberTest, or split them into a separate IntegerTest:
public function testIntegerSkipOnError(): void
{
$this->testSkipOnErrorInternal(new Integer(), new Integer(skipOnError: true));
}
-
Optionally, prevent this kind of copy-paste mistake: add one data-provider test that runs the skipOnError/when checks over a list of all rules, instead of a separate method in each test class. Then adding a new rule means adding one line to the provider. The #[DataProvider] returns rule × option pairs (skipOnError, when); each pair gives a factory, the option name, its default, a new value and the getter name.
Additional info
| Q |
A |
| Version |
2.x-dev (9be0c31) |
| PHP version |
any |
| Operating system |
any |
What steps will reproduce the problem?
1.
DateTimeTestandTimeTesttest theDaterule instead of their own rule.Three methods in
tests/Rule/Date/DateTimeTest.phpandtests/Rule/Date/TimeTest.phpcreatenew Date()instead ofnew DateTime()/new Time():So
getName(),skipOnErrorandwhenare never tested forDateTimeandTime.DateTestalready coversDate, so these checks only run twice more on the same rule.2. The
Integerrule has no tests forgetName(),skipOnErrorandwhen.Integerhas no test class of its own.NumberTestchecks howIntegervalidates values (dataValidationPassed()/dataValidationFailed()), buttestGetName(),testSkipOnError()andtestWhen()there only usenew Number().AbstractNumber::getName()returnsstatic::class, soIntegerreturns its own name, and no test checks that.What is the expected result?
Every rule has these contract checks run on an instance of that rule.
What do you get instead?
DateTime,TimeandIntegerare not covered. A regression in theirgetName(),skipOnError()orwhen()would go unnoticed. For example: an override in the class, a missing trait after a refactoring, or a wrong default in the constructor.Suggested fix
In
DateTimeTest/TimeTest, replaceDatewithDateTime/Timein the three methods above.getName()returns'date'for all of them (fromBaseDate), so the expected value stays the same.Add
testIntegerGetName(),testIntegerSkipOnError()andtestIntegerWhen()toNumberTest, or split them into a separateIntegerTest:Optionally, prevent this kind of copy-paste mistake: add one data-provider test that runs the
skipOnError/whenchecks over a list of all rules, instead of a separate method in each test class. Then adding a new rule means adding one line to the provider. The#[DataProvider]returns rule × option pairs (skipOnError,when); each pair gives a factory, the option name, its default, a new value and the getter name.Additional info
9be0c31)