test: directive composition pins + CHECK-rename TODO by winebarrel · Pull Request #90 · winebarrel/myschema

winebarrel · 2026-05-05T15:32:35Z

Summary

Fifth and final item from the PR #84 postmortem audit (TODO.md "Medium — test coverage audit", item 5). Inline -- myschema:renamed-from directives across all four supported kinds (column / index / FK / CHECK) in one CREATE TABLE body must route to their own buckets without competing.

TestExtractInlineRenamesAllKindsInOneTable (parser): puts a rename of each kind in one body and asserts each routes to its matching map (Columns / Indexes / ForeignKeys / Constraints), with length checks pinning "exactly one" per bucket so a duplicating bug fails too.
TestExtractInlineRenamesColumnAndIndexShareName (parser): a column named email AND an index also named email (UNIQUE KEY email (email) — explicitly named, just colliding with the column) can both carry rename directives without confusing the line classifier.
testdata/apply/inline_rename_all_kinds.yml exercises the apply → re-plan-empty round-trip (4 kinds renamed simultaneously). The YAML harness applies the desired SQL, asserts the emitted DDL, then re-plans against the post-apply catalog and asserts an empty plan via verify_no_drift: true.

A real bug surfaced and went into TODO

While composing the YAML fixture, hit a cross-cutting issue:

When a CHECK constraint references the renamed column, MySQL rejects the RENAME COLUMN step (Error 3959 Check constraint X uses column Y, hence column cannot be dropped or renamed) because the diff layer's pass-order emits column renames before CHECK drops.

The fixture works around it by referencing a different column in the CHECK. The underlying bug is now logged as a new TODO entry under "Medium — silent diffs / fidelity gaps" so a follow-up can route CHECK drops into a dedicated bucket the way FK drops already are.

This PR closes the test-coverage audit started after PR #84.

Test plan

All three new tests pass.
`make lint` clean.
`make test` all six packages green.
End-to-end YAML round-trip closes (`verify_no_drift: true`).

🤖 Generated with Claude Code

codecov · 2026-05-05T15:35:12Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.90%. Comparing base (ad8f5ac) to head (66248fb).

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #90   +/-   ##
=======================================
  Coverage   86.90%   86.90%           
=======================================
  Files          30       30           
  Lines        3277     3277           
=======================================
  Hits         2848     2848           
  Misses        263      263           
  Partials      166      166

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Adds coverage for inline -- myschema:renamed-from directive composition across supported CREATE TABLE body targets, and records a newly discovered CHECK/column-rename ordering limitation in the project TODOs. This fits the parser/apply regression suite by tightening rename-directive behavior around multi-kind tables.

Changes:

Adds parser tests for multi-kind inline rename classification and for a column/index name-collision case.
Adds an apply YAML fixture covering simultaneous column, index, foreign-key, and CHECK renames in one table.
Documents the uncovered CHECK-constraint/column-rename ordering bug in TODO.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
`TODO.md`	Adds a new medium-priority TODO describing the CHECK-before-rename ordering bug.
`testdata/apply/inline_rename_all_kinds.yml`	Adds an integration fixture for simultaneous inline rename directives across all supported kinds.
`parser/directive_test.go`	Adds parser regression tests for multi-kind directive routing and shared column/index names.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- TestExtractInlineRenamesAllKindsInOneTable: add Len(...) checks per bucket so a duplicating-classifier bug (directive routed to its own bucket *and* an unrelated one) fails the test instead of passing on "expected mappings present, didn't look at the rest." - TestExtractInlineRenamesColumnAndIndexShareName: drop the "auto-named index" wording in the doc — the fixture spells the index name out (`UNIQUE KEY email (email)`); MySQL only auto-generates an index name when you omit it. The point is the column / index *name collision*, not how the index got that name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Fifth and final item from the PR #84 postmortem audit. Inline `-- myschema:renamed-from` directives across all four supported kinds (column / index / FK / CHECK) in one CREATE TABLE body must route to their own buckets without competing. - TestExtractInlineRenamesAllKindsInOneTable (parser) puts a rename of each kind in one body and asserts each routes to its matching map (Columns / Indexes / ForeignKeys / Constraints). - TestExtractInlineRenamesColumnAndIndexShareName (parser): a column named `email` AND a UNIQUE KEY auto-named `email` (the default when the user writes `UNIQUE KEY email (email)`) can both carry rename directives without confusing the line classifier. - testdata/apply/inline_rename_all_kinds.yml exercises the full plan → apply → re-plan-empty round-trip. While composing the YAML fixture, hit a real cross-cutting bug: when a CHECK constraint references the renamed column, MySQL rejects the RENAME COLUMN step (`Error 3959 Check constraint X uses column Y, hence column cannot be dropped or renamed`) because the diff layer's pass-order emits column renames *before* CHECK drops. The fixture works around it by referencing a different column in the CHECK; the underlying bug is logged as a new TODO entry under "Medium — silent diffs / fidelity gaps" so a follow-up can route CHECK drops into a dedicated bucket the way FK drops already are. This closes the test-coverage audit started after PR #84. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- TestExtractInlineRenamesAllKindsInOneTable: add Len(...) checks per bucket so a duplicating-classifier bug (directive routed to its own bucket *and* an unrelated one) fails the test instead of passing on "expected mappings present, didn't look at the rest." - TestExtractInlineRenamesColumnAndIndexShareName: drop the "auto-named index" wording in the doc — the fixture spells the index name out (`UNIQUE KEY email (email)`); MySQL only auto-generates an index name when you omit it. The point is the column / index *name collision*, not how the index got that name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

PR #90 reviewer pointed out that inline_rename_all_kinds.yml proves the column / index inline `-- myschema:renamed-from` directives are recognized (the emitted DDL shifts from DROP+ADD to RENAME COLUMN / RENAME INDEX), but the FK and CHECK directives are invisible in `applied`: MySQL has no RENAME FOREIGN KEY / RENAME CONSTRAINT, so diffForeignKeys / diffConstraints emit the same DROP+ADD whether RenameFrom is set or not. A regression that drops inline-FK or inline-CHECK directive recognition would keep that fixture green. Add typo-guard error fixtures so the apply test catches that regression: passing a non-existent old name in the directive must fail the apply with the source-not-found typo guard. With these in place, dropping recognition would silently ignore the directive and the apply would succeed with a DROP+ADD instead of erroring. - error_inline_rename_fk_typo.yml (FK directive typo guard) - error_inline_rename_check_typo.yml (CHECK directive typo guard) - inline_rename_all_kinds.yml: comment now states which kinds are observable at apply layer and points to the new error fixtures + the parser-level bucket-routing test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Five of the six items from the PR #84 postmortem audit landed: - composed-state catalog tests → PR #86 - kitchen-sink round-trip → PR #87 - emitter ordering pins → PR #88 - diff cross-cutting → PR #89 - directive composition → PR #90 Drop the closed items. Keep `parseCreateTable` column-loop interactions under "Medium — test coverage gaps" — that one was scoped out of the kitchen-sink because the open shapes (inline `UNIQUE × DEFAULT`, inline `REFERENCES × NOT NULL × ON UPDATE`, inline `PK × column-level CHECK`) need direct parser unit tests, not a round-trip fixture. The kitchen-sink uses `email VARCHAR NOT NULL UNIQUE` (no DEFAULT), an explicit named CONSTRAINT FK (not inline REFERENCES), and a table-level CHECK (not the column-level shape that exercises the vitess promotion path), so the gap is real. The "Medium — silent diffs / fidelity gaps" section keeps the real open items, including the three new ones surfaced by the audit itself (REFERENCES auto-name, vitess-keyword identifier drift, CHECK referencing renamed column). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings May 5, 2026 15:32

Copilot started reviewing on behalf of winebarrel May 5, 2026 15:33 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

Comment thread parser/directive_test.go

Comment thread parser/directive_test.go Outdated

Comment thread testdata/apply/inline_rename_all_kinds.yml

winebarrel requested a review from Copilot May 5, 2026 15:57

Copilot started reviewing on behalf of winebarrel May 5, 2026 15:59 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

Comment thread testdata/apply/inline_rename_all_kinds.yml

winebarrel and others added 3 commits May 6, 2026 09:06

winebarrel force-pushed the test-directive-composition branch from 7ac83e0 to 66248fb Compare May 6, 2026 00:08

winebarrel requested a review from Copilot May 6, 2026 00:09

Copilot started reviewing on behalf of winebarrel May 6, 2026 00:10 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

winebarrel merged commit b668b6e into main May 6, 2026
9 checks passed

winebarrel deleted the test-directive-composition branch May 6, 2026 00:12

winebarrel mentioned this pull request May 6, 2026

docs: trim completed test-coverage-audit items from TODO #91

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: directive composition pins + CHECK-rename TODO#90

test: directive composition pins + CHECK-rename TODO#90
winebarrel merged 3 commits into
mainfrom
test-directive-composition

winebarrel commented May 5, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

winebarrel commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

A real bug surfaced and went into TODO

Test plan

Uh oh!

codecov Bot commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

winebarrel commented May 5, 2026 •

edited

Loading

codecov Bot commented May 5, 2026 •

edited

Loading