Skip to content

Cover the current spec with tests#18

Merged
winebarrel merged 1 commit into
mainfrom
expand-test-coverage
May 2, 2026
Merged

Cover the current spec with tests#18
winebarrel merged 1 commit into
mainfrom
expand-test-coverage

Conversation

@winebarrel

Copy link
Copy Markdown
Owner

Summary

Goal: pin the v1 surface so future refactors can't silently regress. Each test file targets a code path that previously had thin or no coverage.

Coverage delta

Package Before After
myschema (root) 74.8% 81.9%
parser 49.8% 72.5%
model 0% 51.6% (new)
diff 56.6% 56.6% (already covered by expr_test.go from PR #17)

Bug found and fixed

parser/parser.go set Column.Stored = true when opts.Storage == VirtualStorage — exactly backwards. Surfaced by TestParseColumnAttributes. Fixed in this PR.

Tests added

parser/parser_test.go (+8 tests):

  • TestParseInlineForeignKey — column-level REFERENCES other(id) shorthand
  • TestParseColumnAttributes — DEFAULT, COLLATE, COMMENT, ON UPDATE, GENERATED ALWAYS AS … STORED
  • TestParseSpatialAndFulltextIndex — FULLTEXT KEY / SPATIAL KEY → model.IndexFulltext / IndexSpatial
  • TestParseIndexQuirks — prefix length, DESC, multi-column, INVISIBLE, USING BTREE
  • TestParseCheckConstraint — inline + ALTER TABLE ADD CONSTRAINT, NOT ENFORCED preservation
  • TestParseMultiColumnPrimaryKey — PK column ordering + implicit NOT NULL on PK members
  • TestParseViewWithColumnListCREATE VIEW v (a, b) AS …
  • TestParseTableOptions — ENGINE / CHARSET / COLLATE / COMMENT / AUTO_INCREMENT
  • TestParseDuplicateRejection — duplicate table / view / column

model/*_test.go (4 new files): util, view, index, foreignkey — exhaustive SQL() shape tests including reserved-word quoting, multi-column, MATCH FULL, expression indexes, comments

options_test.go (root): FilterOptionsMatchName, FilterOptionsAfterApply, DropPolicyIsDropAllowed, ObjectCount summary format

YAML fixtures (12 new):

  • apply/: add_foreign_key, add_index, drop_column, drop_foreign_key, drop_index, modify_column_type
  • plan/: modify_default, include_filter, disallowed_drop_column, no_changes_with_check (verifies equalCheckDef tolerates backticks), no_changes_with_default_variants
  • dump/: with_index_and_fk

Harness extension

planTestCase grows a disallowed_drops field so plan fixtures can assert on the -- skipped: … lines that go to PlanResult.DisallowedDrops separately from the executable plan SQL.

Test plan

  • make test (parser + diff + catalog + model + YAML harness)
  • make test-scenario
  • golangci-lint
  • All three sample DBs round-trip without drift
  • CI green on this PR

🤖 Generated with Claude Code

Goal: pin the v1 surface so future refactors can't silently regress.
Each test file targets a code path that previously had thin or no
coverage.

parser/parser_test.go (+8 tests, parser coverage 49.8% → 72.5%):
- TestParseInlineForeignKey: inline `col INT REFERENCES other(id)`,
  auto-name as `<table>_ibfk_<col>`
- TestParseColumnAttributes: DEFAULT, COLLATE, COMMENT, ON UPDATE,
  GENERATED ALWAYS AS … STORED
- TestParseSpatialAndFulltextIndex: FULLTEXT KEY / SPATIAL KEY map
  to model.IndexFulltext / IndexSpatial
- TestParseIndexQuirks: prefix length, DESC ordering, multi-column,
  INVISIBLE, USING BTREE
- TestParseCheckConstraint: inline + ALTER TABLE forms, NOT ENFORCED
  preservation
- TestParseMultiColumnPrimaryKey: PK column-list ordering + implicit
  NOT NULL on PK members
- TestParseViewWithColumnList: CREATE VIEW v (a, b) AS …
- TestParseTableOptions: ENGINE / CHARSET / COLLATE / COMMENT /
  AUTO_INCREMENT
- TestParseDuplicateRejection: duplicate table / view / column all
  surface a clear error

model/*_test.go (model coverage 0% → 51.6%):
- util_test: Ident with safe / reserved-word / unsafe-character /
  embedded-backtick names; QuoteLiteral with single quote / backslash
- view_test: CreateSQL with / without column list / WITH LOCAL
  CHECK OPTION; CheckOption=NONE suppression; DropSQL; FQVN
- index_test: BTREE / UNIQUE multi-col / FULLTEXT / prefix length /
  DESC / INVISIBLE / expression / comment
- foreignkey_test: basic + ON DELETE CASCADE/ON UPDATE SET NULL +
  multi-column + MATCH FULL

options_test.go (root coverage 74.8% → 81.9%):
- FilterOptionsMatchName: include / exclude / both / glob patterns
- FilterOptionsAfterApply: invalid include and exclude patterns
- DropPolicyIsDropAllowed: empty / `all` wildcard / specific / multi
- ObjectCount: DBLabel + Summary

YAML fixtures:
- apply: add_foreign_key, add_index, drop_column (with allow_drop),
  drop_foreign_key, drop_index, modify_column_type
- plan: modify_default, include_filter, disallowed_drop_column,
  no_changes_with_check (verifies equalCheckDef tolerates backticks),
  no_changes_with_default_variants
- dump: with_index_and_fk

Harness extension: planTestCase grows a `disallowed_drops` field so
plan fixtures can assert on the `-- skipped: …` lines that go to
PlanResult.DisallowedDrops separately from the executable plan SQL.

Bug found and fixed: parser/parser.go set Column.Stored to true when
opts.Storage == VirtualStorage — i.e. exactly backwards. Now uses
StoredStorage. Surfaced by TestParseColumnAttributes.

TODO.md: tick the generated-column-expression diff item (the equalExpr
plumbing in PR #17 plus the Stored bug fix here together resolve it).

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

codecov Bot commented May 2, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@             Coverage Diff             @@
##             main      #18       +/-   ##
===========================================
+ Coverage   40.20%   54.10%   +13.89%     
===========================================
  Files          25       25               
  Lines        1634     1634               
===========================================
+ Hits          657      884      +227     
+ Misses        852      626      -226     
+ Partials      125      124        -1     

☔ 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 winebarrel merged commit 1f86ee8 into main May 2, 2026
4 checks passed
@winebarrel winebarrel deleted the expand-test-coverage branch May 2, 2026 05:43
winebarrel added a commit that referenced this pull request May 2, 2026
Copilot review #18 (single comment): line 442 of directive.go still
held a literal `\“` (backslash + U+201C "left double quotation mark")
where backticks were intended in the doc comment for
stripUntilAfterBacktickedName. The previous round of curly-quote
cleanup missed this site and another nearby one in the tokenize doc.

Both comments are now rephrased without nested-backtick examples so
the prose carries the meaning without relying on quoting tricks. No
behaviour change.

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

1 participant