diff --git a/CHANGELOG.md b/CHANGELOG.md index 3188f41..7557307 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - `assertValidationNotContains()` to assert that specific validation rules are absent, ignoring others on the same field. - `assertValidationListContains()` and `assertValidationListNotContains()` list helpers. - Optional custom `$expected` parameter for `assertValidationForeignKey()` and `assertValidationIsUnique()`. +- Migration script `migrate.php` to assist upgrading from 2.x to 3.x. See the [migration guide](docs/Migration.md). [#47](https://github.com/orca-services/cakephp-data-validation-testing/issues/47) ### Changed - **BREAKING CHANGE:** Replace `testDataValidation` prefix in all test trait method names with `assertValidation`. E.g. `testDataValidationNotEmpty()` becomes `assertValidationNotEmpty()`. diff --git a/README.md b/README.md index 515efc0..5b37656 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,8 @@ A CakePHP plugin to help testing data validation. | Plugin Version | CakePHP Compatibility | Branch | Status | |----------------|-----------------------|-------------| --------- | -| 2.x | 5.x | cakephp-5.x | Supported | +| 3.x | 5.x | cakephp-5.x | Supported | +| 2.x | 5.x | - | EOL | | 1.x | 4.x | cakephp-4.x | Supported | | 0.x | 2.x | cakephp-2.x | EOL | @@ -22,6 +23,11 @@ See the [installation documentation](docs/Installation.md). You can use the plugin as shown in [usage documentation](docs/Usage.md). +## Upgrading + +See the [migration guide](docs/Migration.md) for upgrading from 2.x to 3.x. +It includes a migration script that renames the old method calls and lists the places that need a manual review. + ## Versioning The releases of this plugin are versioned using [SemVer](http://semver.org/). diff --git a/docs/Migration.md b/docs/Migration.md index 548cf47..2a7d9e5 100644 --- a/docs/Migration.md +++ b/docs/Migration.md @@ -5,6 +5,46 @@ This major version bundles two breaking changes: 1. **Method renames** — [PR #45](https://github.com/orca-services/cakephp-data-validation-testing/pull/45) (closes [#44](https://github.com/orca-services/cakephp-data-validation-testing/issues/44)) 2. **Rule-dedicated methods now check only their own rule** — [PR #40](https://github.com/orca-services/cakephp-data-validation-testing/pull/40) (closes [#38](https://github.com/orca-services/cakephp-data-validation-testing/issues/38)) +You can use the [Migration Script](#migration-script) to rename the old method calls automatically +and to find the places that need a manual review. + +--- + +## Migration Script + +The Migration Script does not fully automate the upgrade, but it takes care of the method renames +and lists everything else you need to review manually. It is shipped with the package, +so first update the package to 3.x: + +```bash +composer require --dev orca-services/cakephp-data-validation-testing:^3.0 -W +``` + +Then run the script from the root of your application: + +```bash +php vendor/orca-services/cakephp-data-validation-testing/migrate.php +``` + +The script interactively asks for the directories to migrate (comma separated, default: `tests,plugins`). + +What it does: + +1. Renames all calls of the old method names to the new ones (see [Method renames](#1-method-renames)). + Only method calls and references preceded by `->` or `::` are replaced, + so your own test methods with similar names are left untouched. +2. Replaces calls of the removed `testRules()` with `assertRules()` and lists them for manual review + (see [`testRules()` removal](#testrules-removal)). +3. Lists all calls of rule-dedicated methods for manual review + (see [Rule-dedicated methods now check only their own rule](#2-rule-dedicated-methods-now-check-only-their-own-rule)). +4. Lists old method names it did not replace, e.g. in strings, callables or own methods with the same name. + +The script modifies your files in place, so make sure your working tree is clean (e.g. committed in Git) before running it. +Afterward: + +- Review the diff (e.g. `git diff`) and the listed findings. +- Run your test suite and fix any failing tests. + --- ## 1. Method renames @@ -52,6 +92,13 @@ This avoids PHPUnit mistaking them for actual test methods. **No logic changed** | `testDataValidationForeignKey()` | `assertValidationForeignKey()` | | `testDataValidationIsUnique()` | `assertValidationIsUnique()` | +### `testRules()` removal + +`testRules()` was removed without a direct equivalent. Use `assertRules()` instead, but note the difference in behavior: +`assertRules()` skips data validation (`validate => false`) and asserts that saving fails, +while `testRules()` first asserted that there are no validation errors. +Review each replaced call to make sure the test still covers what you intend. + --- ## 2. Rule-dedicated methods now check only their own rule @@ -59,3 +106,6 @@ This avoids PHPUnit mistaking them for actual test methods. **No logic changed** Previously, methods like `assertValidationBoolean()`, `assertValidationEmail()`, `assertValidationInteger()`, etc. compared the **entire** error array for a field against an expected array (or `[]` for valid values). If a field had multiple validation errors, this could hide unrelated errors or cause false failures. Now these methods assert **only their own rule key** (present or absent), ignoring any other errors on the same field. + +Review your calls of these methods, especially where a custom `$expected` array containing several rules is passed, +and make sure the tests still cover what you intend. The [Migration Script](#migration-script) lists all these calls for you. diff --git a/migrate.php b/migrate.php new file mode 100644 index 0000000..b2ea2f2 --- /dev/null +++ b/migrate.php @@ -0,0 +1,331 @@ + [ + 'Directories to migrate', + 'comma separated, relative to ' . getcwd(), + 'tests,plugins', + ], +]; + +/** + * Old method name => new method name + * + * Section 1 of the migration guide. + */ +$renames = [ + 'testDataValidationNotEmpty' => 'assertValidationNotEmpty', + 'testDataValidationEmpty' => 'assertValidationEmpty', + 'testDataValidationRequired' => 'assertValidationRequired', + 'testDataValidationNotRequired' => 'assertValidationNotRequired', + 'testDataValidationBoolean' => 'assertValidationBoolean', + 'testDataValidationURLWithProtocol' => 'assertValidationURLWithProtocol', + 'testDataValidationDateTime' => 'assertValidationDateTime', + 'testDataValidationDate' => 'assertValidationDate', + 'testDataValidationInList' => 'assertValidationInList', + 'testDataValidation' => 'assertValidation', + 'testDataValidationNoErrors' => 'assertValidationNoErrors', + 'testFullDataValidation' => 'assertValidationTableErrors', + 'testFullDataValidationNoErrors' => 'assertValidationTableNoErrors', + 'testDataValidationContains' => 'assertValidationContains', + 'testDataValidationNotContains' => 'assertValidationNotContains', + 'assertDataValidationErrorsContain' => 'assertValidationErrorsContain', + 'testDataValidationListContains' => 'assertValidationListContains', + 'testDataValidationListNotContains' => 'assertValidationListNotContains', + 'testDataRules' => 'assertRules', + // Removed, assertRules() is the replacement. See $manualReviewRenames. + 'testRules' => 'assertRules', + 'testDataRulesNoErrors' => 'assertRulesNoErrors', + 'testDataValidationMaxLength' => 'assertValidationMaxLength', + 'testDataValidationMinLength' => 'assertValidationMinLength', + 'testDataValidationScalar' => 'assertValidationScalar', + 'testDataValidationDecimal' => 'assertValidationDecimal', + 'testDataValidationInteger' => 'assertValidationInteger', + 'testDataValidationNonNegativeInteger' => 'assertValidationNonNegativeInteger', + 'testDataValidationGreaterThanOrEqual' => 'assertValidationGreaterThanOrEqual', + 'testDataValidationEmail' => 'assertValidationEmail', + 'testDataValidationUuid' => 'assertValidationUuid', + 'testDataValidationLengthBetween' => 'assertValidationLengthBetween', + 'testDataValidationRange' => 'assertValidationRange', + 'testDataValidationNaturalNumber' => 'assertValidationNaturalNumber', + 'testDataValidationForeignKey' => 'assertValidationForeignKey', + 'testDataValidationIsUnique' => 'assertValidationIsUnique', +]; + +/** + * Old method names whose replacement does not behave exactly the same, with the reason + */ +$manualReviewRenames = [ + 'testRules' => 'testRules() was removed. assertRules() skips data validation (validate => false) ' + . 'and asserts that saving fails, while testRules() asserted that there are no validation errors first.', +]; + +/** + * Rule-dedicated methods, which now only check their own rule (section 2 of the migration guide) + */ +$ruleDedicatedMethods = [ + 'assertValidationNotEmpty', + 'assertValidationEmpty', + 'assertValidationRequired', + 'assertValidationNotRequired', + 'assertValidationBoolean', + 'assertValidationURLWithProtocol', + 'assertValidationDateTime', + 'assertValidationDate', + 'assertValidationMaxLength', + 'assertValidationMinLength', + 'assertValidationScalar', + 'assertValidationDecimal', + 'assertValidationInteger', + 'assertValidationNonNegativeInteger', + 'assertValidationGreaterThanOrEqual', + 'assertValidationEmail', + 'assertValidationUuid', + 'assertValidationLengthBetween', + 'assertValidationRange', + 'assertValidationNaturalNumber', +]; + +$values = []; + +function read_from_console($prompt) +{ + if (function_exists('readline')) { + $line = trim(readline($prompt)); + if (!empty($line)) { + readline_add_history($line); + } + } else { + echo $prompt; + $line = trim(fgets(STDIN)); + } + + return $line; +} + +/** + * Build the regular expression matching calls/references of the given method names + * + * Only matches names directly preceded by "->" or "::", so that e.g. own test methods named alike are not touched. + * The longest names come first and a word boundary is enforced, so `testDataValidation` never matches a longer name. + */ +function method_call_pattern(array $methodNames) +{ + usort($methodNames, static function ($a, $b) { + return strlen($b) - strlen($a); + }); + $alternatives = implode('|', array_map(static function ($name) { + return preg_quote($name, '/'); + }, $methodNames)); + + return '/(->|::)(\s*)(' . $alternatives . ')\b/'; +} + +/** + * Find all PHP files in the given directories + */ +function find_php_files(array $paths) +{ + $files = []; + foreach ($paths as $path) { + if (is_file($path) && substr($path, -4) === '.php') { + $files[] = $path; + continue; + } + if (!is_dir($path)) { + echo "Warning: '$path' does not exist, skipping.\n"; + continue; + } + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS), + ); + foreach ($iterator as $file) { + if ($file->isFile() && $file->getExtension() === 'php') { + $files[] = $file->getPathname(); + } + } + } + sort($files); + + return array_unique($files); +} + +/** + * Get the line number of a byte offset within a text + */ +function line_of($text, $offset) +{ + return substr_count($text, "\n", 0, $offset) + 1; +} + +$modify = 'n'; +do { + if ($modify == 'q') { + exit; + } + + $values = []; + + echo "----------------------------------------------------------------------\n"; + echo 'Migration of ' . PACKAGE_NAME . " to 3.x\n"; + echo "Please, provide the following information:\n"; + echo "----------------------------------------------------------------------\n"; + foreach ($fields as $fieldKey => $field) { + $default = $field[COL_DEFAULT] ?? ''; + $prompt = sprintf( + '%s%s%s: ', + $field[COL_DESCRIPTION], + $field[COL_HELP] ? ' (' . $field[COL_HELP] . ')' : '', + $default !== '' ? ' [' . $default . ']' : '', + ); + $values[$fieldKey] = read_from_console($prompt); + if (empty($values[$fieldKey])) { + $values[$fieldKey] = $default; + } + } + echo "\n"; + + echo "----------------------------------------------------------------------\n"; + echo "Please, check that everything is correct:\n"; + echo "----------------------------------------------------------------------\n"; + foreach ($fields as $fieldKey => $field) { + echo $field[COL_DESCRIPTION] . ": $values[$fieldKey]\n"; + } + echo "\n"; +} while (($modify = strtolower(read_from_console('Migrate files with these values? [y/N/q] '))) !== 'y'); +echo "\n"; + +$paths = array_filter(array_map('trim', explode(',', $values['paths']))); +$filesToMigrate = find_php_files($paths); + +$renamePattern = method_call_pattern(array_keys($renames)); +$ruleDedicatedPattern = method_call_pattern($ruleDedicatedMethods); +$leftoverPattern = '/\b(' . implode('|', array_map(static function ($name) { + return preg_quote($name, '/'); + }, array_keys($renames))) . ')\b/'; + +$totalReplacements = 0; +$changedFiles = 0; +$manualReview = []; +$ruleDedicatedCalls = []; +$leftovers = []; + +echo "----------------------------------------------------------------------\n"; +echo "Renaming methods:\n"; +echo "----------------------------------------------------------------------\n"; +foreach ($filesToMigrate as $filename) { + $contentToReplaceIn = file_get_contents($filename); + + // Collect the calls that need a manual review, before replacing them + if (preg_match_all($renamePattern, $contentToReplaceIn, $matches, PREG_OFFSET_CAPTURE)) { + foreach ($matches[3] as $match) { + if (isset($manualReviewRenames[$match[0]])) { + $manualReview[] = sprintf( + '%s:%d: %s', + $filename, + line_of($contentToReplaceIn, $match[1]), + $manualReviewRenames[$match[0]], + ); + } + } + } + + $count = 0; + $migratedContent = preg_replace_callback( + $renamePattern, + static function ($match) use ($renames) { + return $match[1] . $match[2] . $renames[$match[3]]; + }, + $contentToReplaceIn, + -1, + $count, + ); + + if ($count > 0) { + file_put_contents($filename, $migratedContent); + echo "$filename: $count replacement(s)\n"; + $totalReplacements += $count; + $changedFiles++; + } + + // Rule-dedicated methods now only check their own rule + if (preg_match_all($ruleDedicatedPattern, $migratedContent, $matches, PREG_OFFSET_CAPTURE)) { + foreach ($matches[3] as $match) { + $ruleDedicatedCalls[] = sprintf( + '%s:%d: %s()', + $filename, + line_of($migratedContent, $match[1]), + $match[0], + ); + } + } + + // Old names still present, e.g. in strings, callables or own method definitions + if (preg_match_all($leftoverPattern, $migratedContent, $matches, PREG_OFFSET_CAPTURE)) { + foreach ($matches[1] as $match) { + $leftovers[] = sprintf( + '%s:%d: %s', + $filename, + line_of($migratedContent, $match[1]), + $match[0], + ); + } + } +} +echo "\n$totalReplacements replacement(s) in $changedFiles of " . count($filesToMigrate) . " file(s).\n\n"; + +echo "Done.\n\n"; + +if (!empty($manualReview)) { + echo "----------------------------------------------------------------------\n"; + echo "Please, review these replacements manually:\n"; + echo "----------------------------------------------------------------------\n"; + echo implode("\n", $manualReview) . "\n\n"; +} + +if (!empty($leftovers)) { + echo "----------------------------------------------------------------------\n"; + echo "Old method names that were not replaced automatically:\n"; + echo "(e.g. in strings, callables or own methods with the same name)\n"; + echo "----------------------------------------------------------------------\n"; + echo implode("\n", $leftovers) . "\n\n"; +} + +if (!empty($ruleDedicatedCalls)) { + echo "----------------------------------------------------------------------\n"; + echo "Rule-dedicated methods now check only their own rule.\n"; + echo "Previously the entire error array of the field was compared, which could hide\n"; + echo "unrelated errors. Please, check that these tests still cover what you intend,\n"; + echo "especially where a custom \$expected with several rules is passed:\n"; + echo "----------------------------------------------------------------------\n"; + echo implode("\n", $ruleDedicatedCalls) . "\n\n"; +} + +echo "\nNext steps:\n"; +echo "- Run your test suite and fix failing tests.\n"; +echo "- Review the diff (e.g. git diff) before committing.\n\n"; + +echo "See https://github.com/orca-services/cakephp-data-validation-testing/blob/cakephp-5.x/docs/Migration.md\n";