Conversation
|
As discussed, the documentation adjustments are not done, yet. |
|
Also as discussed, we should update the target branch once we agreed on a naming. |
| static::assertSame($expectedErrors, $duplicate->getError($field)); | ||
|
|
||
| $this->testDataRules($this->table, $field, $dataSet, $expectedErrors); | ||
| $this->assertDataRules($this->table, $field, $dataSet, $expectedErrors); |
There was a problem hiding this comment.
Let's remove the Data from DataRules, too.
There was a problem hiding this comment.
That's not possible because we had a testDataRules and testRules methode before.
These methods became assertDataRules and assertRules.
There was a problem hiding this comment.
OK, thanks.
But I'm confused now as why we have these two methods in the first place because they almost look and read the same.
assertDataRules( Table $table, string $fieldName, array $dataSet, array $expected, array $options = []
assertRules( Table $table, string $fieldName, array $dataSet, array $expected, array $options = []
assertRulesNoErrors(Table $table, string $fieldName, array $dataSet, array $options = []
If you compare both methods in their initial state:
/**
* Validate that a given data set for a given table leads to the expected rule errors
*
* @param Table $table The table to test
* @param string $fieldName The field to check for errors.
* @param array $dataSet The data set to test.
* @param array $expected The expected errors.
* @param array $options Additional options for newEntity.
* @return void
* @todo Move to a rules dedicated helper class.
*/
protected function _testDataRules(
Table $table,
string $fieldName,
array $dataSet,
array $expected,
array $options = []
) {
$defaultOptions = ['validate' => false];
$options = $defaultOptions + $options;
$entity = $table->newEntity($dataSet, $options);
$entitySaved = $table->save($entity, $options);
static::assertFalse($entitySaved);
$errors = $entity->getError($fieldName);
static::assertEquals($expected, $errors);
}
vs.
/**
* Validate that a given data set for a given table leads to the expected table rules errors
*
* @param Table $table The table to test
* @param string $fieldName The field to check for table rules errors.
* @param array $dataSet The data set to test.
* @param array $expected The expected table rules errors.
* @param array $options Additional options for newEntity.
* @return void
*/
protected function _testRules(
Table $table,
string $fieldName,
array $dataSet,
array $expected,
array $options = [])
{
$entity = $table->newEntity($dataSet, $options);
$errors = $entity->getError($fieldName);
static::assertEmpty($errors);
$table->save($entity);
$errors = $entity->getError($fieldName);
static::assertEquals($expected, $errors);
}
I suspect they are a copy-pasta error. Each method originated in a slightly different form from a different application project and both ended up side-by-side in our open source project here.
I think we only need assertDataRules() but named assertRules(), as it merges the options with the disabled validation and thus is similar to assertRulesNoErrors().
There was a problem hiding this comment.
The difference is that assertRules() first validates the entity and only checks the rules if the entity is valid. The assertDataRules() method disables entity validation and checks only the rules.
Good question if we really need to make that distinction? If so, I would rename the method names to assertRulesWithValidation() and assertRulesOnly() so their purpose is clear.
However, we could also rename assertDataRules() to assertRules() and remove the old assertRules() method.
| */ | ||
| protected function testFullDataValidation(Table $table, array $dataSet, array $expected, array $options = []): void | ||
| { | ||
| protected function assertFullDataValidation( |
There was a problem hiding this comment.
Should we rename this and the other "full" one, too?
I'm not sure Full is the best description.
cakephp-data-validation-testing/docs/Usage.md
Lines 100 to 101 in 4a96d4b
There was a problem hiding this comment.
Yeah, I agree with you there. Do you have any ideas? How about assertValidationErrors() and assertNoValidationErrors()?
Co-authored-by: Marc Würth <marc.wuerth@orca.ch>
…-improve-method-names
Closes #44