Skip to content

Commit ef18fb6

Browse files
sj-robertclaude
authored andcommitted
test: add multi-file FK integration tests for PGM003
Tests PGM003 (FK without covering index) across migration boundaries, covering all changed-file combinations: only FK file, only index file, both files, and all files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 75d3605 commit ef18fb6

6 files changed

Lines changed: 107 additions & 8 deletions

File tree

src/input/liquibase_xml.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -778,11 +778,7 @@ fn generate_add_fk_sql(
778778
/// Required fields (`table_name`, `column_names`) are passed as pre-validated
779779
/// parameters. Optional fields (`constraintName`, `schemaName`) are extracted
780780
/// from `attrs`.
781-
fn generate_add_pk_sql(
782-
attrs: &[(String, String)],
783-
table_name: &str,
784-
column_names: &str,
785-
) -> String {
781+
fn generate_add_pk_sql(attrs: &[(String, String)], table_name: &str, column_names: &str) -> String {
786782
let constraint_name = get_attr(attrs, "constraintName").unwrap_or_default();
787783
let schema_name = get_attr(attrs, "schemaName");
788784

@@ -1484,8 +1480,7 @@ mod tests {
14841480
assert_eq!(units.len(), 1);
14851481
let sql = &units[0].sql;
14861482
assert_eq!(
1487-
sql,
1488-
"ALTER TABLE orders ADD FOREIGN KEY (user_id) REFERENCES users (id);",
1483+
sql, "ALTER TABLE orders ADD FOREIGN KEY (user_id) REFERENCES users (id);",
14891484
"Expected valid FK SQL without CONSTRAINT keyword, got: {}",
14901485
sql
14911486
);

tests/fixtures/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ This directory contains test fixtures for pg-migration-lint.
88
- `clean/` - All migrations correct, expect 0 findings
99
- `all-rules/` - One violation per rule (PGM001-PGM011), expect 11 findings
1010
- `suppressed/` - All violations suppressed, expect 0 findings
11+
- `fk-with-later-index/` - Tests PGM003 FK detection across migration boundaries
1112
- `liquibase-xml/` - Tests Liquibase fallback parser
1213

1314
## Usage
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
CREATE TABLE customers (
2+
id serial PRIMARY KEY,
3+
name text NOT NULL
4+
);
5+
6+
CREATE TABLE orders (
7+
id serial PRIMARY KEY,
8+
customer_id integer NOT NULL,
9+
total numeric(12,2) NOT NULL
10+
);
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE orders ADD CONSTRAINT fk_orders_customer
2+
FOREIGN KEY (customer_id) REFERENCES customers(id);
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
CREATE INDEX idx_orders_customer_id ON orders (customer_id);

tests/integration.rs

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,10 @@ fn test_pgm004_finding_details() {
262262
fn test_pgm002_finding_details() {
263263
// V003 drops idx_customers_email WITHOUT CONCURRENTLY.
264264
// V001 is replayed as baseline (creates the index), V002 and V003 are changed.
265-
let findings = lint_fixture("all-rules", &["V002__violations.sql", "V003__more_violations.sql"]);
265+
let findings = lint_fixture(
266+
"all-rules",
267+
&["V002__violations.sql", "V003__more_violations.sql"],
268+
);
266269
let pgm002: Vec<&Finding> = findings.iter().filter(|f| f.rule_id == "PGM002").collect();
267270

268271
assert_eq!(pgm002.len(), 1, "Expected exactly 1 PGM002 finding");
@@ -1826,3 +1829,90 @@ fn test_sarif_and_sonarqube_finding_counts_match() {
18261829
findings.len()
18271830
);
18281831
}
1832+
1833+
// ===========================================================================
1834+
// Cross-file FK detection (PGM003) with fk-with-later-index fixture
1835+
// ===========================================================================
1836+
1837+
#[test]
1838+
fn test_fk_without_index_cross_file_only_fk_changed() {
1839+
// Only V002 is changed. V001 is replayed as history (creates tables).
1840+
// V002 adds FK on orders.customer_id but V003 (which adds the covering
1841+
// index) has NOT been replayed yet. PGM003 should fire because
1842+
// catalog_after has no covering index at this point.
1843+
let findings = lint_fixture("fk-with-later-index", &["V002__add_fk.sql"]);
1844+
let pgm003: Vec<&Finding> = findings.iter().filter(|f| f.rule_id == "PGM003").collect();
1845+
1846+
assert_eq!(
1847+
pgm003.len(),
1848+
1,
1849+
"Expected exactly 1 PGM003 finding for FK without index. Got:\n {}",
1850+
format_findings(&findings)
1851+
);
1852+
assert!(
1853+
pgm003[0].message.contains("customer_id"),
1854+
"PGM003 message should mention 'customer_id'. Got: {}",
1855+
pgm003[0].message
1856+
);
1857+
}
1858+
1859+
#[test]
1860+
fn test_fk_with_later_index_only_index_changed() {
1861+
// Only V003 is changed. V001 and V002 are replayed as history.
1862+
// The FK from V002 already exists in catalog_before, and V003 adds
1863+
// the covering index. Since V002 is not being linted, no PGM003
1864+
// should fire -- the FK was in a prior file, not in the current lint set.
1865+
let findings = lint_fixture("fk-with-later-index", &["V003__add_index.sql"]);
1866+
let pgm003: Vec<&Finding> = findings.iter().filter(|f| f.rule_id == "PGM003").collect();
1867+
1868+
assert!(
1869+
pgm003.is_empty(),
1870+
"PGM003 should NOT fire when only the index file is linted. Got:\n {}",
1871+
format_findings(&findings)
1872+
);
1873+
}
1874+
1875+
#[test]
1876+
fn test_fk_cross_file_both_changed() {
1877+
// Both V002 and V003 are changed. V001 is replayed as history.
1878+
// When linting V002: FK is added but no covering index yet -> PGM003 fires.
1879+
// When linting V003: index is added, no new FK in this file -> no PGM003.
1880+
let findings = lint_fixture(
1881+
"fk-with-later-index",
1882+
&["V002__add_fk.sql", "V003__add_index.sql"],
1883+
);
1884+
let pgm003: Vec<&Finding> = findings.iter().filter(|f| f.rule_id == "PGM003").collect();
1885+
1886+
assert_eq!(
1887+
pgm003.len(),
1888+
1,
1889+
"Expected exactly 1 PGM003 finding (from V002 only). Got:\n {}",
1890+
format_findings(&findings)
1891+
);
1892+
assert!(
1893+
pgm003[0].message.contains("customer_id"),
1894+
"PGM003 message should mention 'customer_id'. Got: {}",
1895+
pgm003[0].message
1896+
);
1897+
}
1898+
1899+
#[test]
1900+
fn test_fk_cross_file_all_changed() {
1901+
// All files are changed (empty changed set). V001 creates tables (no FK,
1902+
// no finding). V002 adds FK without covering index -> PGM003 fires.
1903+
// V003 adds the covering index -> no additional PGM003.
1904+
let findings = lint_fixture("fk-with-later-index", &[]);
1905+
let pgm003: Vec<&Finding> = findings.iter().filter(|f| f.rule_id == "PGM003").collect();
1906+
1907+
assert_eq!(
1908+
pgm003.len(),
1909+
1,
1910+
"Expected exactly 1 PGM003 finding (from V002). Got:\n {}",
1911+
format_findings(&findings)
1912+
);
1913+
assert!(
1914+
pgm003[0].message.contains("customer_id"),
1915+
"PGM003 message should mention 'customer_id'. Got: {}",
1916+
pgm003[0].message
1917+
);
1918+
}

0 commit comments

Comments
 (0)