diff --git a/src/Console/Renderers/ReportRenderer.php b/src/Console/Renderers/ReportRenderer.php index c380787..6d651d5 100644 --- a/src/Console/Renderers/ReportRenderer.php +++ b/src/Console/Renderers/ReportRenderer.php @@ -10,33 +10,41 @@ use Symfony\Component\Console\Output\OutputInterface; /** - * Prints a migration analysis: findings grouped by rule and risk, worst first, then - * the tally underneath. + * Prints a migration analysis, in one of two modes. * - * ## Why grouped, and why the prose is printed once + * ## Summary, by default * - * The first version printed every finding in full, and on a real application that - * was unusable: 124 cascading-foreign-key findings meant the same five-line - * explanation and three-line remediation printed 124 times — about a thousand lines - * of identical prose. An explanation is a property of the *rule*, not of each place - * the rule fired, and printing it per occurrence buried the one thing the reader - * needed, which is the list of places. + * A count per risk level with the rules contributing to it, the worst few findings, + * and where to find the rest. Its length does not depend on how many findings there + * are, which is the entire point: this renderer previously printed 693 lines against + * a real 170-migration application, and output that long is not read, it is scrolled + * past — so the findings in it are worth nothing however correct they are. * - * So a group whose findings all share the same explanation prints it once and then - * lists the occurrences as one line each. A group whose explanations genuinely - * differ — `drop-column` names the indexes on each column, `add-index` quotes each - * table's row count — prints the first few in full and says how many it held back. - * The distinction is made by comparing the text, so it stays right as rules change. + * ## Detail, with `-v` * - * Three things are never abbreviated away: the risk level, whether the operation is - * destructive, and whether the parser understood the whole file. + * Every finding, grouped by rule, risk and the prose it carries, so a shared + * explanation is printed once for the whole group rather than once per finding. + * + * That grouping only works because rules keep per-occurrence facts out of their + * explanations and put them in {@see MigrationFinding::$context} instead. When they + * did not — when `drop-column` appended each table's row count to its paragraph — + * every finding became a group of one and the detail view was ten times longer than + * it needed to be. + * + * Four things are never abbreviated away, in either mode: the risk tally, the count + * of accepted findings, whether the database was reachable, and what the parser + * could not read. They are what a reader would not know to ask for, and dropping + * them is how a summary becomes a lie. */ final class ReportRenderer { /** How many occurrences a group shows before it starts counting instead. */ private const int PREVIEW = 3; - public function render(OutputInterface $output, MigrationReport $report): void + /** + * @param string $command What to point the reader at for the full detail. + */ + public function render(OutputInterface $output, MigrationReport $report, string $command = 'difflock:lint'): void { if ($report->migrations === []) { $this->nothingFound($output); @@ -44,12 +52,100 @@ public function render(OutputInterface $output, MigrationReport $report): void return; } + // Summary unless asked otherwise. On a real application this is the + // difference between twenty lines and seven hundred, and seven hundred lines + // of correct findings are worth nothing because nobody reads them. + $output->isVerbose() + ? $this->detail($output, $report) + : $this->summary($output, $report, $command); + + $this->warnings($output, $report); + } + + /** Every finding, grouped. What `-v` gives you. */ + public function detail(OutputInterface $output, MigrationReport $report): void + { foreach ($this->grouped($report->findings) as $group) { - $this->group($output, $group, $output->isVerbose()); + $this->group($output, $group, true); } $this->tally($output, $report); - $this->warnings($output, $report); + } + + /** + * The bounded view: what was found, the worst of it, and where the rest is. + * + * Length is independent of the number of findings — one line per risk level that + * has any, plus a fixed-size worst list. The tally, the accepted count and the + * parser warnings are never abbreviated away, because they are the things a + * reader would not know to ask for. + */ + public function summary(OutputInterface $output, MigrationReport $report, string $command): void + { + $summary = $report->summary(); + + if ($summary->total === 0) { + $output->writeln(' ✓ Nothing to report.'); + $output->writeln(''); + $this->analysed($output, $report); + + return; + } + + foreach (array_reverse(RiskLevel::ascending()) as $level) { + $count = $summary->count($level); + + if ($count === 0) { + continue; + } + + $output->writeln( + ' colour().';options=bold>'.$level->glyph().' '.Text::pad($level->label(), 9).'' + .str_pad((string) $count, 4, ' ', STR_PAD_LEFT) + .' '.implode(', ', $this->rulesAt($report->findings, $level)).'', + ); + } + + $output->writeln(''); + $output->writeln(' Worst'); + + foreach (array_slice($report->findings, 0, self::PREVIEW) as $finding) { + $output->writeln(' '.$finding->message); + $output->writeln(' '.$this->where($finding).''); + } + + $output->writeln(''); + $this->analysed($output, $report); + + foreach ([ + $command.' -v' => 'every finding in full', + $command.' --rule=NAME' => 'one rule at a time', + 'difflock:report' => 'a shareable HTML report', + ] as $invocation => $describes) { + $output->writeln(' → '.Text::pad($invocation, 30).$describes.''); + } + + $output->writeln(''); + } + + /** + * The rules contributing to a level, so the tally says what kind of problem it is + * rather than only how much of it there is. + * + * @param list $findings + * @return list + */ + private function rulesAt(array $findings, RiskLevel $level): array + { + $rules = []; + + foreach ($findings as $finding) { + if ($finding->risk === $level) { + $rules[$finding->rule] = true; + } + } + + return array_keys($rules); } /** @@ -136,10 +232,16 @@ private function prose(OutputInterface $output, MigrationFinding $finding, strin */ private function occurrences(OutputInterface $output, array $group, bool $verbose): void { + // `-v` means every finding — the summary is where brevity lives now, so + // truncating here as well would leave no way to see the whole picture. $shown = $verbose ? $group : array_slice($group, 0, self::PREVIEW); foreach ($shown as $finding) { $output->writeln(' '.$finding->message.' '.$this->where($finding).''); + + if ($finding->context !== null) { + $output->writeln(' '.$finding->context.''); + } } $this->remainder($output, count($group) - count($shown), $group[0]->rule); @@ -196,7 +298,6 @@ private function nothingFound(OutputInterface $output): void private function tally(OutputInterface $output, MigrationReport $report): void { $summary = $report->summary(); - $analyzed = count($report->migrations); $output->writeln(' Risk'); @@ -210,10 +311,25 @@ private function tally(OutputInterface $output, MigrationReport $report): void } $output->writeln(''); - $output->writeln(' '.$analyzed.' migration'.($analyzed === 1 ? '' : 's').' analysed.'); + $this->analysed($output, $report); + } + + /** + * How much was looked at, and what was held back. + * + * Printed in both modes. An accepted backlog nobody can see is a backlog that + * quietly becomes permanent. + */ + private function analysed(OutputInterface $output, MigrationReport $report): void + { + $analyzed = count($report->migrations); + $total = $report->summary()->total; + + $output->writeln( + ' '.$analyzed.' migration'.($analyzed === 1 ? '' : 's').' analysed' + .($total === 0 ? '' : ' · '.$total.' finding'.($total === 1 ? '' : 's')).'.', + ); - // Never silent: an accepted backlog that nobody can see is a backlog that - // quietly becomes permanent. if ($report->accepted !== []) { $output->writeln( ' '.count($report->accepted).' previously accepted finding' diff --git a/src/Migration/MigrationContext.php b/src/Migration/MigrationContext.php index 7637139..4f62c8a 100644 --- a/src/Migration/MigrationContext.php +++ b/src/Migration/MigrationContext.php @@ -98,6 +98,7 @@ public function finding( bool $destructive = false, bool $reversible = true, ?Operation $operation = null, + ?string $context = null, ): MigrationFinding { return new MigrationFinding( rule: $rule, @@ -113,6 +114,7 @@ public function finding( reversible: $reversible, line: $operation->line ?? $this->statement->line, conditional: $operation->conditional ?? $this->statement->conditional, + context: $context, ); } } diff --git a/src/Migration/MigrationFinding.php b/src/Migration/MigrationFinding.php index e4d037c..a0a4eb7 100644 --- a/src/Migration/MigrationFinding.php +++ b/src/Migration/MigrationFinding.php @@ -32,6 +32,11 @@ * @param string|null $subject The column, index or constraint the finding is about. * @param bool $conditional Whether the operation sits inside an `if` or a loop, so it * may not run at all. The message is phrased accordingly. + * @param string|null $context A short phrase about *this* occurrence — `82,325 rows`, + * `covered by users_email_index`. Everything that varies + * between two findings of the same rule belongs here, so + * that the explanation can stay invariant and be printed + * once for the whole group instead of once per finding. */ public function __construct( public string $rule, @@ -47,6 +52,7 @@ public function __construct( public bool $reversible = true, public ?int $line = null, public bool $conditional = false, + public ?string $context = null, ) {} /** @@ -95,6 +101,7 @@ public function toArray(): array 'reversible' => $this->reversible, 'conditional' => $this->conditional, 'line' => $this->line, + 'context' => $this->context, ]; if ($this->subject !== null && $this->subjectType !== Subject::None) { diff --git a/src/Migration/Rules/AddIndexRule.php b/src/Migration/Rules/AddIndexRule.php index 39a5208..67a5187 100644 --- a/src/Migration/Rules/AddIndexRule.php +++ b/src/Migration/Rules/AddIndexRule.php @@ -77,10 +77,6 @@ private function finding(MigrationContext $context, Operation $operation): Migra .'takes a lock, and for how long, depends on the database engine and version — Difflock ' .'does not know which applies here, so it judges by size alone.'; - $explanation .= $size === null - ? ' The size of the table could not be determined.' - : ' The table holds '.$size.'.'; - if ($unique) { $explanation .= ' A unique index also fails outright if the existing rows contain ' .'duplicates, which stops the migration partway through.'; @@ -97,6 +93,7 @@ private function finding(MigrationContext $context, Operation $operation): Migra subjectType: Subject::Index, reversible: $context->reversible(), operation: $operation, + context: $size ?? 'table size could not be determined', ); } diff --git a/src/Migration/Rules/AddNotNullColumnRule.php b/src/Migration/Rules/AddNotNullColumnRule.php index edf9da6..8099859 100644 --- a/src/Migration/Rules/AddNotNullColumnRule.php +++ b/src/Migration/Rules/AddNotNullColumnRule.php @@ -70,6 +70,9 @@ private function finding( $column = Blueprint::columnsOf($operation)[0] ?? ''; $table = $context->tableName() ?? ''; + // Three genuinely different arguments, not three renderings of one — so these + // stay in the explanation and collapse to three groups. The row count itself + // varies per finding and goes in the context line. [$risk, $because] = match (true) { $rows === null => [ RiskLevel::Medium, @@ -78,9 +81,8 @@ private function finding( ], $rows > 0 => [ RiskLevel::High, - 'The table holds '.($context->database->describeSize($table) ?? $rows.' rows') - .', and every one of them needs a value the migration does not supply. Most ' - .'engines refuse the statement rather than inventing one.', + 'The table is not empty, and every existing row needs a value the migration does not ' + .'supply. Most engines refuse the statement rather than inventing one.', ], default => [ RiskLevel::Low, @@ -100,6 +102,7 @@ private function finding( subjectType: Subject::Column, reversible: $context->reversible(), operation: $operation, + context: $context->database->describeSize($table), ); } } diff --git a/src/Migration/Rules/DropColumnRule.php b/src/Migration/Rules/DropColumnRule.php index c576334..0da1aba 100644 --- a/src/Migration/Rules/DropColumnRule.php +++ b/src/Migration/Rules/DropColumnRule.php @@ -64,20 +64,29 @@ private function column(MigrationContext $context, Operation $operation, string { $table = $context->tableName() ?? ''; + // Invariant: the same sentence for every dropped column, so the renderer can + // print it once for the whole group. Everything specific to *this* column — + // how many rows, what is built on it — goes in the context line. $explanation = 'Dropping a column destroys the values in it. A `down()` that adds the column ' .'back gives you the column and not one row of what was in it.'; + if (str_starts_with($operation->method, 'dropConstrainedForeignId')) { + $explanation .= ' This form also drops the foreign key constraint on the column, so ' + .'anything relying on it for referential integrity loses it.'; + } + + $facts = []; + $size = $context->database->describeSize($context->tableName()); if ($size !== null) { - $explanation .= ' The table holds '.$size.'.'; + $facts[] = $size; } - $explanation .= $this->dependants($context->liveTable(), $column); + $dependants = $this->dependants($context->liveTable(), $column); - if (str_starts_with($operation->method, 'dropConstrainedForeignId')) { - $explanation .= ' This also drops the foreign key constraint on the column, so anything ' - .'relying on that constraint for referential integrity loses it.'; + if ($dependants !== '') { + $facts[] = $dependants; } return $context->finding( @@ -92,6 +101,7 @@ private function column(MigrationContext $context, Operation $operation, string destructive: true, reversible: false, operation: $operation, + context: $facts === [] ? null : implode(' · ', $facts), ); } @@ -123,20 +133,20 @@ private function dependants(?Table $table, string $column): string } } - $notes = ''; + $notes = []; if ($indexes !== []) { - $notes .= ' It is covered by '.$this->list(array_map( + $notes[] = 'covered by '.$this->list(array_map( static fn (Index $index): string => $index->name, $indexes, - )).', which the drop takes with it.'; + )); } if ($keys !== []) { - $notes .= ' The foreign key '.$this->list($keys).' is built on it.'; + $notes[] = 'foreign key '.$this->list($keys).' built on it'; } - return $notes; + return implode(', ', $notes); } private function unresolved(MigrationContext $context, Operation $operation): MigrationFinding diff --git a/src/Migration/Rules/DropIndexRule.php b/src/Migration/Rules/DropIndexRule.php index 6e5b246..a10bf1c 100644 --- a/src/Migration/Rules/DropIndexRule.php +++ b/src/Migration/Rules/DropIndexRule.php @@ -12,6 +12,7 @@ use Difflock\Migration\Subject; use Difflock\Migration\Thresholds; use Difflock\Risk\RiskLevel; +use Difflock\Schema\Index; /** * `dropIndex()`, `dropUnique()`, `dropPrimary()`. @@ -81,9 +82,22 @@ private function finding(MigrationContext $context, Operation $operation): Migra ? 'This removes a constraint the database was enforcing. Rows that would have been ' .'rejected a moment ago are now accepted, and restoring the constraint later means ' .'finding and resolving whatever got in while it was gone.' - : $this->usage($context, $scans); + : $this->usage($scans); - $explanation .= $this->covered($context, $operation); + $facts = []; + + if ($scans !== null) { + $window = $context->database->indexObservedDays(); + + $facts[] = Thresholds::format($scans).' read'.($scans === 1 ? '' : 's') + .($window === null ? '' : ' in '.$window.' day'.($window === 1 ? '' : 's')); + } + + $covered = $this->covered($context, $operation); + + if ($covered !== '') { + $facts[] = $covered; + } return $context->finding( rule: $this->identifier(), @@ -96,6 +110,7 @@ private function finding(MigrationContext $context, Operation $operation): Migra subjectType: Subject::Index, reversible: $context->reversible(), operation: $operation, + context: $facts === [] ? null : implode(' · ', $facts), ); } @@ -106,7 +121,7 @@ private function finding(MigrationContext $context, Operation $operation): Migra * without one is uninterpretable — zero reads since a restart an hour ago is not * evidence of anything. */ - private function usage(MigrationContext $context, ?int $scans): string + private function usage(?int $scans): string { if ($scans === null) { return 'The engine would not say how often this index has been read, so Difflock cannot ' @@ -114,21 +129,18 @@ private function usage(MigrationContext $context, ?int $scans): string .'around it will be planned differently once it is gone.'; } - $window = $context->database->indexObservedDays(); - $over = $window === null - ? 'since the engine last reset its statistics' - : 'over the '.$window.' day'.($window === 1 ? '' : 's').' since the engine last reset its statistics'; - + // Invariant per branch: the count and its window are facts about this index + // and live on the context line, so a hundred unused indexes share one + // paragraph rather than printing a hundred near-identical ones. if ($scans === 0) { - return 'The engine reports this index has been read '.($window === null ? 'no times ' : 'no times ') - .$over.'. That is the strongest evidence available that nothing needs it — with two ' - .'caveats: the counters are per instance, so a replica serving reads is invisible ' - .'from here, and a short window since a restart proves nothing.'; + return 'The engine reports no reads of this index since it last reset its statistics. ' + .'That is the strongest evidence available that nothing needs it — with two caveats: ' + .'the counters are per instance, so a replica serving reads is invisible from here, ' + .'and a short window since a restart proves nothing.'; } - return 'The engine reports this index has been read '.Thresholds::format($scans).' time' - .($scans === 1 ? '' : 's').' '.$over.'. Something is using it, and those queries will be ' - .'planned differently once it is gone.'; + return 'The engine reports this index is being read. Something is using it, and those queries ' + .'will be planned differently once it is gone.'; } private function suggestion(bool $constraint, ?int $scans): string @@ -158,10 +170,10 @@ private function covered(MigrationContext $context, Operation $operation): strin $name = $operation->stringArgument(0); $index = $name === null ? null : $context->liveTable()?->index($name); - if (! $index instanceof \Difflock\Schema\Index) { + if (! $index instanceof Index) { return ''; } - return ' It currently covers ('.implode(', ', $index->columns).').'; + return 'covers ('.implode(', ', $index->columns).')'; } } diff --git a/src/Migration/Rules/DropTableRule.php b/src/Migration/Rules/DropTableRule.php index 3908754..ad20455 100644 --- a/src/Migration/Rules/DropTableRule.php +++ b/src/Migration/Rules/DropTableRule.php @@ -47,15 +47,16 @@ public function analyze(MigrationContext $context): array .'and may be subject to a retention obligation that outlives the feature that wrote them.'; } + $facts = []; + if ($size !== null) { - $explanation .= ' This table currently holds '.$size.'.'; + $facts[] = $size; } if (! $context->database->available) { - $explanation .= ' The database could not be reached, so the size of the table is unknown.'; + $facts[] = 'database unreachable, size unknown'; } elseif ($table !== null && ! $context->database->hasTable($table)) { - $explanation .= ' The table does not exist on the inspected database, so this drop may ' - .'already have run there, or may target a database this one is not.'; + $facts[] = 'not present on the inspected database'; } return [$context->finding( @@ -69,6 +70,7 @@ public function analyze(MigrationContext $context): array subjectType: Subject::Table, destructive: true, reversible: false, + context: $facts === [] ? null : implode(' · ', $facts), )]; } diff --git a/src/Migration/Rules/ForeignKeyRule.php b/src/Migration/Rules/ForeignKeyRule.php index d52146f..31a9e48 100644 --- a/src/Migration/Rules/ForeignKeyRule.php +++ b/src/Migration/Rules/ForeignKeyRule.php @@ -126,14 +126,14 @@ private function added(MigrationContext $context, Operation $operation): Migrati .($column === '' ? '' : '.'.$column), explanation: 'Adding a constraint to an existing table makes the database validate every ' .'row already in it. If any of them points at a parent that is not there, the ' - .'statement is refused and the migration stops partway through.' - .($size === null ? ' The size of the table could not be determined.' : ' The table holds '.$size.'.'), + .'statement is refused and the migration stops partway through.', suggestion: 'Find and resolve the orphans before deploying — a `whereNotExists` against the ' .'parent table is usually enough to know whether there are any.', subject: $column === '' ? null : $column, subjectType: Subject::Constraint, reversible: $context->reversible(), operation: $operation, + context: $size ?? 'table size could not be determined', ); } diff --git a/src/Migration/Rules/LargeTableRule.php b/src/Migration/Rules/LargeTableRule.php index 96ec0e1..c427424 100644 --- a/src/Migration/Rules/LargeTableRule.php +++ b/src/Migration/Rules/LargeTableRule.php @@ -48,18 +48,18 @@ public function analyze(MigrationContext $context): array return [$context->finding( rule: $this->identifier(), risk: RiskLevel::Medium, - message: 'ALTER on a large table: '.($table ?? '') - .' ('.$context->database->describeSize($table).')', - explanation: 'The table is above the size Difflock is configured to treat as large' - .($bytes === null ? '' : ', and occupies about '.Bytes::human($bytes)) - .'. Any statement that rewrites it, scans it, or holds a lock on it is felt for as ' - .'long as that takes — how long, and what is blocked meanwhile, depends on the ' - .'engine and version rather than on anything visible in the migration.', + message: 'ALTER on a large table: '.($table ?? ''), + explanation: 'The table is above the size Difflock is configured to treat as large. Any ' + .'statement that rewrites it, scans it, or holds a lock on it is felt for as long as ' + .'that takes — how long, and what is blocked meanwhile, depends on the engine and ' + .'version rather than on anything visible in the migration.', suggestion: 'Read the other findings for this migration with the size in mind, and ' .'consider running the statement outside the deploy window.', subject: $table, subjectType: Subject::Table, reversible: $context->reversible(), + context: $context->database->describeSize($table) + .($bytes === null ? '' : ' · about '.Bytes::human($bytes)), )]; } } diff --git a/src/Migration/Rules/UnindexedForeignKeyRule.php b/src/Migration/Rules/UnindexedForeignKeyRule.php index 02ac276..0199d8b 100644 --- a/src/Migration/Rules/UnindexedForeignKeyRule.php +++ b/src/Migration/Rules/UnindexedForeignKeyRule.php @@ -138,12 +138,6 @@ private function finding( .'created for the column a foreign key points from; on MySQL and MariaDB one is created ' .'automatically and this finding does not apply.'; - $size = $context->database->describeSize($context->tableName()); - - if ($size !== null) { - $explanation .= ' The table holds '.$size.'.'; - } - return $context->finding( rule: $this->identifier(), risk: $this->risk($context, $known), @@ -155,6 +149,7 @@ private function finding( subjectType: Subject::Column, reversible: $context->reversible(), operation: $operation, + context: $context->database->describeSize($context->tableName()), ); } diff --git a/tests/Feature/CommandsTest.php b/tests/Feature/CommandsTest.php index 9ac5905..8e44c83 100644 --- a/tests/Feature/CommandsTest.php +++ b/tests/Feature/CommandsTest.php @@ -25,11 +25,12 @@ it('reports findings and fails at the configured threshold', function (): void { [$exit, $output] = runCommand('difflock:lint'); + // Summary by default: the worst finding, the level tally, and where the rest is. expect($exit)->toBe(1) ->and($output)->toContain('DROP COLUMN users.legacy_token') ->toContain('CRITICAL') ->toContain('2026_08_10_120000_remove_legacy_token') - ->toContain('Risk'); + ->toContain('difflock:lint -v'); }); it('passes when nothing reaches the threshold', function (): void { diff --git a/tests/Feature/ReportRenderingTest.php b/tests/Feature/ReportRenderingTest.php index fca365a..c7d1dab 100644 --- a/tests/Feature/ReportRenderingTest.php +++ b/tests/Feature/ReportRenderingTest.php @@ -48,21 +48,26 @@ function render(MigrationReport $report, bool $verbose = false): string it('prints a shared explanation once, however many findings share it', function (): void { $findings = array_map(shared(...), range(1, 124)); - $output = render(reportOf(...$findings)); + $output = render(reportOf(...$findings), verbose: true); expect(substr_count($output, 'A cascading delete removes child rows'))->toBe(1) ->and(substr_count($output, 'Consider restrictOnDelete'))->toBe(1) ->and($output)->toContain('124 findings'); }); -it('lists a few occurrences and counts the rest', function (): void { +it('bounds the summary however many findings there are', function (): void { + $lines = substr_count(render(reportOf(...array_map(shared(...), range(1, 200)))), "\n"); + + expect($lines)->toBeLessThan(25); +}); + +it('points at the ways to see more', function (): void { $output = render(reportOf(...array_map(shared(...), range(1, 124)))); - expect($output)->toContain('table_1.user_id') - ->and($output)->toContain('table_3.user_id') - ->and($output)->not->toContain('table_4.user_id') - ->and($output)->toContain('121 more') - ->and($output)->toContain('--rule=foreign-key'); + expect($output)->toContain('-v') + ->toContain('--rule=') + ->toContain('difflock:report') + ->toContain('124 findings'); }); it('lists every occurrence when asked to be verbose', function (): void { @@ -86,7 +91,7 @@ function render(MigrationReport $report, bool $verbose = false): string 'The table holds 12 rows.', table: 'a', line: 1), new MigrationFinding('drop-column', RiskLevel::Critical, 'm2', 'DROP COLUMN c.d', 'The table holds 8,000,000 rows.', table: 'c', line: 2), - )); + ), verbose: true); expect($output)->toContain('12 rows') ->and($output)->toContain('8,000,000 rows'); @@ -111,7 +116,7 @@ function render(MigrationReport $report, bool $verbose = false): string [shared(2)], ); - expect(render($report))->toContain('Risk') + expect(render($report, verbose: true))->toContain('Risk') ->toContain('1 previously accepted finding'); }); diff --git a/tests/Unit/IndexEvidenceTest.php b/tests/Unit/IndexEvidenceTest.php index 408d151..dd37fd8 100644 --- a/tests/Unit/IndexEvidenceTest.php +++ b/tests/Unit/IndexEvidenceTest.php @@ -42,8 +42,8 @@ function usageContext(string $body, array $tables, array $scans = [], ?int $days )); expect($findings[0]->risk)->toBe(RiskLevel::Low) - ->and($findings[0]->explanation)->toContain('read no times') - ->and($findings[0]->explanation)->toContain('274 days') + ->and($findings[0]->explanation)->toContain('no reads of this index') + ->and($findings[0]->context)->toContain('274 days') ->and($findings[0]->suggestion)->toContain('safe to drop'); }); @@ -53,7 +53,7 @@ function usageContext(string $body, array $tables, array $scans = [], ?int $days )); expect($findings[0]->risk)->toBe(RiskLevel::High) - ->and($findings[0]->explanation)->toContain('2,100,000 times') + ->and($findings[0]->context)->toContain('2,100,000 reads') ->and($findings[0]->suggestion)->toContain('Find what reads it'); }); diff --git a/tests/Unit/RulesTest.php b/tests/Unit/RulesTest.php index 8549fae..75ea299 100644 --- a/tests/Unit/RulesTest.php +++ b/tests/Unit/RulesTest.php @@ -47,19 +47,19 @@ function run(MigrationRule $rule, MigrationContext $context): array ['orders' => 8_421_392], )); - expect($findings[0]->explanation)->toContain('8,421,392 rows'); + expect($findings[0]->context)->toContain('8,421,392 rows'); }); it('says when the database could not be reached', function (): void { $context = contextFor(parseUp("Schema::drop('orders');"), available: false); - expect(run(new DropTableRule, $context)[0]->explanation) - ->toContain('could not be reached'); + expect(run(new DropTableRule, $context)[0]->context) + ->toContain('unreachable'); }); it('says when the table is not on the inspected database', function (): void { - expect(run(new DropTableRule, ruleContext("Schema::drop('orders');"))[0]->explanation) - ->toContain('does not exist on the inspected database'); + expect(run(new DropTableRule, ruleContext("Schema::drop('orders');"))[0]->context) + ->toContain('not present on the inspected database'); }); it('reports dropping every table', function (): void { @@ -128,7 +128,7 @@ function run(MigrationRule $rule, MigrationContext $context): array ['orders' => 12], )); - expect($findings[0]->explanation) + expect($findings[0]->context) ->toContain('orders_customer_id_index') ->toContain('orders_customer_id_foreign') ->toContain('12 rows'); @@ -360,7 +360,7 @@ function run(MigrationRule $rule, MigrationContext $context): array expect($findings[0]->risk)->toBe(RiskLevel::High) ->and($findings[0]->message)->toBe('ADD NOT NULL COLUMN users.status with no default') - ->and($findings[0]->explanation)->toContain('4,000 rows'); + ->and($findings[0]->context)->toContain('4,000 rows'); }); it('is low on an empty table', function (): void { @@ -454,7 +454,7 @@ function run(MigrationRule $rule, MigrationContext $context): array "Schema::table('orders', fn (Blueprint \$t) => \$t->index('customer_id'));", )); - expect($findings[0]->explanation)->toContain('could not be determined') + expect($findings[0]->context)->toContain('could not be determined') ->and($findings[0]->explanation)->toContain('depends on the database engine'); }); @@ -503,7 +503,7 @@ function run(MigrationRule $rule, MigrationContext $context): array ['users' => 10], )); - expect($findings[0]->explanation)->toContain('covers (first, last)'); + expect($findings[0]->context)->toContain('covers (first, last)'); }); }); @@ -538,7 +538,7 @@ function run(MigrationRule $rule, MigrationContext $context): array expect($findings)->toHaveCount(1) ->and($findings[0]->risk)->toBe(RiskLevel::Medium) - ->and($findings[0]->explanation)->toContain('900 rows'); + ->and($findings[0]->context)->toContain('900 rows'); }); it('is low for adding a constraint to an empty table', function (): void { @@ -595,7 +595,7 @@ function run(MigrationRule $rule, MigrationContext $context): array )); expect($findings[0]->risk)->toBe(RiskLevel::Medium) - ->and($findings[0]->message)->toContain('8,421,392 rows') + ->and($findings[0]->context)->toContain('8,421,392 rows') ->and($findings[0]->explanation)->toContain('depends on the'); }); diff --git a/tests/Unit/SnapshotTest.php b/tests/Unit/SnapshotTest.php index 4676b89..77c7ecd 100644 --- a/tests/Unit/SnapshotTest.php +++ b/tests/Unit/SnapshotTest.php @@ -87,7 +87,7 @@ ['users' => 1], )); - expect($findings[0]->explanation) + expect($findings[0]->context) ->toContain('users_email_index and users_email_unique') ->toContain('1 row'); });