Skip to content

test: fill regression-coverage gaps for in-scope features#58

Merged
winebarrel merged 6 commits into
mainfrom
fill-regression-gaps
May 4, 2026
Merged

test: fill regression-coverage gaps for in-scope features#58
winebarrel merged 6 commits into
mainfrom
fill-regression-gaps

Conversation

@winebarrel

Copy link
Copy Markdown
Owner

Summary

Spec audit pass — survey AGENTS.md "In scope" against the existing fixture set, identify shapes that are documented as supported but not pinned end-to-end, and add minimal apply + verify_no_drift fixtures for each.

No behaviour change. All new fixtures pin the current output of make test so they will fail if a future commit accidentally regresses any of these shapes — that's the point.

26 fixtures total (24 apply + 2 plan), +733 lines.

Coverage delta

Index shapes (7)

  • add_index_desc.ymlKEY (col DESC) round-trip.
  • add_index_invisible.ymlKEY ... INVISIBLE round-trip.
  • add_index_with_prefix_length.ymlKEY (col(N)) on VARCHAR.
  • add_index_on_blob_with_prefix_length.yml — same on BLOB (where MySQL requires the prefix length).
  • add_multi_column_unique_index.yml — column ordering preserved through STATISTICS.SEQ_IN_INDEX.
  • add_fulltext_index.ymlCREATE FULLTEXT INDEX shape (one of the 4 CREATE INDEX variants appendAlterHints handles).
  • modify_index_columns.yml — index column-list change must DROP+CREATE in that order.

Column shapes (7)

  • add_column_on_update_current_timestamp.ymlON UPDATE CURRENT_TIMESTAMP.
  • add_column_default_current_timestamp_only.ymlDEFAULT CURRENT_TIMESTAMP without ON UPDATE.
  • add_column_with_comment.yml — column-level COMMENT.
  • add_generated_column_stored.ymlGENERATED ALWAYS AS (...) STORED.
  • add_enum_column.yml — basic ENUM column round-trip (element-list diff is a known TODO; this only pins the CREATE shape).
  • add_json_column.yml — JSON column round-trip.
  • add_decimal_column.ymlDECIMAL(M, D) precision/scale.

Foreign key shapes (5)

  • add_fk_on_delete_cascade.yml — explicit ON DELETE CASCADE.
  • add_fk_on_update_cascade.yml — explicit ON UPDATE CASCADE.
  • add_self_referential_fk.yml — same-table FK (employees.manager_id → employees.id).
  • add_multi_column_fk.yml — composite FK column list ordering through KEY_COLUMN_USAGE.ORDINAL_POSITION.
  • cross_table_fk_ripple_on_column_rename.yml — parent-side column rename rewrites child-side FK RefCols (rewriteCrossTableFKRefCols) so the child FK doesn't diff as DROP+ADD.

Constraint / view shapes (4)

  • change_primary_key.yml — single-column → composite PK (DROP PRIMARY KEY then ADD PRIMARY KEY ordering).
  • modify_view_body.yml — view body change emits CREATE OR REPLACE VIEW and converges.
  • create_view_with_column_aliases.yml — view's explicit column-alias list (uid, uname).
  • add_column_and_dependent_index.yml — ADD COLUMN + CREATE INDEX in same plan; CREATE INDEX must come after ADD COLUMN.

CLI / filter (3)

  • add_column_with_alter_hints_instant.yml--alter-algorithm=INSTANT on a column-level ALTER (trailing-comma splice path; complement to the existing partition-op leading-position splice fixtures).
  • exclude_filter.yml--exclude=<table> companion to the existing include_filter.yml (only-exact-name include was pinned, the exclude direction wasn't).
  • include_filter_glob.yml--include=tmp_* glob pattern (existing include_filter.yml only used an exact name).

Untracked spec gaps surfaced during the audit

Three probes during the audit found shapes where the round-trip doesn't actually converge today. Per the no-spec-change scope of this branch I removed those fixtures rather than pinning broken behaviour. Listing them here so we can decide separately whether to fix the diff layer or document the limitation:

  • CHECK constraint NOT ENFORCED — myschema doesn't preserve the NOT ENFORCED keyword. verify_no_drift fails because re-plan emits DROP CHECK chk + ADD CONSTRAINT chk CHECK (...) repeatedly.
  • TIMESTAMP NULL DEFAULT NULL — same drift loop; re-plan repeatedly emits MODIFY COLUMN ts timestamp DEFAULT null.
  • Table-level COMMENT='…' changes — not detected as a diff at all (silent; the table-level COMMENT difference is invisible to the diff layer).

Test plan

  • make lint
  • make test (8.0)
  • make test-scenario

Copilot AI review requested due to automatic review settings May 4, 2026 07:09
winebarrel added a commit that referenced this pull request May 4, 2026
PR #58's spec audit found three shapes that look like they
should round-trip end-to-end but don't (probed and confirmed).
Fixture additions were withheld from PR #58 since pinning
broken behaviour without a fix or documented limitation
violates the no-spec-change scope of that branch. Land them
as TODO entries so they're not lost.

All three live under "Medium — silent diffs / fidelity gaps"
alongside the existing ENUM/SET and FK-to-other-DB items
(same flavour: catalog has the data, but the model / diff
layer drops or mishandles it):

- CHECK constraint `NOT ENFORCED` — drift loop after first
  apply.
- TIMESTAMP NULL DEFAULT NULL — drift loop emitting
  `MODIFY COLUMN ts timestamp DEFAULT null` repeatedly.
- Table-level `COMMENT='…'` — silent (no diff at all).

Each entry documents what the diff layer would need to learn
(or, alternatively, where to document the limitation in
CAVEATS.md) so a future contributor can pick one up without
re-doing the probe.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds regression fixtures to pin current myschema output for several documented “in-scope” schema shapes (indexes/columns/FKs/views/CLI filters), ensuring make test fails on future accidental drift without introducing behavior changes.

Changes:

  • Adds apply fixtures (with verify_no_drift) for additional supported DDL shapes: index variants, timestamp defaults, generated columns, FK actions, PK changes, and view modifications.
  • Adds plan fixtures to cover --include glob matching and --exclude table filtering behavior.
  • Expands fixture coverage for ordering-sensitive operations (e.g., DROP+CREATE index, ADD COLUMN before dependent CREATE INDEX).

Reviewed changes

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

Show a summary per file
File Description
testdata/plan/include_filter_glob.yml Pins --include=tmp_* glob behavior and disallowed drops output.
testdata/plan/exclude_filter.yml Pins --exclude=audit_log behavior (excluded tables invisible to diff/plan).
testdata/apply/modify_view_body.yml Pins view body changes emitting CREATE OR REPLACE VIEW + no-drift.
testdata/apply/modify_index_columns.yml Pins index column-list change as DROP INDEX then CREATE INDEX + no-drift.
testdata/apply/cross_table_fk_ripple_on_column_rename.yml Pins parent-side rename ripple to cross-table FK refcols + no-drift.
testdata/apply/create_view_with_column_aliases.yml Pins views with explicit column-alias list + no-drift.
testdata/apply/change_primary_key.yml Pins PK change ordering (DROP PK then ADD PK) + no-drift.
testdata/apply/add_self_referential_fk.yml Pins same-table FK creation + no-drift.
testdata/apply/add_multi_column_unique_index.yml Pins multi-column UNIQUE index column ordering round-trip + no-drift.
testdata/apply/add_multi_column_fk.yml Pins composite FK column ordering round-trip + no-drift.
testdata/apply/add_json_column.yml Pins JSON column add round-trip + no-drift.
testdata/apply/add_index_with_prefix_length.yml Pins prefix-length index on VARCHAR + no-drift.
testdata/apply/add_index_on_blob_with_prefix_length.yml Pins required prefix-length index on BLOB + no-drift.
testdata/apply/add_index_invisible.yml Pins INVISIBLE secondary index round-trip + no-drift.
testdata/apply/add_index_desc.yml Pins DESC index column modifier round-trip + no-drift.
testdata/apply/add_generated_column_stored.yml Pins generated STORED column round-trip + no-drift.
testdata/apply/add_fulltext_index.yml Pins FULLTEXT index creation path + no-drift.
testdata/apply/add_fk_on_update_cascade.yml Pins FK with ON UPDATE CASCADE round-trip + no-drift.
testdata/apply/add_fk_on_delete_cascade.yml Pins FK with ON DELETE CASCADE round-trip + no-drift.
testdata/apply/add_enum_column.yml Pins basic ENUM column shape round-trip + no-drift.
testdata/apply/add_decimal_column.yml Pins DECIMAL precision/scale formatting round-trip + no-drift.
testdata/apply/add_column_with_comment.yml Pins column COMMENT clause round-trip + no-drift.
testdata/apply/add_column_with_alter_hints_instant.yml Pins alter-hint splice for column-level ADD COLUMN (INSTANT) + no-drift.
testdata/apply/add_column_on_update_current_timestamp.yml Pins ON UPDATE CURRENT_TIMESTAMP column round-trip + no-drift.
testdata/apply/add_column_default_current_timestamp_only.yml Pins DEFAULT CURRENT_TIMESTAMP (no ON UPDATE) round-trip + no-drift.
testdata/apply/add_column_and_dependent_index.yml Pins statement ordering: ADD COLUMN before dependent CREATE INDEX + no-drift.

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

Comment thread testdata/apply/add_column_with_alter_hints_instant.yml
@codecov

codecov Bot commented May 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.16%. Comparing base (a02a503) to head (26167af).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #58   +/-   ##
=======================================
  Coverage   71.16%   71.16%           
=======================================
  Files          28       28           
  Lines        3062     3062           
=======================================
  Hits         2179     2179           
  Misses        647      647           
  Partials      236      236           

☔ 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.

winebarrel added a commit that referenced this pull request May 4, 2026
Fixture header for `add_column_with_alter_hints_instant.yml`
claimed coverage of `--alter-algorithm=INSTANT` AND
`--alter-lock=NONE`, but only `alter_algorithm: INSTANT` is
set on the fixture (and the expected `applied` DDL only
emits `ALGORITHM=INSTANT`). Updated the header to drop the
`--alter-lock=NONE` claim and added a note about why the
combination isn't pinned: MySQL rejects
`ALGORITHM=INSTANT` with any explicit `LOCK=…` (Error 1221
"Incorrect usage of ALGORITHM=INSTANT and LOCK=…"), so
combining them in this fixture would be untestable. Pointed
at the existing plan fixture
`testdata/plan/alter_hints.yml` and the partition-op apply
fixtures as the lock-splice coverage instead.

No behaviour / test change — comment-only.
@winebarrel winebarrel requested a review from Copilot May 4, 2026 07:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


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

Comment thread TODO.md Outdated
winebarrel added a commit that referenced this pull request May 4, 2026
Fix the IS column reference in the CHECK NOT ENFORCED TODO entry —
information_schema.CHECK_CONSTRAINTS does not carry ENFORCED; the
flag lives on TABLE_CONSTRAINTS, which catalog/tables.go already
joins to CHECK_CONSTRAINTS for exactly this reason.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@winebarrel winebarrel requested a review from Copilot May 4, 2026 07:27
winebarrel and others added 4 commits May 4, 2026 16:30
Spec audit pass — survey AGENTS.md "In scope" against the
existing fixture set, identify shapes that are documented as
supported but not pinned end-to-end, and add minimal apply +
verify_no_drift fixtures for each. All new fixtures pass on
the current main with no behaviour change.

24 apply fixtures (all `verify_no_drift: true` — strongest
form: pins generated SQL string, MySQL accepting it, AND
catalog round-tripping back without spurious diff):

  Index shapes:
  - add_index_desc.yml — `KEY (col DESC)` round-trip.
  - add_index_invisible.yml — `KEY ... INVISIBLE` round-trip.
  - add_index_with_prefix_length.yml — `KEY (col(N))` on
    VARCHAR.
  - add_index_on_blob_with_prefix_length.yml — same on BLOB
    (where MySQL *requires* the prefix length).
  - add_multi_column_unique_index.yml — column ordering
    preserved through `STATISTICS.SEQ_IN_INDEX`.
  - add_fulltext_index.yml — `CREATE FULLTEXT INDEX` shape
    (one of the 4 CREATE INDEX variants `appendAlterHints`
    handles).
  - modify_index_columns.yml — index column-list change must
    DROP+CREATE in that order.

  Column shapes:
  - add_column_on_update_current_timestamp.yml — `ON UPDATE
    CURRENT_TIMESTAMP`.
  - add_column_default_current_timestamp_only.yml — `DEFAULT
    CURRENT_TIMESTAMP` without ON UPDATE.
  - add_column_with_comment.yml — column-level COMMENT.
  - add_generated_column_stored.yml — `GENERATED ALWAYS AS
    (...) STORED`.
  - add_enum_column.yml — basic ENUM column round-trip
    (element-list diff is a known TODO; this only pins the
    CREATE shape).
  - add_json_column.yml — JSON column round-trip.
  - add_decimal_column.yml — `DECIMAL(M, D)` precision/scale.

  Foreign key shapes:
  - add_fk_on_delete_cascade.yml — explicit ON DELETE
    CASCADE.
  - add_fk_on_update_cascade.yml — explicit ON UPDATE
    CASCADE.
  - add_self_referential_fk.yml — same-table FK
    (employee.manager_id → employee.id).
  - add_multi_column_fk.yml — composite FK column list
    ordering through `KEY_COLUMN_USAGE.ORDINAL_POSITION`.
  - cross_table_fk_ripple_on_column_rename.yml — parent-side
    column rename re-writes child-side FK RefCols
    (`rewriteCrossTableFKRefCols`) so the child FK doesn't
    diff as DROP+ADD.

  Constraint shapes:
  - change_primary_key.yml — single-column → composite PK
    (DROP PRIMARY KEY then ADD PRIMARY KEY ordering).

  View shapes:
  - modify_view_body.yml — view body change emits
    `CREATE OR REPLACE VIEW` and converges.
  - create_view_with_column_aliases.yml — view's explicit
    column-alias list `(uid, uname)`.

  Combined operations:
  - add_column_and_dependent_index.yml — ADD COLUMN + CREATE
    INDEX in same plan; CREATE INDEX must come after ADD
    COLUMN.
  - add_column_with_alter_hints_instant.yml — `--alter-
    algorithm=INSTANT` on a column-level ALTER (trailing-
    comma splice path, complement to the partition-op
    leading-position splice path).

2 plan fixtures (filter coverage):

  - exclude_filter.yml — `--exclude=<table>` companion to
    the existing `include_filter.yml` (only-exact-name case
    was pinned, the exclude direction wasn't).
  - include_filter_glob.yml — `--include=tmp_*` glob
    pattern (existing `include_filter.yml` only used an
    exact name).

Two probes during the audit surfaced known untracked spec
gaps where the round-trip doesn't actually converge today
(CHECK constraint `NOT ENFORCED` and `TIMESTAMP NULL
DEFAULT NULL`). Per the "no spec changes" scope of this
branch I removed those fixtures; they're worth a separate
follow-up to either fix the diff layer or document the
limitation.

Likewise table-level `COMMENT='…'` changes aren't tracked
by the diff today — fixture removed, would need a separate
spec discussion.

`make lint` clean, full `go test -p 1 ./...` green, all 26
new fixtures pass.
PR #58's spec audit found three shapes that look like they
should round-trip end-to-end but don't (probed and confirmed).
Fixture additions were withheld from PR #58 since pinning
broken behaviour without a fix or documented limitation
violates the no-spec-change scope of that branch. Land them
as TODO entries so they're not lost.

All three live under "Medium — silent diffs / fidelity gaps"
alongside the existing ENUM/SET and FK-to-other-DB items
(same flavour: catalog has the data, but the model / diff
layer drops or mishandles it):

- CHECK constraint `NOT ENFORCED` — drift loop after first
  apply.
- TIMESTAMP NULL DEFAULT NULL — drift loop emitting
  `MODIFY COLUMN ts timestamp DEFAULT null` repeatedly.
- Table-level `COMMENT='…'` — silent (no diff at all).

Each entry documents what the diff layer would need to learn
(or, alternatively, where to document the limitation in
CAVEATS.md) so a future contributor can pick one up without
re-doing the probe.
Fixture header for `add_column_with_alter_hints_instant.yml`
claimed coverage of `--alter-algorithm=INSTANT` AND
`--alter-lock=NONE`, but only `alter_algorithm: INSTANT` is
set on the fixture (and the expected `applied` DDL only
emits `ALGORITHM=INSTANT`). Updated the header to drop the
`--alter-lock=NONE` claim and added a note about why the
combination isn't pinned: MySQL rejects
`ALGORITHM=INSTANT` with any explicit `LOCK=…` (Error 1221
"Incorrect usage of ALGORITHM=INSTANT and LOCK=…"), so
combining them in this fixture would be untestable. Pointed
at the existing plan fixture
`testdata/plan/alter_hints.yml` and the partition-op apply
fixtures as the lock-splice coverage instead.

No behaviour / test change — comment-only.
Fix the IS column reference in the CHECK NOT ENFORCED TODO entry —
information_schema.CHECK_CONSTRAINTS does not carry ENFORCED; the
flag lives on TABLE_CONSTRAINTS, which catalog/tables.go already
joins to CHECK_CONSTRAINTS for exactly this reason.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@winebarrel winebarrel force-pushed the fill-regression-gaps branch from 9576788 to 37bc702 Compare May 4, 2026 07:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


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

Comment thread TODO.md Outdated
Grammar: "apply once" → "applies once".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


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

Comment thread TODO.md Outdated
Comment thread TODO.md Outdated
Comment thread testdata/apply/add_json_column.yml Outdated
- TODO: rewrite the CHECK NOT ENFORCED entry to point at the real
  gap (catalog/tables.go hard-codes Enforced=true) — model and diff
  layers already plumb the flag.
- TODO: rewrite the table-level COMMENT entry to point at the diff
  layer (Engine/Comment intentionally not diffed) — model and
  catalog already plumb the field.
- testdata/apply/add_json_column.yml: COLUMN_TYPE, not DATA_TYPE,
  matches what catalog/tables.go loadColumns actually selects.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 27 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.

@winebarrel winebarrel merged commit de8909a into main May 4, 2026
9 checks passed
@winebarrel winebarrel deleted the fill-regression-gaps branch May 4, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants