From 40af5a67b1c392edc4b61648c142483938a1e651 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 20 Dec 2024 08:50:53 -0500 Subject: [PATCH] tree-wide: Move most tests under `mod test` This is a general best practice; specifically motivated by handling test-specific imports. Signed-off-by: Colin Walters --- lib/src/blockdev.rs | 36 +-- lib/src/cli.rs | 231 +++++++-------- lib/src/generator.rs | 94 ++++--- lib/src/glyph.rs | 11 +- lib/src/install/config.rs | 578 +++++++++++++++++++------------------- lib/src/kernel.rs | 17 +- lib/src/lints.rs | 258 ++++++++--------- lib/src/utils.rs | 87 +++--- 8 files changed, 672 insertions(+), 640 deletions(-) diff --git a/lib/src/blockdev.rs b/lib/src/blockdev.rs index 4532c49f3..c086868db 100644 --- a/lib/src/blockdev.rs +++ b/lib/src/blockdev.rs @@ -334,28 +334,28 @@ pub(crate) fn parse_size_mib(mut s: &str) -> Result { Ok(v * mul) } -#[test] -fn test_parse_size_mib() { - let ident_cases = [0, 10, 9, 1024].into_iter().map(|k| (k.to_string(), k)); - let cases = [ - ("0M", 0), - ("10M", 10), - ("10MiB", 10), - ("1G", 1024), - ("9G", 9216), - ("11T", 11 * 1024 * 1024), - ] - .into_iter() - .map(|(k, v)| (k.to_string(), v)); - for (s, v) in ident_cases.chain(cases) { - assert_eq!(parse_size_mib(&s).unwrap(), v as u64, "Parsing {s}"); - } -} - #[cfg(test)] mod test { use super::*; + #[test] + fn test_parse_size_mib() { + let ident_cases = [0, 10, 9, 1024].into_iter().map(|k| (k.to_string(), k)); + let cases = [ + ("0M", 0), + ("10M", 10), + ("10MiB", 10), + ("1G", 1024), + ("9G", 9216), + ("11T", 11 * 1024 * 1024), + ] + .into_iter() + .map(|(k, v)| (k.to_string(), v)); + for (s, v) in ident_cases.chain(cases) { + assert_eq!(parse_size_mib(&s).unwrap(), v as u64, "Parsing {s}"); + } + } + #[test] fn test_parse_sfdisk() -> Result<()> { let fixture = indoc::indoc! { r#" diff --git a/lib/src/cli.rs b/lib/src/cli.rs index 4e2e38d48..c3b441893 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -1125,127 +1125,132 @@ async fn run_from_opt(opt: Opt) -> Result<()> { } } -#[test] -fn test_callname() { - use std::os::unix::ffi::OsStrExt; - - // Cases that change - let mapped_cases = [ - ("", "bootc"), - ("/foo/bar", "bar"), - ("/foo/bar/", "bar"), - ("foo/bar", "bar"), - ("../foo/bar", "bar"), - ("usr/bin/ostree-container", "ostree-container"), - ]; - for (input, output) in mapped_cases { - assert_eq!( - output, - callname_from_argv0(OsStr::new(input)), - "Handling mapped case {input}" - ); - } +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_callname() { + use std::os::unix::ffi::OsStrExt; + + // Cases that change + let mapped_cases = [ + ("", "bootc"), + ("/foo/bar", "bar"), + ("/foo/bar/", "bar"), + ("foo/bar", "bar"), + ("../foo/bar", "bar"), + ("usr/bin/ostree-container", "ostree-container"), + ]; + for (input, output) in mapped_cases { + assert_eq!( + output, + callname_from_argv0(OsStr::new(input)), + "Handling mapped case {input}" + ); + } - // Invalid UTF-8 - assert_eq!("bootc", callname_from_argv0(OsStr::from_bytes(b"foo\x80"))); + // Invalid UTF-8 + assert_eq!("bootc", callname_from_argv0(OsStr::from_bytes(b"foo\x80"))); + + // Cases that are identical + let ident_cases = ["foo", "bootc"]; + for case in ident_cases { + assert_eq!( + case, + callname_from_argv0(OsStr::new(case)), + "Handling ident case {case}" + ); + } + } - // Cases that are identical - let ident_cases = ["foo", "bootc"]; - for case in ident_cases { + #[test] + fn test_parse_install_args() { + // Verify we still process the legacy --target-no-signature-verification + let o = Opt::try_parse_from([ + "bootc", + "install", + "to-filesystem", + "--target-no-signature-verification", + "/target", + ]) + .unwrap(); + let o = match o { + Opt::Install(InstallOpts::ToFilesystem(fsopts)) => fsopts, + o => panic!("Expected filesystem opts, not {o:?}"), + }; + assert!(o.target_opts.target_no_signature_verification); + assert_eq!(o.filesystem_opts.root_path.as_str(), "/target"); + // Ensure we default to old bound images behavior assert_eq!( - case, - callname_from_argv0(OsStr::new(case)), - "Handling ident case {case}" + o.config_opts.bound_images, + crate::install::BoundImagesOpt::Stored ); } -} - -#[test] -fn test_parse_install_args() { - // Verify we still process the legacy --target-no-signature-verification - let o = Opt::try_parse_from([ - "bootc", - "install", - "to-filesystem", - "--target-no-signature-verification", - "/target", - ]) - .unwrap(); - let o = match o { - Opt::Install(InstallOpts::ToFilesystem(fsopts)) => fsopts, - o => panic!("Expected filesystem opts, not {o:?}"), - }; - assert!(o.target_opts.target_no_signature_verification); - assert_eq!(o.filesystem_opts.root_path.as_str(), "/target"); - // Ensure we default to old bound images behavior - assert_eq!( - o.config_opts.bound_images, - crate::install::BoundImagesOpt::Stored - ); -} -#[test] -fn test_parse_opts() { - assert!(matches!( - Opt::parse_including_static(["bootc", "status"]), - Opt::Status(StatusOpts { - json: false, - format: None, - format_version: None, - booted: false - }) - )); - assert!(matches!( - Opt::parse_including_static(["bootc", "status", "--format-version=0"]), - Opt::Status(StatusOpts { - format_version: Some(0), - .. - }) - )); -} + #[test] + fn test_parse_opts() { + assert!(matches!( + Opt::parse_including_static(["bootc", "status"]), + Opt::Status(StatusOpts { + json: false, + format: None, + format_version: None, + booted: false + }) + )); + assert!(matches!( + Opt::parse_including_static(["bootc", "status", "--format-version=0"]), + Opt::Status(StatusOpts { + format_version: Some(0), + .. + }) + )); + } -#[test] -fn test_parse_generator() { - assert!(matches!( - Opt::parse_including_static([ - "/usr/lib/systemd/system/bootc-systemd-generator", - "/run/systemd/system" - ]), - Opt::Internals(InternalsOpts::SystemdGenerator { normal_dir, .. }) if normal_dir == "/run/systemd/system" - )); -} + #[test] + fn test_parse_generator() { + assert!(matches!( + Opt::parse_including_static([ + "/usr/lib/systemd/system/bootc-systemd-generator", + "/run/systemd/system" + ]), + Opt::Internals(InternalsOpts::SystemdGenerator { normal_dir, .. }) if normal_dir == "/run/systemd/system" + )); + } -#[test] -fn test_parse_ostree_ext() { - assert!(matches!( - Opt::parse_including_static(["bootc", "internals", "ostree-container"]), - Opt::Internals(InternalsOpts::OstreeContainer { .. }) - )); - - fn peel(o: Opt) -> Vec { - match o { - Opt::Internals(InternalsOpts::OstreeExt { args }) => args, - o => panic!("unexpected {o:?}"), + #[test] + fn test_parse_ostree_ext() { + assert!(matches!( + Opt::parse_including_static(["bootc", "internals", "ostree-container"]), + Opt::Internals(InternalsOpts::OstreeContainer { .. }) + )); + + fn peel(o: Opt) -> Vec { + match o { + Opt::Internals(InternalsOpts::OstreeExt { args }) => args, + o => panic!("unexpected {o:?}"), + } } - } - let args = peel(Opt::parse_including_static([ - "/usr/libexec/libostree/ext/ostree-ima-sign", - "ima-sign", - "--repo=foo", - "foo", - "bar", - "baz", - ])); - assert_eq!( - args.as_slice(), - ["ima-sign", "--repo=foo", "foo", "bar", "baz"] - ); + let args = peel(Opt::parse_including_static([ + "/usr/libexec/libostree/ext/ostree-ima-sign", + "ima-sign", + "--repo=foo", + "foo", + "bar", + "baz", + ])); + assert_eq!( + args.as_slice(), + ["ima-sign", "--repo=foo", "foo", "bar", "baz"] + ); - let args = peel(Opt::parse_including_static([ - "/usr/libexec/libostree/ext/ostree-container", - "container", - "image", - "pull", - ])); - assert_eq!(args.as_slice(), ["container", "image", "pull"]); + let args = peel(Opt::parse_including_static([ + "/usr/libexec/libostree/ext/ostree-container", + "container", + "image", + "pull", + ])); + assert_eq!(args.as_slice(), ["container", "image", "pull"]); + } } diff --git a/lib/src/generator.rs b/lib/src/generator.rs index b0ff1566c..d1fd4bf31 100644 --- a/lib/src/generator.rs +++ b/lib/src/generator.rs @@ -88,55 +88,58 @@ ExecStart=bootc internals fixup-etc-fstab\n\ } #[cfg(test)] -fn fixture() -> Result { - let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; - tempdir.create_dir("etc")?; - tempdir.create_dir("run")?; - tempdir.create_dir_all("run/systemd/system")?; - Ok(tempdir) -} - -#[test] -fn test_generator_no_fstab() -> Result<()> { - let tempdir = fixture()?; - let unit_dir = &tempdir.open_dir("run/systemd/system")?; - fstab_generator_impl(&tempdir, &unit_dir).unwrap(); - - assert_eq!(unit_dir.entries()?.count(), 0); - Ok(()) -} - -#[cfg(test)] -mod test { +mod tests { use super::*; - use ostree_ext::container_utils::OSTREE_BOOTED; + fn fixture() -> Result { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + tempdir.create_dir("etc")?; + tempdir.create_dir("run")?; + tempdir.create_dir_all("run/systemd/system")?; + Ok(tempdir) + } #[test] - fn test_generator_fstab() -> Result<()> { + fn test_generator_no_fstab() -> Result<()> { let tempdir = fixture()?; let unit_dir = &tempdir.open_dir("run/systemd/system")?; - // Should still be a no-op - tempdir.atomic_write("etc/fstab", "# Some dummy fstab")?; fstab_generator_impl(&tempdir, &unit_dir).unwrap(); - assert_eq!(unit_dir.entries()?.count(), 0); - // Also a no-op, not booted via ostree - tempdir.atomic_write("etc/fstab", &format!("# {FSTAB_ANACONDA_STAMP}"))?; - fstab_generator_impl(&tempdir, &unit_dir).unwrap(); assert_eq!(unit_dir.entries()?.count(), 0); - - // Now it should generate - tempdir.atomic_write(OSTREE_BOOTED, "ostree booted")?; - fstab_generator_impl(&tempdir, &unit_dir).unwrap(); - assert_eq!(unit_dir.entries()?.count(), 2); - Ok(()) } - #[test] - fn test_generator_fstab_idempotent() -> Result<()> { - let anaconda_fstab = indoc::indoc! { " + #[cfg(test)] + mod test { + use super::*; + + use ostree_ext::container_utils::OSTREE_BOOTED; + + #[test] + fn test_generator_fstab() -> Result<()> { + let tempdir = fixture()?; + let unit_dir = &tempdir.open_dir("run/systemd/system")?; + // Should still be a no-op + tempdir.atomic_write("etc/fstab", "# Some dummy fstab")?; + fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + assert_eq!(unit_dir.entries()?.count(), 0); + + // Also a no-op, not booted via ostree + tempdir.atomic_write("etc/fstab", &format!("# {FSTAB_ANACONDA_STAMP}"))?; + fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + assert_eq!(unit_dir.entries()?.count(), 0); + + // Now it should generate + tempdir.atomic_write(OSTREE_BOOTED, "ostree booted")?; + fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + assert_eq!(unit_dir.entries()?.count(), 2); + + Ok(()) + } + + #[test] + fn test_generator_fstab_idempotent() -> Result<()> { + let anaconda_fstab = indoc::indoc! { " # # /etc/fstab # Created by anaconda on Tue Mar 19 12:24:29 2024 @@ -151,15 +154,16 @@ mod test { UUID=715be2b7-c458-49f2-acec-b2fdb53d9089 / xfs ro 0 0 UUID=341c4712-54e8-4839-8020-d94073b1dc8b /boot xfs defaults 0 0 " }; - let tempdir = fixture()?; - let unit_dir = &tempdir.open_dir("run/systemd/system")?; + let tempdir = fixture()?; + let unit_dir = &tempdir.open_dir("run/systemd/system")?; - tempdir.atomic_write("etc/fstab", anaconda_fstab)?; - tempdir.atomic_write(OSTREE_BOOTED, "ostree booted")?; - let updated = fstab_generator_impl(&tempdir, &unit_dir).unwrap(); - assert!(!updated); - assert_eq!(unit_dir.entries()?.count(), 0); + tempdir.atomic_write("etc/fstab", anaconda_fstab)?; + tempdir.atomic_write(OSTREE_BOOTED, "ostree booted")?; + let updated = fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + assert!(!updated); + assert_eq!(unit_dir.entries()?.count(), 0); - Ok(()) + Ok(()) + } } } diff --git a/lib/src/glyph.rs b/lib/src/glyph.rs index 9ac51cc06..2f60953b3 100644 --- a/lib/src/glyph.rs +++ b/lib/src/glyph.rs @@ -30,7 +30,12 @@ impl Display for Glyph { } } -#[test] -fn test_glyph() { - assert_eq!(Glyph::BlackCircle.as_utf8(), "●"); +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_glyph() { + assert_eq!(Glyph::BlackCircle.as_utf8(), "●"); + } } diff --git a/lib/src/install/config.rs b/lib/src/install/config.rs index b0fe3e76a..b8c870fff 100644 --- a/lib/src/install/config.rs +++ b/lib/src/install/config.rs @@ -213,340 +213,344 @@ pub(crate) fn load_config() -> Result> { Ok(config) } -#[test] -/// Verify that we can parse our default config file -fn test_parse_config() { - use super::baseline::Filesystem; +#[cfg(test)] +mod tests { + use super::*; - let env = EnvProperties { - sys_arch: "x86_64".to_string(), - }; - let c: InstallConfigurationToplevel = toml::from_str( - r##"[install] + use super::super::baseline::Filesystem; + + #[test] + /// Verify that we can parse our default config file + fn test_parse_config() { + let env = EnvProperties { + sys_arch: "x86_64".to_string(), + }; + let c: InstallConfigurationToplevel = toml::from_str( + r##"[install] root-fs-type = "xfs" "##, - ) - .unwrap(); - let mut install = c.install.unwrap(); - assert_eq!(install.root_fs_type.unwrap(), Filesystem::Xfs); - let other = InstallConfigurationToplevel { - install: Some(InstallConfiguration { - root_fs_type: Some(Filesystem::Ext4), - ..Default::default() - }), - }; - install.merge(other.install.unwrap(), &env); - assert_eq!( - install.root_fs_type.as_ref().copied().unwrap(), - Filesystem::Ext4 - ); - // This one shouldn't have been set - assert!(install.filesystem_root().is_none()); - install.canonicalize(); - assert_eq!(install.root_fs_type.as_ref().unwrap(), &Filesystem::Ext4); - assert_eq!( - install.filesystem_root().unwrap().fstype.unwrap(), - Filesystem::Ext4 - ); + ) + .unwrap(); + let mut install = c.install.unwrap(); + assert_eq!(install.root_fs_type.unwrap(), Filesystem::Xfs); + let other = InstallConfigurationToplevel { + install: Some(InstallConfiguration { + root_fs_type: Some(Filesystem::Ext4), + ..Default::default() + }), + }; + install.merge(other.install.unwrap(), &env); + assert_eq!( + install.root_fs_type.as_ref().copied().unwrap(), + Filesystem::Ext4 + ); + // This one shouldn't have been set + assert!(install.filesystem_root().is_none()); + install.canonicalize(); + assert_eq!(install.root_fs_type.as_ref().unwrap(), &Filesystem::Ext4); + assert_eq!( + install.filesystem_root().unwrap().fstype.unwrap(), + Filesystem::Ext4 + ); - let c: InstallConfigurationToplevel = toml::from_str( - r##"[install] + let c: InstallConfigurationToplevel = toml::from_str( + r##"[install] root-fs-type = "ext4" kargs = ["console=ttyS0", "foo=bar"] "##, - ) - .unwrap(); - let mut install = c.install.unwrap(); - assert_eq!(install.root_fs_type.unwrap(), Filesystem::Ext4); - let other = InstallConfigurationToplevel { - install: Some(InstallConfiguration { - kargs: Some( - ["console=tty0", "nosmt"] + ) + .unwrap(); + let mut install = c.install.unwrap(); + assert_eq!(install.root_fs_type.unwrap(), Filesystem::Ext4); + let other = InstallConfigurationToplevel { + install: Some(InstallConfiguration { + kargs: Some( + ["console=tty0", "nosmt"] + .into_iter() + .map(ToOwned::to_owned) + .collect(), + ), + ..Default::default() + }), + }; + install.merge(other.install.unwrap(), &env); + assert_eq!(install.root_fs_type.unwrap(), Filesystem::Ext4); + assert_eq!( + install.kargs, + Some( + ["console=ttyS0", "foo=bar", "console=tty0", "nosmt"] .into_iter() .map(ToOwned::to_owned) - .collect(), - ), - ..Default::default() - }), - }; - install.merge(other.install.unwrap(), &env); - assert_eq!(install.root_fs_type.unwrap(), Filesystem::Ext4); - assert_eq!( - install.kargs, - Some( - ["console=ttyS0", "foo=bar", "console=tty0", "nosmt"] - .into_iter() - .map(ToOwned::to_owned) - .collect() + .collect() + ) ) - ) -} + } -#[test] -fn test_parse_filesystems() { - use super::baseline::Filesystem; - let env = EnvProperties { - sys_arch: "x86_64".to_string(), - }; - let c: InstallConfigurationToplevel = toml::from_str( - r##"[install.filesystem.root] + #[test] + fn test_parse_filesystems() { + let env = EnvProperties { + sys_arch: "x86_64".to_string(), + }; + let c: InstallConfigurationToplevel = toml::from_str( + r##"[install.filesystem.root] type = "xfs" "##, - ) - .unwrap(); - let mut install = c.install.unwrap(); - assert_eq!( - install.filesystem_root().unwrap().fstype.unwrap(), - Filesystem::Xfs - ); - let other = InstallConfigurationToplevel { - install: Some(InstallConfiguration { - filesystem: Some(BasicFilesystems { - root: Some(RootFS { - fstype: Some(Filesystem::Ext4), + ) + .unwrap(); + let mut install = c.install.unwrap(); + assert_eq!( + install.filesystem_root().unwrap().fstype.unwrap(), + Filesystem::Xfs + ); + let other = InstallConfigurationToplevel { + install: Some(InstallConfiguration { + filesystem: Some(BasicFilesystems { + root: Some(RootFS { + fstype: Some(Filesystem::Ext4), + }), }), + ..Default::default() }), - ..Default::default() - }), - }; - install.merge(other.install.unwrap(), &env); - assert_eq!( - install.filesystem_root().unwrap().fstype.unwrap(), - Filesystem::Ext4 - ); -} + }; + install.merge(other.install.unwrap(), &env); + assert_eq!( + install.filesystem_root().unwrap().fstype.unwrap(), + Filesystem::Ext4 + ); + } -#[test] -fn test_parse_block() { - let env = EnvProperties { - sys_arch: "x86_64".to_string(), - }; - let c: InstallConfigurationToplevel = toml::from_str( - r##"[install.filesystem.root] + #[test] + fn test_parse_block() { + let env = EnvProperties { + sys_arch: "x86_64".to_string(), + }; + let c: InstallConfigurationToplevel = toml::from_str( + r##"[install.filesystem.root] type = "xfs" "##, - ) - .unwrap(); - let mut install = c.install.unwrap(); - // Verify the default (but note canonicalization mutates) - { - let mut install = install.clone(); - install.canonicalize(); - assert_eq!(install.get_block_setup(None).unwrap(), BlockSetup::Direct); - } - let other = InstallConfigurationToplevel { - install: Some(InstallConfiguration { - block: Some(vec![]), - ..Default::default() - }), - }; - install.merge(other.install.unwrap(), &env); - // Should be set, but zero length - assert_eq!(install.block.as_ref().unwrap().len(), 0); - assert!(install.get_block_setup(None).is_err()); + ) + .unwrap(); + let mut install = c.install.unwrap(); + // Verify the default (but note canonicalization mutates) + { + let mut install = install.clone(); + install.canonicalize(); + assert_eq!(install.get_block_setup(None).unwrap(), BlockSetup::Direct); + } + let other = InstallConfigurationToplevel { + install: Some(InstallConfiguration { + block: Some(vec![]), + ..Default::default() + }), + }; + install.merge(other.install.unwrap(), &env); + // Should be set, but zero length + assert_eq!(install.block.as_ref().unwrap().len(), 0); + assert!(install.get_block_setup(None).is_err()); - let c: InstallConfigurationToplevel = toml::from_str( - r##"[install] + let c: InstallConfigurationToplevel = toml::from_str( + r##"[install] block = ["tpm2-luks"]"##, - ) - .unwrap(); - let mut install = c.install.unwrap(); - install.canonicalize(); - assert_eq!(install.block.as_ref().unwrap().len(), 1); - assert_eq!(install.get_block_setup(None).unwrap(), BlockSetup::Tpm2Luks); + ) + .unwrap(); + let mut install = c.install.unwrap(); + install.canonicalize(); + assert_eq!(install.block.as_ref().unwrap().len(), 1); + assert_eq!(install.get_block_setup(None).unwrap(), BlockSetup::Tpm2Luks); - // And verify passing a disallowed config is an error - assert!(install.get_block_setup(Some(BlockSetup::Direct)).is_err()); -} + // And verify passing a disallowed config is an error + assert!(install.get_block_setup(Some(BlockSetup::Direct)).is_err()); + } -#[test] -/// Verify that kargs are only applied to supported architectures -fn test_arch() { - // no arch specified, kargs ensure that kargs are applied unconditionally - let env = EnvProperties { - sys_arch: "x86_64".to_string(), - }; - let c: InstallConfigurationToplevel = toml::from_str( - r##"[install] + #[test] + /// Verify that kargs are only applied to supported architectures + fn test_arch() { + // no arch specified, kargs ensure that kargs are applied unconditionally + let env = EnvProperties { + sys_arch: "x86_64".to_string(), + }; + let c: InstallConfigurationToplevel = toml::from_str( + r##"[install] root-fs-type = "xfs" "##, - ) - .unwrap(); - let mut install = c.install.unwrap(); - let other = InstallConfigurationToplevel { - install: Some(InstallConfiguration { - kargs: Some( + ) + .unwrap(); + let mut install = c.install.unwrap(); + let other = InstallConfigurationToplevel { + install: Some(InstallConfiguration { + kargs: Some( + ["console=tty0", "nosmt"] + .into_iter() + .map(ToOwned::to_owned) + .collect(), + ), + ..Default::default() + }), + }; + install.merge(other.install.unwrap(), &env); + assert_eq!( + install.kargs, + Some( ["console=tty0", "nosmt"] .into_iter() .map(ToOwned::to_owned) - .collect(), - ), - ..Default::default() - }), - }; - install.merge(other.install.unwrap(), &env); - assert_eq!( - install.kargs, - Some( - ["console=tty0", "nosmt"] - .into_iter() - .map(ToOwned::to_owned) - .collect() - ) - ); - let env = EnvProperties { - sys_arch: "aarch64".to_string(), - }; - let c: InstallConfigurationToplevel = toml::from_str( - r##"[install] + .collect() + ) + ); + let env = EnvProperties { + sys_arch: "aarch64".to_string(), + }; + let c: InstallConfigurationToplevel = toml::from_str( + r##"[install] root-fs-type = "xfs" "##, - ) - .unwrap(); - let mut install = c.install.unwrap(); - let other = InstallConfigurationToplevel { - install: Some(InstallConfiguration { - kargs: Some( + ) + .unwrap(); + let mut install = c.install.unwrap(); + let other = InstallConfigurationToplevel { + install: Some(InstallConfiguration { + kargs: Some( + ["console=tty0", "nosmt"] + .into_iter() + .map(ToOwned::to_owned) + .collect(), + ), + ..Default::default() + }), + }; + install.merge(other.install.unwrap(), &env); + assert_eq!( + install.kargs, + Some( ["console=tty0", "nosmt"] .into_iter() .map(ToOwned::to_owned) - .collect(), - ), - ..Default::default() - }), - }; - install.merge(other.install.unwrap(), &env); - assert_eq!( - install.kargs, - Some( - ["console=tty0", "nosmt"] - .into_iter() - .map(ToOwned::to_owned) - .collect() - ) - ); + .collect() + ) + ); - // one arch matches and one doesn't, ensure that kargs are only applied for the matching arch - let env = EnvProperties { - sys_arch: "aarch64".to_string(), - }; - let c: InstallConfigurationToplevel = toml::from_str( - r##"[install] + // one arch matches and one doesn't, ensure that kargs are only applied for the matching arch + let env = EnvProperties { + sys_arch: "aarch64".to_string(), + }; + let c: InstallConfigurationToplevel = toml::from_str( + r##"[install] root-fs-type = "xfs" "##, - ) - .unwrap(); - let mut install = c.install.unwrap(); - let other = InstallConfigurationToplevel { - install: Some(InstallConfiguration { - kargs: Some( - ["console=ttyS0", "foo=bar"] - .into_iter() - .map(ToOwned::to_owned) - .collect(), - ), - match_architectures: Some(["x86_64"].into_iter().map(ToOwned::to_owned).collect()), - ..Default::default() - }), - }; - install.merge(other.install.unwrap(), &env); - assert_eq!(install.kargs, None); - let other = InstallConfigurationToplevel { - install: Some(InstallConfiguration { - kargs: Some( + ) + .unwrap(); + let mut install = c.install.unwrap(); + let other = InstallConfigurationToplevel { + install: Some(InstallConfiguration { + kargs: Some( + ["console=ttyS0", "foo=bar"] + .into_iter() + .map(ToOwned::to_owned) + .collect(), + ), + match_architectures: Some(["x86_64"].into_iter().map(ToOwned::to_owned).collect()), + ..Default::default() + }), + }; + install.merge(other.install.unwrap(), &env); + assert_eq!(install.kargs, None); + let other = InstallConfigurationToplevel { + install: Some(InstallConfiguration { + kargs: Some( + ["console=tty0", "nosmt"] + .into_iter() + .map(ToOwned::to_owned) + .collect(), + ), + match_architectures: Some(["aarch64"].into_iter().map(ToOwned::to_owned).collect()), + ..Default::default() + }), + }; + install.merge(other.install.unwrap(), &env); + assert_eq!( + install.kargs, + Some( ["console=tty0", "nosmt"] .into_iter() .map(ToOwned::to_owned) - .collect(), - ), - match_architectures: Some(["aarch64"].into_iter().map(ToOwned::to_owned).collect()), - ..Default::default() - }), - }; - install.merge(other.install.unwrap(), &env); - assert_eq!( - install.kargs, - Some( - ["console=tty0", "nosmt"] - .into_iter() - .map(ToOwned::to_owned) - .collect() - ) - ); + .collect() + ) + ); - // multiple arch specified, ensure that kargs are applied to both archs - let env = EnvProperties { - sys_arch: "x86_64".to_string(), - }; - let c: InstallConfigurationToplevel = toml::from_str( - r##"[install] + // multiple arch specified, ensure that kargs are applied to both archs + let env = EnvProperties { + sys_arch: "x86_64".to_string(), + }; + let c: InstallConfigurationToplevel = toml::from_str( + r##"[install] root-fs-type = "xfs" "##, - ) - .unwrap(); - let mut install = c.install.unwrap(); - let other = InstallConfigurationToplevel { - install: Some(InstallConfiguration { - kargs: Some( + ) + .unwrap(); + let mut install = c.install.unwrap(); + let other = InstallConfigurationToplevel { + install: Some(InstallConfiguration { + kargs: Some( + ["console=tty0", "nosmt"] + .into_iter() + .map(ToOwned::to_owned) + .collect(), + ), + match_architectures: Some( + ["x86_64", "aarch64"] + .into_iter() + .map(ToOwned::to_owned) + .collect(), + ), + ..Default::default() + }), + }; + install.merge(other.install.unwrap(), &env); + assert_eq!( + install.kargs, + Some( ["console=tty0", "nosmt"] .into_iter() .map(ToOwned::to_owned) - .collect(), - ), - match_architectures: Some( - ["x86_64", "aarch64"] - .into_iter() - .map(ToOwned::to_owned) - .collect(), - ), - ..Default::default() - }), - }; - install.merge(other.install.unwrap(), &env); - assert_eq!( - install.kargs, - Some( - ["console=tty0", "nosmt"] - .into_iter() - .map(ToOwned::to_owned) - .collect() - ) - ); - let env = EnvProperties { - sys_arch: "aarch64".to_string(), - }; - let c: InstallConfigurationToplevel = toml::from_str( - r##"[install] + .collect() + ) + ); + let env = EnvProperties { + sys_arch: "aarch64".to_string(), + }; + let c: InstallConfigurationToplevel = toml::from_str( + r##"[install] root-fs-type = "xfs" "##, - ) - .unwrap(); - let mut install = c.install.unwrap(); - let other = InstallConfigurationToplevel { - install: Some(InstallConfiguration { - kargs: Some( + ) + .unwrap(); + let mut install = c.install.unwrap(); + let other = InstallConfigurationToplevel { + install: Some(InstallConfiguration { + kargs: Some( + ["console=tty0", "nosmt"] + .into_iter() + .map(ToOwned::to_owned) + .collect(), + ), + match_architectures: Some( + ["x86_64", "aarch64"] + .into_iter() + .map(ToOwned::to_owned) + .collect(), + ), + ..Default::default() + }), + }; + install.merge(other.install.unwrap(), &env); + assert_eq!( + install.kargs, + Some( ["console=tty0", "nosmt"] .into_iter() .map(ToOwned::to_owned) - .collect(), - ), - match_architectures: Some( - ["x86_64", "aarch64"] - .into_iter() - .map(ToOwned::to_owned) - .collect(), - ), - ..Default::default() - }), - }; - install.merge(other.install.unwrap(), &env); - assert_eq!( - install.kargs, - Some( - ["console=tty0", "nosmt"] - .into_iter() - .map(ToOwned::to_owned) - .collect() - ) - ); + .collect() + ) + ); + } } diff --git a/lib/src/kernel.rs b/lib/src/kernel.rs index 7f6aeff9d..72c34a07c 100644 --- a/lib/src/kernel.rs +++ b/lib/src/kernel.rs @@ -37,10 +37,15 @@ pub(crate) fn find_first_cmdline_arg<'a>( .next() } -#[test] -fn test_find_first() { - let kargs = &["foo=bar", "root=/dev/vda", "blah", "root=/dev/other"]; - let kargs = || kargs.iter().copied(); - assert_eq!(find_first_cmdline_arg(kargs(), "root"), Some("/dev/vda")); - assert_eq!(find_first_cmdline_arg(kargs(), "nonexistent"), None); +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_find_first() { + let kargs = &["foo=bar", "root=/dev/vda", "blah", "root=/dev/other"]; + let kargs = || kargs.iter().copied(); + assert_eq!(find_first_cmdline_arg(kargs(), "root"), Some("/dev/vda")); + assert_eq!(find_first_cmdline_arg(kargs(), "nonexistent"), None); + } } diff --git a/lib/src/lints.rs b/lib/src/lints.rs index 935d71d72..6fbea7adc 100644 --- a/lib/src/lints.rs +++ b/lib/src/lints.rs @@ -117,143 +117,147 @@ fn check_utf8(dir: &Dir) -> Result<()> { } #[cfg(test)] -fn fixture() -> Result { - let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; - Ok(tempdir) -} +mod tests { + use super::*; -#[test] -fn test_open_noxdev() -> Result<()> { - let root = Dir::open_ambient_dir("/", cap_std::ambient_authority())?; - // This hard requires the host setup to have /usr/bin on the same filesystem as / - let usr = Dir::open_ambient_dir("/usr", cap_std::ambient_authority())?; - assert!(open_dir_noxdev(&usr, "bin").unwrap().is_some()); - // Requires a mounted /proc, but that also seems ane. - assert!(open_dir_noxdev(&root, "proc").unwrap().is_none()); - Ok(()) -} + fn fixture() -> Result { + let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; + Ok(tempdir) + } -#[test] -fn test_var_run() -> Result<()> { - let root = &fixture()?; - // This one should pass - check_var_run(root).unwrap(); - root.create_dir_all("var/run/foo")?; - assert!(check_var_run(root).is_err()); - root.remove_dir_all("var/run")?; - // Now we should pass again - check_var_run(root).unwrap(); - Ok(()) -} + #[test] + fn test_open_noxdev() -> Result<()> { + let root = Dir::open_ambient_dir("/", cap_std::ambient_authority())?; + // This hard requires the host setup to have /usr/bin on the same filesystem as / + let usr = Dir::open_ambient_dir("/usr", cap_std::ambient_authority())?; + assert!(open_dir_noxdev(&usr, "bin").unwrap().is_some()); + // Requires a mounted /proc, but that also seems ane. + assert!(open_dir_noxdev(&root, "proc").unwrap().is_none()); + Ok(()) + } -#[test] -fn test_kernel_lint() -> Result<()> { - let root = &fixture()?; - // This one should pass - check_kernel(root).unwrap(); - root.create_dir_all("usr/lib/modules/5.7.2")?; - root.write("usr/lib/modules/5.7.2/vmlinuz", "old vmlinuz")?; - root.create_dir_all("usr/lib/modules/6.3.1")?; - root.write("usr/lib/modules/6.3.1/vmlinuz", "new vmlinuz")?; - assert!(check_kernel(root).is_err()); - root.remove_dir_all("usr/lib/modules/5.7.2")?; - // Now we should pass again - check_kernel(root).unwrap(); - Ok(()) -} + #[test] + fn test_var_run() -> Result<()> { + let root = &fixture()?; + // This one should pass + check_var_run(root).unwrap(); + root.create_dir_all("var/run/foo")?; + assert!(check_var_run(root).is_err()); + root.remove_dir_all("var/run")?; + // Now we should pass again + check_var_run(root).unwrap(); + Ok(()) + } -#[test] -fn test_kargs() -> Result<()> { - let root = &fixture()?; - check_parse_kargs(root).unwrap(); - root.create_dir_all("usr/lib/bootc")?; - root.write("usr/lib/bootc/kargs.d", "not a directory")?; - assert!(check_parse_kargs(root).is_err()); - Ok(()) -} + #[test] + fn test_kernel_lint() -> Result<()> { + let root = &fixture()?; + // This one should pass + check_kernel(root).unwrap(); + root.create_dir_all("usr/lib/modules/5.7.2")?; + root.write("usr/lib/modules/5.7.2/vmlinuz", "old vmlinuz")?; + root.create_dir_all("usr/lib/modules/6.3.1")?; + root.write("usr/lib/modules/6.3.1/vmlinuz", "new vmlinuz")?; + assert!(check_kernel(root).is_err()); + root.remove_dir_all("usr/lib/modules/5.7.2")?; + // Now we should pass again + check_kernel(root).unwrap(); + Ok(()) + } -#[test] -fn test_usr_etc() -> Result<()> { - let root = &fixture()?; - // This one should pass - check_usretc(root).unwrap(); - root.create_dir_all("etc")?; - root.create_dir_all("usr/etc")?; - assert!(check_usretc(root).is_err()); - root.remove_dir_all("etc")?; - // Now we should pass again - check_usretc(root).unwrap(); - Ok(()) -} + #[test] + fn test_kargs() -> Result<()> { + let root = &fixture()?; + check_parse_kargs(root).unwrap(); + root.create_dir_all("usr/lib/bootc")?; + root.write("usr/lib/bootc/kargs.d", "not a directory")?; + assert!(check_parse_kargs(root).is_err()); + Ok(()) + } -#[test] -fn test_non_utf8() { - use std::{ffi::OsStr, os::unix::ffi::OsStrExt}; + #[test] + fn test_usr_etc() -> Result<()> { + let root = &fixture()?; + // This one should pass + check_usretc(root).unwrap(); + root.create_dir_all("etc")?; + root.create_dir_all("usr/etc")?; + assert!(check_usretc(root).is_err()); + root.remove_dir_all("etc")?; + // Now we should pass again + check_usretc(root).unwrap(); + Ok(()) + } - let root = &fixture().unwrap(); + #[test] + fn test_non_utf8() { + use std::{ffi::OsStr, os::unix::ffi::OsStrExt}; - // Try to create some adversarial symlink situations to ensure the walk doesn't crash - root.create_dir("subdir").unwrap(); - // Self-referential symlinks - root.symlink("self", "self").unwrap(); - // Infinitely looping dir symlinks - root.symlink("..", "subdir/parent").unwrap(); - // Broken symlinks - root.symlink("does-not-exist", "broken").unwrap(); - // Out-of-scope symlinks - root.symlink("../../x", "escape").unwrap(); - // Should be fine - check_utf8(root).unwrap(); + let root = &fixture().unwrap(); - // But this will cause an issue - let baddir = OsStr::from_bytes(b"subdir/2/bad\xffdir"); - root.create_dir("subdir/2").unwrap(); - root.create_dir(baddir).unwrap(); - let Err(err) = check_utf8(root) else { - unreachable!("Didn't fail"); - }; - assert_eq!( - err.to_string(), - r#"/subdir/2/: Found non-utf8 filename "bad\xFFdir""# - ); - root.remove_dir(baddir).unwrap(); // Get rid of the problem - check_utf8(root).unwrap(); // Check it + // Try to create some adversarial symlink situations to ensure the walk doesn't crash + root.create_dir("subdir").unwrap(); + // Self-referential symlinks + root.symlink("self", "self").unwrap(); + // Infinitely looping dir symlinks + root.symlink("..", "subdir/parent").unwrap(); + // Broken symlinks + root.symlink("does-not-exist", "broken").unwrap(); + // Out-of-scope symlinks + root.symlink("../../x", "escape").unwrap(); + // Should be fine + check_utf8(root).unwrap(); - // Create a new problem in the form of a regular file - let badfile = OsStr::from_bytes(b"regular\xff"); - root.write(badfile, b"Hello, world!\n").unwrap(); - let Err(err) = check_utf8(root) else { - unreachable!("Didn't fail"); - }; - assert_eq!( - err.to_string(), - r#"/: Found non-utf8 filename "regular\xFF""# - ); - root.remove_file(badfile).unwrap(); // Get rid of the problem - check_utf8(root).unwrap(); // Check it + // But this will cause an issue + let baddir = OsStr::from_bytes(b"subdir/2/bad\xffdir"); + root.create_dir("subdir/2").unwrap(); + root.create_dir(baddir).unwrap(); + let Err(err) = check_utf8(root) else { + unreachable!("Didn't fail"); + }; + assert_eq!( + err.to_string(), + r#"/subdir/2/: Found non-utf8 filename "bad\xFFdir""# + ); + root.remove_dir(baddir).unwrap(); // Get rid of the problem + check_utf8(root).unwrap(); // Check it - // And now test invalid symlink targets - root.symlink(badfile, "subdir/good-name").unwrap(); - let Err(err) = check_utf8(root) else { - unreachable!("Didn't fail"); - }; - assert_eq!( - err.to_string(), - r#"/subdir/good-name: Found non-utf8 symlink target"# - ); - root.remove_file("subdir/good-name").unwrap(); // Get rid of the problem - check_utf8(root).unwrap(); // Check it + // Create a new problem in the form of a regular file + let badfile = OsStr::from_bytes(b"regular\xff"); + root.write(badfile, b"Hello, world!\n").unwrap(); + let Err(err) = check_utf8(root) else { + unreachable!("Didn't fail"); + }; + assert_eq!( + err.to_string(), + r#"/: Found non-utf8 filename "regular\xFF""# + ); + root.remove_file(badfile).unwrap(); // Get rid of the problem + check_utf8(root).unwrap(); // Check it - // Finally, test a self-referential symlink with an invalid name. - // We should spot the invalid name before we check the target. - root.symlink(badfile, badfile).unwrap(); - let Err(err) = check_utf8(root) else { - unreachable!("Didn't fail"); - }; - assert_eq!( - err.to_string(), - r#"/: Found non-utf8 filename "regular\xFF""# - ); - root.remove_file(badfile).unwrap(); // Get rid of the problem - check_utf8(root).unwrap(); // Check it + // And now test invalid symlink targets + root.symlink(badfile, "subdir/good-name").unwrap(); + let Err(err) = check_utf8(root) else { + unreachable!("Didn't fail"); + }; + assert_eq!( + err.to_string(), + r#"/subdir/good-name: Found non-utf8 symlink target"# + ); + root.remove_file("subdir/good-name").unwrap(); // Get rid of the problem + check_utf8(root).unwrap(); // Check it + + // Finally, test a self-referential symlink with an invalid name. + // We should spot the invalid name before we check the target. + root.symlink(badfile, badfile).unwrap(); + let Err(err) = check_utf8(root) else { + unreachable!("Didn't fail"); + }; + assert_eq!( + err.to_string(), + r#"/: Found non-utf8 filename "regular\xFF""# + ); + root.remove_file(badfile).unwrap(); // Get rid of the problem + check_utf8(root).unwrap(); // Check it + } } diff --git a/lib/src/utils.rs b/lib/src/utils.rs index d5d1006a3..8e33ceef3 100644 --- a/lib/src/utils.rs +++ b/lib/src/utils.rs @@ -229,47 +229,52 @@ pub(crate) fn digested_pullspec(image: &str, digest: &str) -> String { format!("{image}@{digest}") } -#[test] -fn test_digested_pullspec() { - let digest = "ebe3bdccc041864e5a485f1e755e242535c3b83d110c0357fe57f110b73b143e"; - assert_eq!( - digested_pullspec("quay.io/example/foo:bar", digest), - format!("quay.io/example/foo:bar@{digest}") - ); - assert_eq!( - digested_pullspec("quay.io/example/foo@sha256:otherdigest", digest), - format!("quay.io/example/foo@{digest}") - ); - assert_eq!( - digested_pullspec("quay.io/example/foo", digest), - format!("quay.io/example/foo@{digest}") - ); -} +#[cfg(test)] +mod tests { + use super::*; -#[test] -fn test_find_mount_option() { - const V1: &str = "rw,relatime,compress=foo,subvol=blah,fast"; - assert_eq!(find_mount_option(V1, "subvol").unwrap(), "blah"); - assert_eq!(find_mount_option(V1, "rw"), None); - assert_eq!(find_mount_option(V1, "somethingelse"), None); -} + #[test] + fn test_digested_pullspec() { + let digest = "ebe3bdccc041864e5a485f1e755e242535c3b83d110c0357fe57f110b73b143e"; + assert_eq!( + digested_pullspec("quay.io/example/foo:bar", digest), + format!("quay.io/example/foo:bar@{digest}") + ); + assert_eq!( + digested_pullspec("quay.io/example/foo@sha256:otherdigest", digest), + format!("quay.io/example/foo@{digest}") + ); + assert_eq!( + digested_pullspec("quay.io/example/foo", digest), + format!("quay.io/example/foo@{digest}") + ); + } -#[test] -fn test_sigpolicy_from_opts() { - assert_eq!( - sigpolicy_from_opts(false, None), - SignatureSource::ContainerPolicy - ); - assert_eq!( - sigpolicy_from_opts(true, None), - SignatureSource::ContainerPolicyAllowInsecure - ); - assert_eq!( - sigpolicy_from_opts(false, Some("foo")), - SignatureSource::OstreeRemote("foo".to_owned()) - ); - assert_eq!( - sigpolicy_from_opts(true, Some("foo")), - SignatureSource::ContainerPolicyAllowInsecure - ); + #[test] + fn test_find_mount_option() { + const V1: &str = "rw,relatime,compress=foo,subvol=blah,fast"; + assert_eq!(find_mount_option(V1, "subvol").unwrap(), "blah"); + assert_eq!(find_mount_option(V1, "rw"), None); + assert_eq!(find_mount_option(V1, "somethingelse"), None); + } + + #[test] + fn test_sigpolicy_from_opts() { + assert_eq!( + sigpolicy_from_opts(false, None), + SignatureSource::ContainerPolicy + ); + assert_eq!( + sigpolicy_from_opts(true, None), + SignatureSource::ContainerPolicyAllowInsecure + ); + assert_eq!( + sigpolicy_from_opts(false, Some("foo")), + SignatureSource::OstreeRemote("foo".to_owned()) + ); + assert_eq!( + sigpolicy_from_opts(true, Some("foo")), + SignatureSource::ContainerPolicyAllowInsecure + ); + } }