Merge pull request #1107 from cgwalters/lint-more5

lints: More tests && add --skip

Author: cgwalters

lib/src/cli.rs

@@ -257,6 +257,13 @@ pub(crate) enum ContainerOpts {
         /// maintaining this exact format; do not parse it via code or scripts.
         #[clap(long)]
         list: bool,
+
+        /// Skip checking the targeted lints, by name. Use `--list` to discover the set
+        /// of available lints.
+        ///
+        /// Example: --skip nonempty-boot --skip baseimage-root
+        #[clap(long)]
+        skip: Vec<String>,
     },
 }
 
@@ -1044,6 +1051,7 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
                 rootfs,
                 fatal_warnings,
                 list,
+                skip,
             } => {
                 if list {
                     return lints::lint_list(std::io::stdout().lock());
@@ -1060,7 +1068,8 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
                 };
 
                 let root = &Dir::open_ambient_dir(rootfs, cap_std::ambient_authority())?;
-                lints::lint(root, warnings, root_type, std::io::stdout().lock())?;
+                let skip = skip.iter().map(|s| s.as_str());
+                lints::lint(root, warnings, root_type, skip, std::io::stdout().lock())?;
                 Ok(())
             }
         },

lib/src/lints.rs

@@ -132,26 +132,37 @@ pub(crate) fn lint_list(output: impl std::io::Write) -> Result<()> {
     Ok(())
 }
 
-/// check for the existence of the /var/run directory
-/// if it exists we need to check that it links to /run if not error
-/// if it does not exist error.
-#[context("Linting")]
-pub(crate) fn lint(
+#[derive(Debug)]
+struct LintExecutionResult {
+    warnings: usize,
+    passed: usize,
+    skipped: usize,
+    fatal: usize,
+}
+
+fn lint_inner<'skip>(
     root: &Dir,
-    warning_disposition: WarningDisposition,
     root_type: RootType,
+    skip: impl IntoIterator<Item = &'skip str>,
     mut output: impl std::io::Write,
-) -> Result<()> {
+) -> Result<LintExecutionResult> {
     let mut fatal = 0usize;
     let mut warnings = 0usize;
     let mut passed = 0usize;
     let mut skipped = 0usize;
+    let skip: std::collections::HashSet<_> = skip.into_iter().collect();
     for lint in LINTS {
         let name = lint.name;
 
+        if skip.contains(name) {
+            skipped += 1;
+            continue;
+        }
+
         if let Some(lint_root_type) = lint.root_type {
             if lint_root_type != root_type {
                 skipped += 1;
+                continue;
             }
         }
 
@@ -177,20 +188,41 @@ pub(crate) fn lint(
             passed += 1;
         }
     }
-    writeln!(output, "Checks passed: {passed}")?;
-    if skipped > 0 {
-        writeln!(output, "Checks skipped: {skipped}")?;
+
+    Ok(LintExecutionResult {
+        passed,
+        skipped,
+        warnings,
+        fatal,
+    })
+}
+
+/// check for the existence of the /var/run directory
+/// if it exists we need to check that it links to /run if not error
+/// if it does not exist error.
+#[context("Linting")]
+pub(crate) fn lint<'skip>(
+    root: &Dir,
+    warning_disposition: WarningDisposition,
+    root_type: RootType,
+    skip: impl IntoIterator<Item = &'skip str>,
+    mut output: impl std::io::Write,
+) -> Result<()> {
+    let r = lint_inner(root, root_type, skip, &mut output)?;
+    writeln!(output, "Checks passed: {}", r.passed)?;
+    if r.skipped > 0 {
+        writeln!(output, "Checks skipped: {}", r.skipped)?;
     }
     let fatal = if matches!(warning_disposition, WarningDisposition::FatalWarnings) {
-        fatal + warnings
+        r.fatal + r.warnings
     } else {
-        fatal
+        r.fatal
     };
-    if warnings > 0 {
-        writeln!(output, "Warnings: {warnings}")?;
+    if r.warnings > 0 {
+        writeln!(output, "Warnings: {}", r.warnings)?;
     }
     if fatal > 0 {
-        anyhow::bail!("Checks failed: {fatal}")
+        anyhow::bail!("Checks failed: {}", fatal)
     }
     Ok(())
 }
@@ -509,11 +541,43 @@ mod tests {
         let root = &passing_fixture()?;
         let mut out = Vec::new();
         let warnings = WarningDisposition::FatalWarnings;
-        let root_type = RootType::Running;
-        lint(root, warnings, root_type, &mut out).unwrap();
+        let root_type = RootType::Alternative;
+        lint(root, warnings, root_type, [], &mut out).unwrap();
         root.create_dir_all("var/run/foo")?;
         let mut out = Vec::new();
-        assert!(lint(root, warnings, root_type, &mut out).is_err());
+        assert!(lint(root, warnings, root_type, [], &mut out).is_err());
+        Ok(())
+    }
+
+    #[test]
+    fn test_lint_inner() -> Result<()> {
+        let root = &passing_fixture()?;
+
+        // Verify that all lints run
+        let mut out = Vec::new();
+        let root_type = RootType::Alternative;
+        let r = lint_inner(root, root_type, [], &mut out).unwrap();
+        assert_eq!(r.passed, LINTS.len());
+        assert_eq!(r.fatal, 0);
+        assert_eq!(r.skipped, 0);
+        assert_eq!(r.warnings, 0);
+
+        let r = lint_inner(root, root_type, ["var-log"], &mut out).unwrap();
+        // Trigger a failure in var-log
+        root.create_dir_all("var/log/dnf")?;
+        root.write("var/log/dnf/dnf.log", b"dummy dnf log")?;
+        assert_eq!(r.passed, LINTS.len().checked_sub(1).unwrap());
+        assert_eq!(r.fatal, 0);
+        assert_eq!(r.skipped, 1);
+        assert_eq!(r.warnings, 0);
+
+        // But verify that not skipping it results in a warning
+        let mut out = Vec::new();
+        let r = lint_inner(root, root_type, [], &mut out).unwrap();
+        assert_eq!(r.passed, LINTS.len().checked_sub(1).unwrap());
+        assert_eq!(r.fatal, 0);
+        assert_eq!(r.skipped, 0);
+        assert_eq!(r.warnings, 1);
         Ok(())
     }