test: add Readme.md for macros, fix some bugs, add expectations (#205)

Author: juleswritescode

crates/pglt_analyse/src/rule.rs

@@ -263,6 +263,11 @@ impl RuleDiagnostic {
     pub fn advices(&self) -> &RuleAdvice {
         &self.rule_advice
     }
+
+    /// Will return the rule's category name as defined via `define_categories! { .. }`.
+    pub fn get_category_name(&self) -> &'static str {
+        self.category.name()
+    }
 }
 
 #[derive(Debug, Clone, Eq)]

crates/pglt_analyser/tests/rules_tests.rs

@@ -83,14 +83,25 @@ fn write_snapshot(snapshot: &mut String, query: &str, diagnostics: &[RuleDiagnos
 enum Expectation {
     NoDiagnostics,
     AnyDiagnostics,
+    OnlyOne(String),
 }
 
 impl Expectation {
     fn from_file(content: &str) -> Self {
         for line in content.lines() {
-            if line.contains("expect-no-diagnostics") {
+            if line.contains("expect_no_diagnostics") {
                 return Self::NoDiagnostics;
             }
+
+            if line.contains("expect_only_") {
+                let kind = line
+                    .splitn(3, "_")
+                    .last()
+                    .expect("Use pattern: `-- expect_only_<category>`")
+                    .trim();
+
+                return Self::OnlyOne(kind.into());
+            }
         }
 
         Self::AnyDiagnostics
@@ -103,7 +114,20 @@ impl Expectation {
                     panic!("This test should not have any diagnostics.");
                 }
             }
-            _ => {}
+            Self::OnlyOne(category) => {
+                let found_kinds = diagnostics
+                    .iter()
+                    .map(|d| d.get_category_name())
+                    .collect::<Vec<&str>>()
+                    .join(", ");
+
+                if diagnostics.len() != 1 || diagnostics[0].get_category_name() != category {
+                    panic!(
+                        "This test should only have one diagnostic of kind: {category}\nReceived: {found_kinds}"
+                    );
+                }
+            }
+            Self::AnyDiagnostics => {}
         }
     }
 }

crates/pglt_analyser/tests/specs/safety/banDropColumn/basic.sql

@@ -1,2 +1,3 @@
+-- expect_only_lint/safety/banDropColumn
 alter table test
 drop column id;

crates/pglt_analyser/tests/specs/safety/banDropColumn/basic.sql.snap

@@ -4,6 +4,7 @@ expression: snapshot
 ---
 # Input
 ```
+-- expect_only_lint/safety/banDropColumn
 alter table test
 drop column id;
 ```

crates/pglt_analyser/tests/specs/safety/banDropNotNull/basic.sql

@@ -1,3 +1,4 @@
+-- expect_only_lint/safety/banDropNotNull
 alter table users
 alter column id
 drop not null;

crates/pglt_analyser/tests/specs/safety/banDropNotNull/basic.sql.snap

@@ -4,6 +4,7 @@ expression: snapshot
 ---
 # Input
 ```
+-- expect_only_lint/safety/banDropNotNull
 alter table users
 alter column id
 drop not null;

crates/pglt_test_macros/README.md

@@ -0,0 +1,67 @@
+# Tests macros
+
+Macros to help auto-generate tests based on files.
+
+## Usage
+
+Pass a glob pattern that'll identify your files and a test-function that'll run for each file. The glob pattern has to start at the root of your crate.
+
+You can add a `.expected.` file next to your test file. Its path will be passed to your test function so you can make outcome-based assertions. (Alternatively, write snapshot tests.)
+
+Given the following file structure:
+
+```txt
+crate/
+|-- src/
+|-- tests/
+    |-- queries/
+        |-- test.sql
+        |-- test.expected.sql
+    |-- querytest.rs
+```
+
+You can generate tests like so:
+
+```rust
+  // crate/tests/querytest.rs
+
+  tests_macros::gen_tests!{
+    "tests/queries/*.sql",
+    crate::run_test  // use `crate::` if the linter complains.
+  }
+
+  fn run_test(
+    test_path: &str,  // absolute path on the machine
+    expected_path: &str, // absolute path of .expected file
+    test_dir: &str // absolute path of the test file's parent
+  ) {
+      // your logic
+  }
+```
+
+Given a `crate/tests/queries/some_test_abc.sql` file, this will generate the following:
+
+```rust
+#[test]
+pub fn some_test_abc()
+{
+    let test_file = "<crate>/tests/queries/some_test_abc.sql";
+    let test_expected_file = "<crate>/tests/queries/some_test_abc.expected.sql";
+    let parent = "<crate>/tests/queries";
+    run_test(test_file, test_expected_file, parent);
+}
+```
+
+This will be replicated for each file matched by the glob pattern.
+
+## Pitfalls
+
+- If you use a Rust-keyword as a file name, this'll result in invalid syntax for the generated tests.
+- You might get linting errors if your test files aren't snake case.
+- All files of the glob-pattern must (currently) be `.sql` files.
+- The `.expected.sql` file-name will always be passed, even if the file doesn't exist.
+- The macro will wrap your tests in a `mod tests { .. }` module. If you need multiple generations, wrap them in modules like so: `mod some_test { tests_macros::gen_tests! { .. } }`.
+
+## How to run
+
+Simply run your `cargo test` commands as usual.

crates/pglt_test_macros/src/lib.rs

@@ -197,38 +197,50 @@ struct Variables {
 impl TryFrom<PathBuf> for Variables {
     type Error = &'static str;
 
-    fn try_from(path: PathBuf) -> Result<Self, Self::Error> {
-        let test_name = path
+    fn try_from(mut path: PathBuf) -> Result<Self, Self::Error> {
+        let test_name: String = path
             .file_stem()
             .ok_or("Cannot get file stem.")?
             .to_str()
-            .ok_or("Cannot convert file stem to string.")?;
+            .ok_or("Cannot convert file stem to string.")?
+            .into();
 
-        let ext = path
+        let ext: String = path
             .extension()
             .ok_or("Cannot get extension.")?
             .to_str()
-            .ok_or("Cannot convert extension to string.")?;
+            .ok_or("Cannot convert extension to string.")?
+            .into();
         assert_eq!(ext, "sql", "Expected .sql extension but received: {}", ext);
 
-        let test_dir = path
+        let test_dir: String = path
             .parent()
             .ok_or("Cannot get parent directory.")?
             .to_str()
-            .ok_or("Cannot convert parent directory to string.")?;
+            .ok_or("Cannot convert parent directory to string.")?
+            .into();
 
-        let test_fullpath = path.to_str().ok_or("Cannot convert path to string.")?;
+        let test_fullpath: String = path
+            .as_os_str()
+            .to_str()
+            .ok_or("Cannot convert file stem to string.")?
+            .into();
+
+        path.set_extension(OsStr::new(""));
 
-        let mut without_ext = test_fullpath.to_string();
-        without_ext.pop();
+        let without_ext: String = path
+            .as_os_str()
+            .to_str()
+            .ok_or("Cannot convert file stem to string.")?
+            .into();
 
         let test_expected_fullpath = format!("{}.expected.{}", without_ext, ext);
 
         Ok(Variables {
-            test_name: test_name.into(),
-            test_fullpath: test_fullpath.into(),
-            test_expected_fullpath: test_expected_fullpath.into(),
-            test_dir: test_dir.into(),
+            test_name,
+            test_fullpath,
+            test_expected_fullpath,
+            test_dir,
         })
     }
 }

xtask/codegen/src/generate_new_analyser_rule.rs

@@ -73,9 +73,9 @@ impl Rule for {rule_name_upper_camel} {{
     )
 }
 
-static EXAMPLE_SQL: &'static str = r#"
-    -- select 1;
-"#;
+fn gen_sql(category_name: &str) -> String {
+    format!("-- expect_only_{category_name}\n-- select 1;").into()
+}
 
 pub fn generate_new_analyser_rule(category: Category, rule_name: &str, group: &str) {
     let rule_name_camel = Case::Camel.convert(rule_name);
@@ -143,7 +143,10 @@ pub fn generate_new_analyser_rule(category: Category, rule_name: &str, group: &s
         std::fs::create_dir(test_folder.clone()).expect("To create the test rule folder");
     }
 
-    let test_file_name = format!("{}/query.sql", test_folder.display());
-    std::fs::write(test_file_name.clone(), EXAMPLE_SQL)
-        .unwrap_or_else(|_| panic!("To write {}", &test_file_name));
+    let test_file_name = format!("{}/basic.sql", test_folder.display());
+    std::fs::write(
+        test_file_name.clone(),
+        gen_sql(format!("lint/{group}/{rule_name_camel}").as_str()),
+    )
+    .unwrap_or_else(|_| panic!("To write {}", &test_file_name));
 }