Skip to content

docs: log the PR #84 postmortem test-coverage audit to TODO#85

Merged
winebarrel merged 1 commit into
mainfrom
docs-test-coverage-audit
May 5, 2026
Merged

docs: log the PR #84 postmortem test-coverage audit to TODO#85
winebarrel merged 1 commit into
mainfrom
docs-test-coverage-audit

Conversation

@winebarrel

Copy link
Copy Markdown
Owner

Summary

PR #84's review caught a real bug (catalog loadColumns leaked the trailing INVISIBLE token into Column.OnUpdate when both attributes sat on the same EXTRA row) that the original test suite missed because every existing test exercised attributes in isolation. The bug class — composed-state parsers where multiple branches read/write the same shared string — has more potential sites in the codebase. Audit those now so the next contributor working in the area can pick them up.

Adds a "Medium — test coverage audit" section to TODO.md with six prioritised items:

  • Composed-state EXTRA / option parsing matrix tests for catalog/loadColumns and loadIndexes. Highest payoff — directly targets the same bug class as PR parser: support column-level INVISIBLE (MySQL 8.0+) #84.
  • parseCreateTable column-loop interactions — inline PK / UNIQUE / REFERENCES combined with NOT NULL / DEFAULT / ON UPDATE / CHECK.
  • Index-comparison emitter ordering pins for columnDefSQL, indexInlineSQL, constraintInlineSQL (PR parser: support column-level INVISIBLE (MySQL 8.0+) #84 pinned only INVISIBLE's slot; the other slots stay Contains-only).
  • Diff cross-cutting / order-sensitivity — column rename + drop with same final name, index rename + DROP INDEX same plan, etc.
  • Directive composition for inline -- myschema:renamed-from across columns × indexes × constraints in one CREATE TABLE.
  • "Kitchen-sink" round-trip fixture — one YAML that exercises every supported attribute together with verify_no_drift: true. Highest single-test ROI.

Each item lists the surface, the missing combination, and a realistic-harm sketch so the work scope is clear without re-deriving context. Combined estimate: 2.5–3 days; recommended order is catalog matrix → kitchen-sink fixture → emitter ordering → diff → directive composition.

Test plan

  • No code changes; doc-only edit.

🤖 Generated with Claude Code

PR #84's review caught a real bug (catalog `loadColumns` leaked the
trailing `INVISIBLE` token into `Column.OnUpdate` when both
attributes sat on the same EXTRA row) that the original test suite
missed because every existing test exercised attributes in
isolation. The bug class — composed-state parsers where multiple
branches read/write the same shared string — has more potential
sites in the codebase. Audit those now so the next contributor
working in the area can pick them up.

Adds a "Medium — test coverage audit" section between the existing
"Medium — silent diffs / fidelity gaps" (data-model fidelity) and
"Low — tests / docs / release" (process), with six prioritised
items:

- Composed-state EXTRA / option parsing matrix tests for
  `catalog/loadColumns` and `loadIndexes`.
- `parseCreateTable` column-loop interactions (inline PK / UNIQUE /
  REFERENCES × column attributes).
- Index-comparison emitter ordering pins for `columnDefSQL`,
  `indexInlineSQL`, `constraintInlineSQL` (PR #84 pinned only
  INVISIBLE's slot).
- Diff cross-cutting / order-sensitivity (column rename + drop with
  same name, index rename + DROP INDEX same plan, etc.).
- Directive-composition pins for inline directives in one CREATE
  TABLE.
- One "kitchen-sink" round-trip YAML fixture exercising every
  supported attribute together.

Each item lists the surface, the missing combination, and a
realistic-harm sketch so the work scope is clear without re-deriving
context. Combined estimate: 2.5–3 days; recommended order is
catalog matrix → kitchen-sink fixture → emitter ordering → diff →
directive composition.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 5, 2026 14:42
@winebarrel winebarrel enabled auto-merge May 5, 2026 14:42
@winebarrel winebarrel merged commit 5172008 into main May 5, 2026
5 checks passed
@winebarrel winebarrel deleted the docs-test-coverage-audit branch May 5, 2026 14:44
@codecov

codecov Bot commented May 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.62%. Comparing base (8264344) to head (64c2176).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #85   +/-   ##
=======================================
  Coverage   86.62%   86.62%           
=======================================
  Files          30       30           
  Lines        3268     3268           
=======================================
  Hits         2831     2831           
  Misses        269      269           
  Partials      168      168           

☔ 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 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 a new TODO section capturing a postmortem-driven audit checklist for improving test coverage around “composed-state” parsing/emission interactions (motivated by the PR #84 review’s discovered bug class), so future contributors have a concrete, prioritized set of high-ROI test gaps to tackle.

Changes:

  • Add “Medium — test coverage audit” section with prioritized, scoped test gap items across catalog parsing, parser interactions, emit ordering, diff order-sensitivity, and directive composition.
  • Include rough effort estimates and a recommended execution order for the audit work.

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

Comment thread TODO.md
Comment on lines +108 to +116
- **Emitter ordering tests (`columnDefSQL`, `indexInlineSQL`,
`constraintInlineSQL`).** PR #84 added an index-comparison
ordering pin for `INVISIBLE`'s position. The other emitters
still rely on `Contains` (order-blind). Add index-comparison
pins for:
- `columnDefSQL` with all attributes set: `CHARSET → COLLATE →
GENERATED → NOT NULL → DEFAULT → ON UPDATE → AUTO_INCREMENT →
INVISIBLE → COMMENT`
- `indexInlineSQL` / `Index.SQL()`: `UNIQUE → KEY_BLOCK_SIZE →
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