Skip to content

test(review): Expand boundary fixtures across all eight rules#117

Merged
mhiro2 merged 3 commits into
mainfrom
feat/review-fixtures-edge-cases
Apr 30, 2026
Merged

test(review): Expand boundary fixtures across all eight rules#117
mhiro2 merged 3 commits into
mainfrom
feat/review-fixtures-edge-cases

Conversation

@mhiro2

@mhiro2 mhiro2 commented Apr 30, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add eight new golden fixtures under fixtures/review/, one per review rule, to lock down the should-NOT-fire side of each rule plus a previously uncovered SET NULLCASCADE transition.
  • Cover destructive-change boundaries (drop-column / drop-table / drop-pk / type-narrow) with paired removal-of-FK / equivalent-PK / dialect-alias inputs that all expect zero findings.
  • Cover additive-change boundaries (add-not-null / add-unique / add-cascade-delete / fk-without-index) including new-table-with-default, composite-prefix-index, and a non-cascade upgrade fixture that exercises the modified-FK branch with Some("SET NULL") rather than the parser-default None.

Changes

  • 996f2df : test(review): add destructive-change boundary fixtures
    • Add drop-column-referenced-fk-also-removed, drop-table-referenced-cascade, drop-pk-replaced-by-equivalent-pk, and type-narrow-equivalent-int4 to lock down fk_is_removed, removed_fks, same_column_set, and int_rank short-circuits as zero-finding contracts.
  • 158568b : test(review): add additive-change boundary fixtures
    • Add add-not-null-new-table-with-default, add-unique-on-new-table, add-cascade-delete-from-noaction, and fk-with-composite-prefix-index to cover new-table iteration skip, default-bearing NOT NULL columns, the composite-index prefix match in fk_columns_are_indexed, and the cascade-upgrade warning path.
  • e3210c8 : test(review): replace cascade-from-noaction fixture with set-null variant
    • Rename to add-cascade-delete-from-set-null and rewrite the before-side ON DELETE clause so old_value.on_delete = Some("SET NULL"), removing duplicate coverage with the existing add-cascade-delete fixture (which the parser already normalizes from absent / explicit NO ACTION to None).

mhiro2 added 3 commits April 30, 2026 21:58
Cover false-positive boundaries on the drop / narrow side of the rule
catalog so the golden suite exercises the "should NOT fire" path for
each destructive rule:

- `drop-column-referenced-fk-also-removed` — the FK column on the
  source side is dropped together with the referencing constraint, so
  `fk_is_removed` short-circuits the outgoing-FK check.
- `drop-table-referenced-cascade` — the dropped table's incoming FK is
  removed in the same migration, exercising the `removed_fks` path
  built from `fk_diffs` removals.
- `drop-pk-replaced-by-equivalent-pk` — the PRIMARY KEY is rebuilt
  under a different constraint name on the same column set, so
  `same_column_set` returns true and no PK-loss finding is emitted.
- `type-narrow-equivalent-int4` — `INT` → `INTEGER` is a dialect alias
  with identical `int_rank`, so `detect_type_narrowing` returns None.

All four fixtures expect an empty findings array, locking in the
no-finding contract against future regressions.
Cover the "add"-side rules with new tables, default values, composite
indexes, and the implicit-NO-ACTION → CASCADE transition so the golden
suite locks down both the should-not-fire and should-fire paths:

- `add-not-null-new-table-with-default` — NOT NULL columns with
  DEFAULT values on a brand-new table do not trigger
  `risk/add-not-null-on-existing` because `run_rules` only iterates
  `modified_tables`.
- `add-unique-on-new-table` — a UNIQUE INDEX created together with a
  new table is similarly skipped by the modified-tables iteration.
- `add-cascade-delete-from-noaction` — an existing FK gains
  `ON DELETE CASCADE` while the before side has no explicit
  `ON DELETE` clause, complementing the existing fixture which uses an
  explicit `NO ACTION`. Expects one warning finding.
- `fk-with-composite-prefix-index` — a newly added FK on `(tenant_id)`
  is covered by a pre-existing composite index on
  `(tenant_id, created_at)`, so `column_list_has_prefix` returns true
  and `risk/fk-without-index` stays silent.
…iant

The parser maps both explicit `ON DELETE NO ACTION` and the absent
clause to `ReferentialAction::NoAction`, and the diff layer normalizes
that to `None` on `old_value.on_delete`. Under that contract the
`add-cascade-delete-from-noaction` fixture provided no additional
coverage over the existing `add-cascade-delete` fixture; only the
table and column names differed.

Use `ON DELETE SET NULL` → `ON DELETE CASCADE` instead so the modified
branch of `check_add_cascade_delete` sees an actual non-CASCADE
referential action on `old_value` (`Some("SET NULL")`) before the
upgrade. The rename keeps the fixture name aligned with the SQL it
ships.
@mhiro2 mhiro2 self-assigned this Apr 30, 2026
@mhiro2 mhiro2 added the enhancement New feature or request label Apr 30, 2026
@github-actions

Copy link
Copy Markdown

Code Metrics Report

main (1aee9bd) #117 (2382f5b) +/-
Coverage 94.6% 94.6% +0.0%
Test Execution Time 1m35s 1m34s -1s
Details
  |                     | main (1aee9bd) | #117 (2382f5b) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          94.6% |          94.6% | +0.0% |
  |   Files             |             81 |             81 |     0 |
  |   Lines             |          37324 |          37324 |     0 |
+ |   Covered           |          35334 |          35344 |   +10 |
+ | Test Execution Time |          1m35s |          1m34s |   -1s |

Code coverage of files in pull request scope (92.2% → 92.8%)

Files Coverage +/- Status
crates/relune-core/src/review/rules.rs 92.8% +0.6% affected

Reported by octocov

@mhiro2
mhiro2 merged commit 0ca4bbe into main Apr 30, 2026
4 checks passed
@mhiro2
mhiro2 deleted the feat/review-fixtures-edge-cases branch April 30, 2026 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant