diff --git a/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs b/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs index d03334744..408efc4b5 100644 --- a/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs +++ b/crates/sqlquack-syntax-coverage/src/cli/_helpers/commands.rs @@ -314,7 +314,7 @@ pub(crate) fn snapshot_command( say("FAILED, a snapshot of a tree with problems claims nothing"); return Err(ExitCode::FAILURE); } - print(true, &snapshot(&check, &coverage.file_hashes, &commit), &[]); + print(true, &snapshot(&check, &coverage, &commit), &[]); Ok(ExitCode::SUCCESS) } @@ -339,7 +339,7 @@ fn read_snapshot(path: &Path) -> Result { /// Lines naming each non-empty list of the diff. fn diff_lines(diff: &SnapshotDiff) -> Vec { - let lists: [(&str, &Vec); 9] = [ + let lists: [(&str, &Vec); 10] = [ ("REGRESSION forms lost", &diff.forms_lost), ("REGRESSION obligations lost", &diff.obligations_lost), ( @@ -350,6 +350,10 @@ fn diff_lines(diff: &SnapshotDiff) -> Vec { "REGRESSION Snowflake syntax errors the parser now accepts", &diff.syntax_error_cases_accepted, ), + ( + "DOCUMENTED (D113) Snowflake syntax errors the parser now accepts, for the auditor", + &diff.documented_cases_accepted, + ), ("CATALOGUE files changed", &diff.catalogue_files_changed), ("CATALOGUE forms added", &diff.forms_added), ("CATALOGUE forms removed", &diff.forms_removed), @@ -378,7 +382,7 @@ pub(crate) fn diff( if !check.problems.is_empty() { return Err(ExitCode::FAILURE); } - snapshot(&check, &coverage.file_hashes, &base.catalogue_commit) + snapshot(&check, &coverage, &base.catalogue_commit) } }; let diff: SnapshotDiff = diff_snapshots(&base, &head); diff --git a/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs b/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs index 07b222ef4..6a622233f 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/_helpers/sections.rs @@ -13,11 +13,12 @@ use crate::formats::_helpers::records::{ }; use crate::formats::_helpers::retired::{retire, withdrawn_receipts}; use crate::formats::constants::{ - ALIASES_FILE, CASES_DIR, CATALOGUE_DIR, INDEXES_FILE, LIMITS_FILE, OBLIGATIONS_DIR, - RECEIPT_REPORT_SUFFIX, RECEIPTS_DIR, REVIEWS_DIR, SOURCES_FILE, UNOBSERVABLE_DIR, + ALIASES_FILE, CASES_DIR, CATALOGUE_DIR, INDEXES_FILE, LIMITS_FILE, MARKS, MARKS_DIR, + OBLIGATIONS_DIR, RECEIPT_REPORT_SUFFIX, RECEIPTS_DIR, REVIEWS_DIR, SOURCES_FILE, + UNOBSERVABLE_DIR, }; use crate::formats::models::{ - Alias, AliasesFile, Case, CatalogueFile, Diagnostic, Index, IndexesFile, LimitsFile, + Alias, AliasesFile, Case, CatalogueFile, Diagnostic, Index, IndexesFile, LimitsFile, Mark, Obligation, ObligationsFile, Reading, Receipt, Review, ReviewsFile, Source, SourcesFile, Unobservable, UnobservableFile, }; @@ -182,6 +183,29 @@ pub(crate) fn load_cases(root: &Path) -> Section>> { Section::of(records, declared, problems) } +/// `marks/.jsonl`: optional, each mark one of MARKS and each case marked at most once. +pub(crate) fn load_marks(root: &Path) -> Section> { + let (paths, others) = listed(&root.join(MARKS_DIR), "jsonl"); + let mut problems: Vec = others.iter().map(|path| stray(path, "jsonl")).collect(); + let mut records: Vec = Vec::new(); + for path in paths { + let (marks, read_problems) = read_jsonl::(&path); + problems.extend(read_problems); + records.extend(marks); + } + problems.extend( + records + .iter() + .filter(|mark| !MARKS.contains(&mark.mark.as_str())) + .map(|mark| format!("mark {}: unknown mark {:?}", mark.case_id, mark.mark)), + ); + let declared: Vec = records + .iter() + .map(|mark| format!("mark {}", mark.case_id)) + .collect(); + Section::of(records, declared, problems) +} + /// Every receipts file, less the receipts superseded or withdrawn by the run reports beside them. pub(crate) fn load_receipts(root: &Path) -> Section<(Vec, Vec)> { let (paths, others) = listed(&root.join(RECEIPTS_DIR), "jsonl"); @@ -296,6 +320,7 @@ pub(crate) struct Sections { pub(crate) reviews: Section>, pub(crate) unobservable: Section<(Vec, Vec)>, pub(crate) readings: Section>, + pub(crate) marks: Section>, } impl Sections { @@ -312,6 +337,7 @@ impl Sections { &self.reviews.problems, &self.unobservable.problems, &self.readings.problems, + &self.marks.problems, ] .into_iter() .flatten() @@ -325,6 +351,7 @@ impl Sections { &self.obligations.declared, &self.cases.declared, &self.reviews.declared, + &self.marks.declared, ] .into_iter() .flatten() @@ -348,6 +375,7 @@ pub(crate) fn load_sections(root: &Path) -> Sections { reviews: load_reviews(root), unobservable: load_unobservable(root), readings: load_readings(root), + marks: load_marks(root), sources, } } diff --git a/crates/sqlquack-syntax-coverage/src/formats/constants.rs b/crates/sqlquack-syntax-coverage/src/formats/constants.rs index 64d269e26..0ddf80c2c 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/constants.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/constants.rs @@ -20,6 +20,16 @@ pub const RECEIPTS_DIR: &str = "receipts"; pub const REVIEWS_DIR: &str = "reviews"; /// Obligations Snowflake answers only with an internal error, one optional TOML file per family. pub const UNOBSERVABLE_DIR: &str = "unobservable"; +/// The catalogue's case marks, one optional JSON Lines file per family (decision D135). +pub const MARKS_DIR: &str = "marks"; +/// A case that follows its documentation against Snowflake's syntax error (D113). +pub const MARK_DOCUMENTED: &str = "documented (D113)"; +/// Every mark a case can carry. +pub const MARKS: [&str; 3] = [ + MARK_DOCUMENTED, + "settled accept (D133)", + "settled reject (D133)", +]; /// Hand-added limits, suppressions and grammar readings; the checker loads the readings. pub const LIMITS_FILE: &str = "limits.toml"; /// Every family, each with a catalogue, an obligation and a case file. diff --git a/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs b/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs index 32e8c851f..92afa166c 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/main/load_coverage.rs @@ -4,12 +4,14 @@ use std::collections::BTreeMap; use std::path::Path; use crate::formats::_helpers::sections::{Sections, load_sections}; -use crate::formats::models::{Definition, Form, SyntaxCoverage}; +use crate::formats::constants::MARK_DOCUMENTED; +use crate::formats::models::{Case, Definition, Form, SyntaxCoverage}; /// Every file under `root`, or every format problem found; nothing is skipped or defaulted. pub fn load_coverage(root: &Path) -> Result> { let sections: Sections = load_sections(root); - let problems: Vec = sections.problems(); + let mut problems: Vec = sections.problems(); + problems.extend(unmatched_marks(§ions)); if !problems.is_empty() { return Err(problems); } @@ -38,5 +40,39 @@ pub fn load_coverage(root: &Path) -> Result> { diagnostics: sections.unobservable.records.1, readings: sections.readings.records, file_hashes, + documented_cases: sections + .marks + .records + .into_iter() + .filter(|mark| mark.mark == MARK_DOCUMENTED) + .map(|mark| mark.case_id) + .collect(), }) } + +/// Marks naming no case, or a case whose SQL or obligations differ from the mark's. +fn unmatched_marks(sections: &Sections) -> Vec { + let cases: BTreeMap<&str, &Case> = sections + .cases + .records + .values() + .flatten() + .map(|case| (case.case_id.as_str(), case)) + .collect(); + sections + .marks + .records + .iter() + .filter(|mark| { + cases.get(mark.case_id.as_str()).is_none_or(|case| { + case.sql_sha256 != mark.sql_sha256 || case.obligations != mark.obligations + }) + }) + .map(|mark| { + format!( + "mark {}: matches no case's SQL and obligations", + mark.case_id + ) + }) + .collect() +} diff --git a/crates/sqlquack-syntax-coverage/src/formats/models.rs b/crates/sqlquack-syntax-coverage/src/formats/models.rs index 97db6155b..1090e3a6e 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/models.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/models.rs @@ -1,6 +1,6 @@ //! The syntax-coverage files: sources, catalogue, obligations, cases, receipts and reviews. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -193,6 +193,16 @@ pub enum ObligationKind { Extreme, } +/// One line of `marks/.jsonl`: how the catalogue settled one case. +#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub struct Mark { + pub case_id: String, + pub sql_sha256: String, + pub obligations: Vec, + pub mark: String, +} + /// One line of `cases/.jsonl`. #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] #[serde(deny_unknown_fields)] @@ -561,4 +571,6 @@ pub struct SyntaxCoverage { pub readings: Vec, /// SHA-256 of the sources, aliases, catalogue and obligation files, by relative path. pub file_hashes: BTreeMap, + /// Cases marked documented (D113), whose acceptance against a syntax error is intended. + pub documented_cases: BTreeSet, } diff --git a/crates/sqlquack-syntax-coverage/src/formats/tests/test_load_coverage.rs b/crates/sqlquack-syntax-coverage/src/formats/tests/test_load_coverage.rs index 475bdd3c4..a532bcd8f 100644 --- a/crates/sqlquack-syntax-coverage/src/formats/tests/test_load_coverage.rs +++ b/crates/sqlquack-syntax-coverage/src/formats/tests/test_load_coverage.rs @@ -290,6 +290,28 @@ fn given_a_valid_tree_when_loading_then_every_record_is_kept() { #[test] fn given_a_format_fault_when_loading_then_the_problem_is_named() { let test_cases = [ + FormatFaultTestCase { + description: "a mark whose SQL hash is not its case's", + mutate: |root| { + write( + root, + "marks/query.jsonl", + "{\"case_id\":\"select-from\",\"sql_sha256\":\"0000000000000000000000000000000000000000000000000000000000000000\",\"obligations\":[\"sf.query.select.o002\"],\"mark\":\"documented (D113)\"}\n", + ) + }, + expected_problem: "mark select-from: matches no case's SQL and obligations", + }, + FormatFaultTestCase { + description: "a mark the catalogue does not define", + mutate: |root| { + write( + root, + "marks/query.jsonl", + "{\"case_id\":\"select-from\",\"sql_sha256\":\"0000000000000000000000000000000000000000000000000000000000000000\",\"obligations\":[],\"mark\":\"documented\"}\n", + ) + }, + expected_problem: "mark select-from: unknown mark \"documented\"", + }, FormatFaultTestCase { description: "a deleted family catalogue", mutate: |root| std::fs::remove_file(root.join("catalogue/dml.toml")).expect("remove"), diff --git a/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs b/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs index 8b5035512..35eeae8fa 100644 --- a/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs +++ b/crates/sqlquack-syntax-coverage/src/report/main/diff_snapshots.rs @@ -20,6 +20,11 @@ pub fn diff_snapshots(base: &Snapshot, head: &Snapshot) -> SnapshotDiff { let forms: BTreeSet<&String> = base.forms.keys().chain(head.forms.keys()).collect(); let in_base = |id: &String| base.forms.get(id); let in_head = |id: &String| head.forms.get(id); + let flipped: Vec = base + .syntax_error_cases_rejected + .intersection(&head.syntax_error_cases_accepted) + .cloned() + .collect(); SnapshotDiff { catalogue_files_changed: paths .into_iter() @@ -59,10 +64,14 @@ pub fn diff_snapshots(base: &Snapshot, head: &Snapshot) -> SnapshotDiff { .intersection(&head.refused_cases) .cloned() .collect(), - syntax_error_cases_accepted: base - .syntax_error_cases_rejected - .intersection(&head.syntax_error_cases_accepted) + syntax_error_cases_accepted: flipped + .iter() + .filter(|id| !head.documented_cases.contains(*id)) .cloned() .collect(), + documented_cases_accepted: flipped + .into_iter() + .filter(|id| head.documented_cases.contains(id)) + .collect(), } } diff --git a/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs b/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs index 9eefea5da..003a8e2d0 100644 --- a/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs +++ b/crates/sqlquack-syntax-coverage/src/report/main/snapshot.rs @@ -1,9 +1,10 @@ //! `snapshot`: the claims of one check, bound to the catalogue it ran on. -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeSet; use crate::check::models::{CheckResult, LocalVerdict, SnowflakeSyntax}; use crate::formats::constants::{ALIASES_FILE, CATALOGUE_DIR, SNAPSHOT_FORMAT, SOURCES_FILE}; +use crate::formats::models::SyntaxCoverage; use crate::report::models::Snapshot; /// Whether a coverage path belongs to the catalogue rather than to obligations or evidence. @@ -31,16 +32,17 @@ fn by_snowflake( .collect() } -/// The snapshot of `check`; `file_hashes` are the loaded files' hashes by relative path. +/// The snapshot of `check` on `coverage`, the tree it checked. pub fn snapshot( check: &CheckResult, - file_hashes: &BTreeMap, + coverage: &SyntaxCoverage, catalogue_commit: &str, ) -> Snapshot { Snapshot { format: SNAPSHOT_FORMAT.to_owned(), catalogue_commit: catalogue_commit.to_owned(), - catalogue_files: file_hashes + catalogue_files: coverage + .file_hashes .iter() .filter(|(path, _)| is_catalogue_file(path)) .map(|(path, hash)| (path.clone(), hash.clone())) @@ -62,5 +64,6 @@ pub fn snapshot( refused_cases: by_snowflake(check, SnowflakeSyntax::Parsed, false), syntax_error_cases_rejected: by_snowflake(check, SnowflakeSyntax::Refused, false), syntax_error_cases_accepted: by_snowflake(check, SnowflakeSyntax::Refused, true), + documented_cases: coverage.documented_cases.clone(), } } diff --git a/crates/sqlquack-syntax-coverage/src/report/models.rs b/crates/sqlquack-syntax-coverage/src/report/models.rs index da7fbed1a..6f78a8192 100644 --- a/crates/sqlquack-syntax-coverage/src/report/models.rs +++ b/crates/sqlquack-syntax-coverage/src/report/models.rs @@ -70,6 +70,9 @@ pub struct Snapshot { /// Receipted cases Snowflake refused as syntax that the parser accepts, opaque or not. #[serde(default)] pub syntax_error_cases_accepted: BTreeSet, + /// Cases the catalogue marks documented (D113): their acceptance against a syntax error is intended. + #[serde(default)] + pub documented_cases: BTreeSet, } /// One snapshot against a later one; coverage and catalogue changes kept apart. @@ -88,6 +91,8 @@ pub struct SnapshotDiff { pub obligations_gained: Vec, /// Cases Snowflake parsed that the base parser accepted, opaque or not, and the head rejects. pub parsed_cases_refused: Vec, - /// Cases Snowflake refused as syntax that the base parser rejected and the head accepts. + /// Cases Snowflake refused as syntax that the base parser rejected and the head accepts, less documented ones. pub syntax_error_cases_accepted: Vec, + /// The same flips on cases marked documented (D113): listed for the auditor, not failing (D135). + pub documented_cases_accepted: Vec, } diff --git a/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs b/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs index f51b61131..830ecd882 100644 --- a/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs +++ b/crates/sqlquack-syntax-coverage/src/report/tests/helpers.rs @@ -33,6 +33,7 @@ pub(crate) fn base() -> Snapshot { refused_cases: BTreeSet::from(["sf.query.with.k2".to_owned()]), syntax_error_cases_rejected: BTreeSet::from(["sf.query.select.k3".to_owned()]), syntax_error_cases_accepted: BTreeSet::new(), + documented_cases: BTreeSet::new(), } } @@ -67,12 +68,30 @@ pub(crate) fn observed_select_from(root: &Path, errno: i64) { ); } +/// Marks `select-from` with `mark` in `marks/query.jsonl`. +pub(crate) fn mark_select_from(root: &Path, mark: &str) { + let coverage = load_coverage(root).expect("load"); + let case = coverage.cases["query"] + .iter() + .find(|case| case.case_id == "select-from") + .expect("select-from"); + let line: String = serde_json::json!({ + "case_id": case.case_id, + "sql_sha256": case.sql_sha256, + "obligations": case.obligations, + "mark": mark, + }) + .to_string(); + std::fs::create_dir_all(root.join("marks")).expect("marks directory"); + std::fs::write(root.join("marks/query.jsonl"), format!("{line}\n")).expect("write marks"); +} + /// The snapshot of the tree at `root` under `parse`. pub(crate) fn snapshot_with(root: &Path, parse: fn(&str) -> LocalVerdict) -> Snapshot { let coverage = load_coverage(root).expect("load"); snapshot( &check_coverage(&coverage, Some(TREE), &parse), - &coverage.file_hashes, + &coverage, "catalogue", ) } diff --git a/crates/sqlquack-syntax-coverage/src/report/tests/test_parser_changes.rs b/crates/sqlquack-syntax-coverage/src/report/tests/test_parser_changes.rs index 7380f43e2..160617a36 100644 --- a/crates/sqlquack-syntax-coverage/src/report/tests/test_parser_changes.rs +++ b/crates/sqlquack-syntax-coverage/src/report/tests/test_parser_changes.rs @@ -2,9 +2,12 @@ use crate::check::tests::helpers::{ append_receipt, opaque, receipt_line, rejecting_select_17, select_17, structural, verified_tree, with_fields, }; +use crate::formats::constants::MARK_DOCUMENTED; use crate::report::main::diff_snapshots::diff_snapshots; use crate::report::models::SnapshotDiff; -use crate::report::tests::helpers::{observed_select_17, observed_select_from, snapshot_with}; +use crate::report::tests::helpers::{ + ids, mark_select_from, observed_select_17, observed_select_from, snapshot_with, +}; use crate::report::tests::test_types::ParserChangeTestCase; /// The shapes of the four regressions #55 brought on observe rows (audit 38bed222), and controls. @@ -19,6 +22,7 @@ fn given_receipted_cases_of_any_expectation_when_the_parser_changes_then_the_dif head: rejecting_select_17, expected_parsed_cases_refused: vec!["select-17"], expected_syntax_error_cases_accepted: Vec::new(), + expected_documented_cases_accepted: Vec::new(), }, ParserChangeTestCase { description: "an observe case that failed semantic analysis, now rejected (database-role-clone)", @@ -27,6 +31,7 @@ fn given_receipted_cases_of_any_expectation_when_the_parser_changes_then_the_dif head: rejecting_select_17, expected_parsed_cases_refused: vec!["select-17"], expected_syntax_error_cases_accepted: Vec::new(), + expected_documented_cases_accepted: Vec::new(), }, ParserChangeTestCase { description: "an observe case Snowflake parsed, opaque before and rejected now (create-stream-clone)", @@ -35,6 +40,7 @@ fn given_receipted_cases_of_any_expectation_when_the_parser_changes_then_the_dif head: rejecting_select_17, expected_parsed_cases_refused: vec!["select-17"], expected_syntax_error_cases_accepted: Vec::new(), + expected_documented_cases_accepted: Vec::new(), }, ParserChangeTestCase { description: "an observe case refused with a positioned 1003, now accepted (create-database)", @@ -43,6 +49,7 @@ fn given_receipted_cases_of_any_expectation_when_the_parser_changes_then_the_dif head: opaque, expected_parsed_cases_refused: Vec::new(), expected_syntax_error_cases_accepted: vec!["select-from"], + expected_documented_cases_accepted: Vec::new(), }, ParserChangeTestCase { description: "an observe case refused with 921, now accepted", @@ -51,6 +58,31 @@ fn given_receipted_cases_of_any_expectation_when_the_parser_changes_then_the_dif head: opaque, expected_parsed_cases_refused: Vec::new(), expected_syntax_error_cases_accepted: vec!["select-from"], + expected_documented_cases_accepted: Vec::new(), + }, + ParserChangeTestCase { + description: "a case marked documented (D113), refused with 1003 and now accepted, is listed apart (D135)", + mutate: |root| { + observed_select_from(root, 1003); + mark_select_from(root, MARK_DOCUMENTED); + }, + base: structural, + head: opaque, + expected_parsed_cases_refused: Vec::new(), + expected_syntax_error_cases_accepted: Vec::new(), + expected_documented_cases_accepted: vec!["select-from"], + }, + ParserChangeTestCase { + description: "a case marked settled, refused with 1003 and now accepted, still fails", + mutate: |root| { + observed_select_from(root, 1003); + mark_select_from(root, "settled reject (D133)"); + }, + base: structural, + head: opaque, + expected_parsed_cases_refused: Vec::new(), + expected_syntax_error_cases_accepted: vec!["select-from"], + expected_documented_cases_accepted: Vec::new(), }, ParserChangeTestCase { description: "a positionless 1003 is no parse, so its rejection is not flagged", @@ -59,6 +91,7 @@ fn given_receipted_cases_of_any_expectation_when_the_parser_changes_then_the_dif head: rejecting_select_17, expected_parsed_cases_refused: Vec::new(), expected_syntax_error_cases_accepted: Vec::new(), + expected_documented_cases_accepted: Vec::new(), }, ParserChangeTestCase { description: "receipts that disagree on the statement are not used", @@ -77,6 +110,7 @@ fn given_receipted_cases_of_any_expectation_when_the_parser_changes_then_the_dif head: rejecting_select_17, expected_parsed_cases_refused: Vec::new(), expected_syntax_error_cases_accepted: Vec::new(), + expected_documented_cases_accepted: Vec::new(), }, ParserChangeTestCase { description: "an unchanged parser flags nothing", @@ -85,6 +119,7 @@ fn given_receipted_cases_of_any_expectation_when_the_parser_changes_then_the_dif head: structural, expected_parsed_cases_refused: Vec::new(), expected_syntax_error_cases_accepted: Vec::new(), + expected_documented_cases_accepted: Vec::new(), }, ]; for test_case in test_cases { @@ -96,18 +131,15 @@ fn given_receipted_cases_of_any_expectation_when_the_parser_changes_then_the_dif &snapshot_with(directory.path(), test_case.head), ); assert_eq!( - (diff.parsed_cases_refused, diff.syntax_error_cases_accepted), ( - test_case - .expected_parsed_cases_refused - .iter() - .map(|id| (*id).to_owned()) - .collect::>(), - test_case - .expected_syntax_error_cases_accepted - .iter() - .map(|id| (*id).to_owned()) - .collect::>(), + diff.parsed_cases_refused, + diff.syntax_error_cases_accepted, + diff.documented_cases_accepted + ), + ( + ids(&test_case.expected_parsed_cases_refused), + ids(&test_case.expected_syntax_error_cases_accepted), + ids(&test_case.expected_documented_cases_accepted), ), "{}", test_case.description diff --git a/crates/sqlquack-syntax-coverage/src/report/tests/test_types.rs b/crates/sqlquack-syntax-coverage/src/report/tests/test_types.rs index f11eeb416..ec1e10877 100644 --- a/crates/sqlquack-syntax-coverage/src/report/tests/test_types.rs +++ b/crates/sqlquack-syntax-coverage/src/report/tests/test_types.rs @@ -18,4 +18,5 @@ pub(crate) struct ParserChangeTestCase { pub(crate) head: fn(&str) -> LocalVerdict, pub(crate) expected_parsed_cases_refused: Vec<&'static str>, pub(crate) expected_syntax_error_cases_accepted: Vec<&'static str>, + pub(crate) expected_documented_cases_accepted: Vec<&'static str>, } diff --git a/docs/syntax-coverage.md b/docs/syntax-coverage.md index 8e6ae6130..0cdfa6ef4 100644 --- a/docs/syntax-coverage.md +++ b/docs/syntax-coverage.md @@ -273,6 +273,15 @@ with `captured` in place of `cases`: a 603 record on a construct no obligation r whose retired cases are not resolved against the cases file. Diagnostics are format-checked and listed, never scored. The checker ignores `semantic-limits/` and `lane-reviews/`. +## Marks (`marks/.jsonl`) + +An optional file per family in which the catalogue records how it settled a case: one JSON object +per line with `case_id`, `sql_sha256`, `obligations` and `mark`. `mark` is `documented (D113)` for a +case that follows its documentation against Snowflake's syntax error, or `settled accept (D133)` or +`settled reject (D133)`. Each mark must name a case with that `sql_sha256` and those `obligations`, +and a case is marked at most once; any other value is refused. The checker uses the documented +marks in `diff` only. + ## Reviews (`reviews/*.toml`) A `[[review]]` has these fields: @@ -391,8 +400,14 @@ results on stdout and progress on stderr: part of `make check-ci`, runs `diff` against it and fails in two cases. The first is a coverage regression: a form the baseline records as verified or excluded that no longer is, a satisfied obligation that no longer is, or a receipted case Snowflake parsed that the base parser accepted -and the current one rejects. That last line catches a case that moves from an opaque acceptance, -which satisfies no obligation, to a rejection; a pull request that has one explains each case. A +and the current one rejects, or a case Snowflake refused as syntax that the base parser rejected +and the current one accepts. Both join each case to its receipts by SQL, setup and session hashes, +whatever the case expects: Snowflake parsed a case when every receipt has a non-syntax phase and an +errno other than 1003, and refused it when every receipt is a syntax error with errno 1003 or 921; +a positionless 1003 or receipts that disagree count as neither. The first line catches a case that +moves from an opaque acceptance, which satisfies no obligation, to a rejection; a pull request that +has one explains each case. A case marked `documented (D113)` that the current parser now accepts +is listed on its own line for the auditor and does not fail the diff (decision D135). A snapshot taken before these case lists existed has none, so it reports no such case. The second is a catalogue change: a changed sources, aliases or catalogue file, or a form added or removed. Catalogue changes are reported separately from coverage changes. Gains pass. After a reviewed gain or catalogue change, re-record the baseline