Repository: woodruffw/zizmor (language: python)

Issue:
Add golden/snapshot tests
See #55.

@ubiratansoares added some amazing acceptance tests with #97, but we can take it one step further and use golden/snapshot/"known-answer" style tests to both increase our test coverage *and* reduce the amount of manual test code that needs to be written.

These will complement (**not** replace) the acceptance tests, since explicit acceptance tests are still valuable for handling concrete cases/adding backstop tests for regressions.

`insta` looks like the ideal library for this: <https://insta.rs/>


Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index f850b3f..4d814ec 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -886,6 +886,18 @@ dependencies = [
  "web-time",
 ]
 
+[[package]]
+name = "insta"
+version = "1.41.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7e9ffc4d4892617c50a928c52b2961cb5174b6fc6ebf252b2fac9d21955c48b8"
+dependencies = [
+ "console",
+ "lazy_static",
+ "linked-hash-map",
+ "similar",
+]
+
 [[package]]
 name = "inventory"
 version = "0.3.15"
@@ -940,6 +952,12 @@ version = "0.2.166"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "c2ccc108bbc0b1331bd061864e7cd823c0cab660bbe6970e66e2c0614decde36"
 
+[[package]]
+name = "linked-hash-map"
+version = "0.5.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f"
+
 [[package]]
 name = "linux-raw-sys"
 version = "0.4.14"
@@ -1723,6 +1741,12 @@ version = "1.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64"
 
+[[package]]
+name = "similar"
+version = "2.6.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1de1d4f81173b03af4c0cbed3c898f6bff5b870e4a7f5d6f4057d62a7a4b686e"
+
 [[package]]
 name = "slab"
 version = "0.4.9"
@@ -2555,6 +2579,7 @@ dependencies = [
  "github-actions-models",
  "human-panic",
  "indicatif",
+ "insta",
  "itertools",
  "log",
  "moka",
diff --git a/Cargo.toml b/Cargo.toml
index 66ee63d..83eec7c 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -36,10 +36,15 @@ tree-sitter = "0.23.2"
 tree-sitter-bash = "0.23.3"
 yamlpath = "0.12.0"
 
+[profile.dev.package]
+insta.opt-level = 3
+similar.opt-level = 3
+
 [profile.release]
 lto = true
 
 [dev-dependencies]
 assert_cmd = "2.0.16"
+insta = { version = "1.41.1" }
 pretty_assertions = "1.4.1"
 serde_json_path = "0.7.1"
diff --git a/docs/development.md b/docs/development.md
index 5f93cb6..91349dd 100644
--- a/docs/development.md
+++ b/docs/development.md
@@ -62,7 +62,7 @@ cargo doc
 cargo doc --open
 ```
 
-### Linting
+## Formatting and linting
 
 `zizmor` is linted with `cargo clippy` and auto-formatted with `cargo fmt`.
 Our CI enforces both, but you should also run them locally to minimize
@@ -73,20 +73,69 @@ cargo fmt
 cargo clippy --fix
 ```
 
-### Testing
+## Testing
 
-`zizmor` uses `cargo test`:
+`zizmor` has both unit and integration tests, and uses `cargo test` to
+orchestrate both of them.
 
 ```bash
+# run only unit tests
+cargo test --bins
+
+# run specific integration tests
+cargo test --test acceptance
+cargo test --test snapshot
+
+# run all of the tests
 cargo test
 ```
 
+### Writing snapshot tests
+
+`zizmor` uses @mitsuhiko/insta for snapshot testing.
+
+The easiest way to use `insta` is to install `cargo-insta`:
+
+```bash
+cargo install --locked cargo-insta
+```
+
+Snapshot tests are useful for a handful of scenarios:
+
+1. For cases when normal acceptance integration tests are too tedious to write;
+1. For regression detection with specific user-submitted workflows;
+1. For testing `zizmor`'s exact output/behavior on error scenarios.
+
+To add a new snapshot test, edit `tests/snapshot.rs` and add (or modify)
+an appropriate test function. You can use the existing ones for reference.
+
+When a new snapshot test is added, `cargo test` will run it and then fail,
+since the new snapshot has not yet been *accepted*. The easiest way to
+accept the new snapshot (or accept changes to other snapshot tests)
+is to use `cargo insta`, as installed above:
+
+```bash
+# run all the tests, generating new snapshots as necessary
+cargo insta test
+
+# review the new snapshots generated above
+cargo insta review
+```
+
+or, as a shortcut:
+
+```bash
+cargo insta test --review
+```
+
+See [insta's documentation] for more details.
+
 ## Building the website
 
 `zizmor`'s website is built with [MkDocs](https://www.mkdocs.org/), which
 means you'll need a Python runtime to develop against it locally.
 
-The easiest way to do this is to use [`uv`](https://github.com/astral-sh/uv),
+The easiest way to do this is to use @astral-sh/uv,
 which is what `zizmor`'s own CI uses. See
 [the `uv` docs](https://docs.astral.sh/uv/getting-started/installation/) for
 installation instructions.
@@ -205,3 +254,5 @@ make snippets
 [clap]: https://docs.rs/clap/latest/clap/index.html
 
 [clap-derive]: https://docs.rs/clap/latest/clap/_derive/index.html
+
+[insta's documentation]: https://insta.rs/docs/


Test patch:
diff --git a/tests/acceptance.rs b/tests/acceptance.rs
index 1476b4f..b7a35f4 100644
--- a/tests/acceptance.rs
+++ b/tests/acceptance.rs
@@ -1,7 +1,9 @@
 use assert_cmd::Command;
+use common::workflow_under_test;
 use serde_json::Value;
 use serde_json_path::JsonPath;
-use std::env::current_dir;
+
+mod common;
 
 // Acceptance tests for zizmor, on top of Json output
 // For now we don't cover tests that depends on Github API under the hood
@@ -13,17 +15,6 @@ fn zizmor() -> Command {
     cmd
 }
 
-fn workflow_under_test(name: &str) -> String {
-    let current_dir = current_dir().expect("Cannot figure out current directory");
-
-    let file_path = current_dir.join("tests").join("test-data").join(name);
-
-    file_path
-        .to_str()
-        .expect("Cannot create string reference for file path")
-        .to_string()
-}
-
 fn assert_value_match(json: &Value, path_pattern: &str, value: &str) {
     let json_path = JsonPath::parse(path_pattern).expect("Cannot evaluate json path");
     let queried = json_path
diff --git a/tests/common.rs b/tests/common.rs
new file mode 100644
index 0000000..1fd7520
--- /dev/null
+++ b/tests/common.rs
@@ -0,0 +1,12 @@
+use std::env::current_dir;
+
+pub fn workflow_under_test(name: &str) -> String {
+    let current_dir = current_dir().expect("Cannot figure out current directory");
+
+    let file_path = current_dir.join("tests").join("test-data").join(name);
+
+    file_path
+        .to_str()
+        .expect("Cannot create string reference for file path")
+        .to_string()
+}
diff --git a/tests/snapshot.rs b/tests/snapshot.rs
new file mode 100644
index 0000000..1e8c4ea
--- /dev/null
+++ b/tests/snapshot.rs
@@ -0,0 +1,61 @@
+use anyhow::Result;
+use assert_cmd::Command;
+use common::workflow_under_test;
+
+mod common;
+
+fn zizmor(workflow: Option<&str>, args: &[&str], stderr: bool) -> Result<String> {
+    let mut cmd = Command::cargo_bin("zizmor")?;
+    // All tests are currently offline.
+    cmd.args(["--offline"]);
+    cmd.args(args);
+
+    if let Some(workflow) = workflow {
+        cmd.arg(workflow);
+    }
+
+    let output = cmd.output()?;
+    let mut raw = if stderr {
+        String::from_utf8(output.stderr)?
+    } else {
+        String::from_utf8(output.stdout)?
+    };
+
+    // Normalize/replace any workflow paths to make them
+    // reproducible across different machines.
+    if let Some(workflow) = workflow {
+        raw = raw.replace(workflow, "@@INPUT@@");
+    }
+
+    Ok(raw)
+}
+
+#[test]
+fn self_hosted() -> Result<()> {
+    insta::assert_snapshot!(zizmor(
+        Some(&workflow_under_test("self-hosted.yml")),
+        &["--pedantic"],
+        false
+    )?);
+
+    Ok(insta::assert_snapshot!(zizmor(
+        Some(&workflow_under_test("self-hosted.yml")),
+        &[],
+        false
+    )?))
+}
+
+#[test]
+fn unpinned_uses() -> Result<()> {
+    insta::assert_snapshot!(zizmor(
+        Some(&workflow_under_test("unpinned-uses.yml")),
+        &["--pedantic"],
+        false
+    )?);
+
+    Ok(insta::assert_snapshot!(zizmor(
+        Some(&workflow_under_test("unpinned-uses.yml")),
+        &[],
+        false
+    )?))
+}
diff --git a/tests/snapshots/snapshot__self_hosted-2.snap b/tests/snapshots/snapshot__self_hosted-2.snap
new file mode 100644
index 0000000..eb3f5dc
--- /dev/null
+++ b/tests/snapshots/snapshot__self_hosted-2.snap
@@ -0,0 +1,6 @@
+---
+source: tests/snapshot.rs
+expression: "zizmor(&workflow_under_test(\"self-hosted.yml\"), &[], false)?"
+snapshot_kind: text
+---
+No findings to report. Good job!
diff --git a/tests/snapshots/snapshot__self_hosted.snap b/tests/snapshots/snapshot__self_hosted.snap
new file mode 100644
index 0000000..9718a98
--- /dev/null
+++ b/tests/snapshots/snapshot__self_hosted.snap
@@ -0,0 +1,14 @@
+---
+source: tests/snapshot.rs
+expression: "zizmor(&workflow_under_test(\"self-hosted.yml\"), &[\"--pedantic\"], false)?"
+snapshot_kind: text
+---
+note[self-hosted-runner]: runs on a self-hosted runner
+ --> @@INPUT@@:8:5
+  |
+8 |     runs-on: [self-hosted, my-ubuntu-box]
+  |     ------------------------------------- note: self-hosted runner used here
+  |
+  = note: audit confidence → High
+
+1 finding: 1 unknown, 0 informational, 0 low, 0 medium, 0 high
diff --git a/tests/snapshots/snapshot__unpinned_uses-2.snap b/tests/snapshots/snapshot__unpinned_uses-2.snap
new file mode 100644
index 0000000..1f926f6
--- /dev/null
+++ b/tests/snapshots/snapshot__unpinned_uses-2.snap
@@ -0,0 +1,38 @@
+---
+source: tests/snapshot.rs
+expression: "zizmor(Some(&workflow_under_test(\"unpinned-uses.yml\")), &[], false)?"
+snapshot_kind: text
+---
+warning[unpinned-uses]: unpinned action reference
+ --> @@INPUT@@:9:9
+  |
+9 |       - uses: actions/checkout
+  |         ---------------------- action is not pinned to a tag, branch, or hash ref
+  |
+  = note: audit confidence → High
+
+warning[unpinned-uses]: unpinned action reference
+  --> @@INPUT@@:19:9
+   |
+19 |       - uses: github/codeql-action/upload-sarif
+   |         --------------------------------------- action is not pinned to a tag, branch, or hash ref
+   |
+   = note: audit confidence → High
+
+warning[unpinned-uses]: unpinned action reference
+  --> @@INPUT@@:22:9
+   |
+22 |       - uses: docker://ubuntu
+   |         --------------------- action is not pinned to a tag, branch, or hash ref
+   |
+   = note: audit confidence → High
+
+warning[unpinned-uses]: unpinned action reference
+  --> @@INPUT@@:28:9
+   |
+28 |       - uses: docker://ghcr.io/pypa/gh-action-pypi-publish
+   |         -------------------------------------------------- action is not pinned to a tag, branch, or hash ref
+   |
+   = note: audit confidence → High
+
+4 findings: 0 unknown, 0 informational, 0 low, 4 medium, 0 high
diff --git a/tests/snapshots/snapshot__unpinned_uses.snap b/tests/snapshots/snapshot__unpinned_uses.snap
new file mode 100644
index 0000000..ccf2b86
--- /dev/null
+++ b/tests/snapshots/snapshot__unpinned_uses.snap
@@ -0,0 +1,46 @@
+---
+source: tests/snapshot.rs
+expression: "zizmor(Some(&workflow_under_test(\"unpinned-uses.yml\")), &[\"--pedantic\"],\nfalse)?"
+snapshot_kind: text
+---
+warning[unpinned-uses]: unpinned action reference
+ --> @@INPUT@@:9:9
+  |
+9 |       - uses: actions/checkout
+  |         ---------------------- action is not pinned to a tag, branch, or hash ref
+  |
+  = note: audit confidence → High
+
+help[unpinned-uses]: unpinned action reference
+  --> @@INPUT@@:14:9
+   |
+14 |       - uses: actions/checkout@v3
+   |         ------------------------- help: action is not pinned to a hash ref
+   |
+   = note: audit confidence → High
+
+warning[unpinned-uses]: unpinned action reference
+  --> @@INPUT@@:19:9
+   |
+19 |       - uses: github/codeql-action/upload-sarif
+   |         --------------------------------------- action is not pinned to a tag, branch, or hash ref
+   |
+   = note: audit confidence → High
+
+warning[unpinned-uses]: unpinned action reference
+  --> @@INPUT@@:22:9
+   |
+22 |       - uses: docker://ubuntu
+   |         --------------------- action is not pinned to a tag, branch, or hash ref
+   |
+   = note: audit confidence → High
+
+warning[unpinned-uses]: unpinned action reference
+  --> @@INPUT@@:28:9
+   |
+28 |       - uses: docker://ghcr.io/pypa/gh-action-pypi-publish
+   |         -------------------------------------------------- action is not pinned to a tag, branch, or hash ref
+   |
+   = note: audit confidence → High
+
+5 findings: 0 unknown, 0 informational, 1 low, 4 medium, 0 high
diff --git a/tests/test-data/unpinned-uses.yml b/tests/test-data/unpinned-uses.yml
index 37ada56..4081000 100644
--- a/tests/test-data/unpinned-uses.yml
+++ b/tests/test-data/unpinned-uses.yml
@@ -10,6 +10,11 @@ jobs:
         with:
           persist-credentials: false
 
+      # PEDANTIC: pinned but unhashed
+      - uses: actions/checkout@v3
+        with:
+          persist-credentials: false
+
       # NOT OK: unpinned
       - uses: github/codeql-action/upload-sarif