Merge pull request #2528 from cgwalters/test-crate-update

tests/inst/Cargo.toml

@@ -14,10 +14,10 @@ structopt = "0.3"
 serde = "1.0.111"
 serde_derive = "1.0.111"
 serde_json = "1.0"
-sh-inline = "0.1.0"
+sh-inline = "0.2.0"
 anyhow = "1.0"
 tempfile = "3.1.0"
-ostree-ext = { version = "0.3.0" }
+ostree-ext = { version = "0.6.0" }
 libtest-mimic = "0.3.0"
 twoway = "0.2.1"
 hyper = { version = "0.14", features = ["runtime", "http1", "http2", "tcp", "server"] }
@@ -28,18 +28,14 @@ tokio = { version = "1.4.0", features = ["full"] }
 futures-util = "0.3.1"
 base64 = "0.12.0"
 procspawn = "0.8"
-rand = "0.7.3"
+rand = "0.8"
-linkme = "0.2"
 strum = "0.18.0"
 strum_macros = "0.18.0"
 openat = "0.1.19"
 openat-ext = "0.1.4"
-nix = "0.20.0"
+nix = "0.23.0"
 # See discussion in https://github.com/coreos/rpm-ostree/pull/2569#issuecomment-780569188
 rpmostree-client = { git = "https://github.com/coreos/rpm-ostree", tag = "v2021.3" }
 
 # This one I might publish to crates.io, not sure yet
 with-procspawn-tempdir = { git = "https://github.com/cgwalters/with-procspawn-tempdir" }
-
-# Internal crate for the test macro
-itest-macro = { path = "itest-macro" }

tests/inst/itest-macro/Cargo.toml

@@ -1,14 +0,0 @@
-[package]
-name = "itest-macro"
-version = "0.1.0"
-edition = "2018"
-
-[lib]
-proc-macro = true
-path = "src/itest-macro.rs"
-
-[dependencies]
-quote = "1.0.3"
-proc-macro2 = "1.0.10"
-syn = { version = "1.0.3", features = ["full"] }
-anyhow = "1.0"

tests/inst/itest-macro/src/itest-macro.rs

@@ -1,71 +0,0 @@
-extern crate proc_macro;
-
-use proc_macro::TokenStream;
-use proc_macro2::Span;
-use quote::quote;
-
-/// Wraps function using `procspawn` to allocate a new temporary directory,
-/// make it the process' working directory, and run the function.
-#[proc_macro_attribute]
-pub fn itest(attrs: TokenStream, input: TokenStream) -> TokenStream {
-    let attrs = syn::parse_macro_input!(attrs as syn::AttributeArgs);
-    if attrs.len() > 1 {
-        return syn::Error::new_spanned(&attrs[1], "itest takes 0 or 1 attributes")
-            .to_compile_error()
-            .into();
-    }
-    let destructive = match attrs.get(0) {
-        Some(syn::NestedMeta::Meta(syn::Meta::NameValue(namevalue))) => {
-            if let Some(name) = namevalue.path.get_ident().map(|i| i.to_string()) {
-                if name == "destructive" {
-                    match &namevalue.lit {
-                        syn::Lit::Bool(v) => v.value,
-                        _ => {
-                            return syn::Error::new_spanned(
-                                &attrs[1],
-                                format!("destructive must be bool {}", name),
-                            )
-                            .to_compile_error()
-                            .into();
-                        }
-                    }
-                } else {
-                    return syn::Error::new_spanned(
-                        &attrs[1],
-                        format!("Unknown argument {}", name),
-                    )
-                    .to_compile_error()
-                    .into();
-                }
-            } else {
-                false
-            }
-        }
-        Some(v) => {
-            return syn::Error::new_spanned(&v, "Unexpected argument")
-                .to_compile_error()
-                .into()
-        }
-        None => false,
-    };
-    let func = syn::parse_macro_input!(input as syn::ItemFn);
-    let fident = func.sig.ident.clone();
-    let varident = quote::format_ident!("ITEST_{}", fident);
-    let fidentstrbuf = format!(r#"{}"#, fident);
-    let fidentstr = syn::LitStr::new(&fidentstrbuf, Span::call_site());
-    let testident = if destructive {
-        quote::format_ident!("{}", "DESTRUCTIVE_TESTS")
-    } else {
-        quote::format_ident!("{}", "NONDESTRUCTIVE_TESTS")
-    };
-    let output = quote! {
-        #[linkme::distributed_slice(#testident)]
-        #[allow(non_upper_case_globals)]
-        static #varident : Test = Test {
-            name: #fidentstr,
-            f: #fident,
-        };
-        #func
-    };
-    output.into()
-}

tests/inst/src/destructive.rs

@@ -152,7 +152,7 @@ fn generate_update(commit: &str) -> Result<()> {
     // but traversing all the objects is expensive.  So here we only prune 1/5 of the time.
     if rand::thread_rng().gen_ratio(1, 5) {
         bash!(
-            "ostree --repo={srvrepo} prune --refs-only --depth=1",
+            "ostree --repo=${srvrepo} prune --refs-only --depth=1",
             srvrepo = SRVREPO
         )?;
     }
@@ -166,10 +166,10 @@ fn generate_update(commit: &str) -> Result<()> {
 fn generate_srv_repo(commit: &str) -> Result<()> {
     bash!(
         r#"
-        ostree --repo={srvrepo} init --mode=archive
+        ostree --repo=${srvrepo} init --mode=archive
-        ostree --repo={srvrepo} config set archive.zlib-level 1
+        ostree --repo=${srvrepo} config set archive.zlib-level 1
-        ostree --repo={srvrepo} pull-local /sysroot/ostree/repo {commit}
+        ostree --repo=${srvrepo} pull-local /sysroot/ostree/repo ${commit}
-        ostree --repo={srvrepo} refs --create={testref} {commit}
+        ostree --repo=${srvrepo} refs --create=${testref} ${commit}
         "#,
         srvrepo = SRVREPO,
         commit = commit,
@@ -310,7 +310,7 @@ fn parse_and_validate_reboot_mark<M: AsRef<str>>(
 fn validate_pending_commit(pending_commit: &str, commitstates: &CommitStates) -> Result<()> {
     if pending_commit != commitstates.target {
         bash!("rpm-ostree status -v")?;
-        bash!("ostree show {pending_commit}", pending_commit)?;
+        bash!("ostree show ${pending_commit}", pending_commit)?;
         anyhow::bail!(
             "Expected target commit={} but pending={} ({:?})",
             commitstates.target,
@@ -450,7 +450,7 @@ fn impl_transaction_test<M: AsRef<str>>(
             let ms = std::cmp::min(cycle_time_ms.saturating_mul(20), 24 * 60 * 60 * 1000);
             time::Duration::from_millis(ms)
         } else {
-            time::Duration::from_millis(rng.gen_range(0, cycle_time_ms))
+            time::Duration::from_millis(rng.gen_range(0..cycle_time_ms))
         };
         println!(
             "force-reboot-time={:?} cycle={:?} status:{:?}",
@@ -463,11 +463,11 @@ fn impl_transaction_test<M: AsRef<str>>(
             "
             systemctl stop rpm-ostreed
             systemctl stop ostree-finalize-staged
-            ostree reset testrepo:{testref} {booted_commit}
+            ostree reset testrepo:${testref} ${booted_commit}
             rpm-ostree cleanup -pbrm
             ",
             testref,
-            booted_commit,
+            booted_commit
         )
         .with_context(|| {
             format!(
@@ -537,8 +537,7 @@ fn impl_transaction_test<M: AsRef<str>>(
     }
 }
 
-#[itest(destructive = true)]
+pub(crate) fn itest_transactionality() -> Result<()> {
-fn transactionality() -> Result<()> {
     testinit()?;
     let mark = get_reboot_mark()?;
     let cancellable = Some(gio::Cancellable::new());
@@ -572,7 +571,7 @@ fn transactionality() -> Result<()> {
         let url = format!("http://{}", addr);
         bash!(
             "ostree remote delete --if-exists testrepo
-             ostree remote add --set=gpg-verify=false testrepo {url}",
+             ostree remote add --set=gpg-verify=false testrepo ${url}",
             url
         )?;
 
@@ -590,9 +589,9 @@ fn transactionality() -> Result<()> {
             let testref = TESTREF;
             bash!(
                 "
-                ostree admin set-origin testrepo {url} {testref}
+                ostree admin set-origin testrepo ${url} ${testref}
-                ostree refs --create testrepo:{testref} {commit}
+                ostree refs --create testrepo:${testref} ${commit}
-                ostree refs --create={origref} {commit}
+                ostree refs --create=${origref} ${commit}
                 ",
                 url,
                 origref,

tests/inst/src/insttestmain.rs

@@ -10,6 +10,25 @@ mod treegen;
 // Written by Ignition
 const DESTRUCTIVE_TEST_STAMP: &str = "/etc/ostree-destructive-test-ok";
 
+macro_rules! test {
+    ($f: path) => {
+        (stringify!($f), $f)
+    };
+}
+
+type StaticTest = (&'static str, fn() -> Result<()>);
+
+const TESTS: &[StaticTest] = &[
+    test!(sysroot::itest_sysroot_ro),
+    test!(sysroot::itest_immutable_bit),
+    test!(sysroot::itest_tmpfiles),
+    test!(repobin::itest_basic),
+    test!(repobin::itest_nofifo),
+    test!(repobin::itest_extensions),
+    test!(repobin::itest_pull_basicauth),
+];
+const DESTRUCTIVE_TESTS: &[StaticTest] = &[test!(destructive::itest_transactionality)];
+
 #[derive(Debug, StructOpt)]
 #[structopt(rename_all = "kebab-case")]
 #[allow(clippy::enum_variant_names)]
@@ -30,18 +49,18 @@ enum NonDestructiveOpts {
     Args(Vec<String>),
 }
 
-fn libtest_from_test(t: &'static test::Test) -> test::TestImpl {
+fn libtest_from_test(t: &StaticTest) -> test::TestImpl {
     libtest_mimic::Test {
-        name: t.name.into(),
+        name: t.0.into(),
         kind: "".into(),
         is_ignored: false,
         is_bench: false,
-        data: t,
+        data: t.1,
     }
 }
 
 fn run_test(test: &test::TestImpl) -> libtest_mimic::Outcome {
-    if let Err(e) = (test.data.f)() {
+    if let Err(e) = (test.data)() {
         libtest_mimic::Outcome::Failed {
             msg: Some(e.to_string()),
         }
@@ -72,8 +91,8 @@ fn main() -> Result<()> {
 
     match opt {
         Opt::ListDestructive => {
-            for t in test::DESTRUCTIVE_TESTS.iter() {
+            for t in DESTRUCTIVE_TESTS {
-                println!("{}", t.name);
+                println!("{}", t.0);
             }
             Ok(())
         }
@@ -81,10 +100,7 @@ fn main() -> Result<()> {
             // FIXME add method to parse subargs
             let NonDestructiveOpts::Args(iter) = subopt;
             let libtestargs = libtest_mimic::Arguments::from_iter(iter);
-            let tests: Vec<_> = test::NONDESTRUCTIVE_TESTS
+            let tests: Vec<_> = TESTS.iter().map(libtest_from_test).collect();
-                .iter()
-                .map(libtest_from_test)
-                .collect();
             libtest_mimic::run_tests(&libtestargs, tests, run_test).exit();
         }
         Opt::RunDestructive { name } => {
@@ -98,10 +114,10 @@ fn main() -> Result<()> {
                 bail!("An ostree-based host is required")
             }
 
-            for t in test::DESTRUCTIVE_TESTS.iter() {
+            for (tname, f) in DESTRUCTIVE_TESTS {
-                if t.name == name {
+                if *tname == name.as_str() {
-                    (t.f)()?;
+                    (f)()?;
-                    println!("ok destructive test: {}", t.name);
+                    println!("ok destructive test: {}", tname);
                     return Ok(());
                 }
             }

tests/inst/src/repobin.rs

@@ -8,15 +8,13 @@ use anyhow::{Context, Result};
 use sh_inline::{bash, bash_command};
 use with_procspawn_tempdir::with_procspawn_tempdir;
 
-#[itest]
+pub(crate) fn itest_basic() -> Result<()> {
-fn test_basic() -> Result<()> {
     bash!(r"ostree --help >/dev/null")?;
     Ok(())
 }
 
-#[itest]
 #[with_procspawn_tempdir]
-fn test_nofifo() -> Result<()> {
+pub(crate) fn itest_nofifo() -> Result<()> {
     assert!(std::path::Path::new(".procspawn-tmpdir").exists());
     bash!(
         r"ostree --repo=repo init --mode=archive
@@ -34,9 +32,8 @@ fn test_nofifo() -> Result<()> {
     Ok(())
 }
 
-#[itest]
 #[with_procspawn_tempdir]
-fn test_mtime() -> Result<()> {
+pub(crate) fn itest_mtime() -> Result<()> {
     bash!(
         r"ostree --repo=repo init --mode=archive
     mkdir tmproot
@@ -50,17 +47,15 @@ fn test_mtime() -> Result<()> {
     Ok(())
 }
 
-#[itest]
 #[with_procspawn_tempdir]
-fn test_extensions() -> Result<()> {
+pub(crate) fn itest_extensions() -> Result<()> {
     bash!(r"ostree --repo=repo init --mode=bare")?;
     assert!(Path::new("repo/extensions").exists());
     Ok(())
 }
 
-#[itest]
 #[with_procspawn_tempdir]
-fn test_pull_basicauth() -> Result<()> {
+pub(crate) fn itest_pull_basicauth() -> Result<()> {
     let opts = TestHttpServerOpts {
         basicauth: true,
         ..Default::default()
@@ -77,14 +72,14 @@ fn test_pull_basicauth() -> Result<()> {
         let osroot = Path::new("osroot");
         crate::treegen::mkroot(&osroot)?;
         bash!(
-            r#"ostree --repo={serverrepo} init --mode=archive
+            r#"ostree --repo=${serverrepo} init --mode=archive
-        ostree --repo={serverrepo} commit -b os --tree=dir={osroot} >/dev/null
+        ostree --repo=${serverrepo} commit -b os --tree=dir=${osroot} >/dev/null
         mkdir client
         cd client
         ostree --repo=repo init --mode=archive
-        ostree --repo=repo remote add --set=gpg-verify=false origin-unauth {baseuri}
+        ostree --repo=repo remote add --set=gpg-verify=false origin-unauth ${baseuri}
-        ostree --repo=repo remote add --set=gpg-verify=false origin-badauth {unauthuri}
+        ostree --repo=repo remote add --set=gpg-verify=false origin-badauth ${unauthuri}
-        ostree --repo=repo remote add --set=gpg-verify=false origin-goodauth {authuri}
+        ostree --repo=repo remote add --set=gpg-verify=false origin-goodauth ${authuri}
         "#,
             osroot = osroot,
             serverrepo = serverrepo,
@@ -95,7 +90,7 @@ fn test_pull_basicauth() -> Result<()> {
         for rem in &["unauth", "badauth"] {
             cmd_fails_with(
                 bash_command!(
-                    r#"ostree --repo=client/repo pull origin-{rem} os >/dev/null"#,
+                    r#"ostree --repo=client/repo pull origin-${rem} os >/dev/null"#,
                     rem = *rem
                 )
                 .unwrap(),

tests/inst/src/sysroot.rs

@@ -13,8 +13,7 @@ fn skip_non_ostree_host() -> bool {
     !std::path::Path::new("/run/ostree-booted").exists()
 }
 
-#[itest]
+pub(crate) fn itest_sysroot_ro() -> Result<()> {
-fn test_sysroot_ro() -> Result<()> {
     // TODO add a skipped identifier
     if skip_non_ostree_host() {
         return Ok(());
@@ -39,8 +38,7 @@ fn test_sysroot_ro() -> Result<()> {
     Ok(())
 }
 
-#[itest]
+pub(crate) fn itest_immutable_bit() -> Result<()> {
-fn test_immutable_bit() -> Result<()> {
     if skip_non_ostree_host() {
         return Ok(());
     }
@@ -49,8 +47,7 @@ fn test_immutable_bit() -> Result<()> {
     Ok(())
 }
 
-#[itest]
+pub(crate) fn itest_tmpfiles() -> Result<()> {
-fn test_tmpfiles() -> Result<()> {
     if skip_non_ostree_host() {
         return Ok(());
     }

tests/inst/src/test.rs

@@ -6,10 +6,8 @@ use std::process::Command;
 use std::time;
 
 use anyhow::{bail, Context, Result};
-use linkme::distributed_slice;
 use rand::Rng;
 
-pub use itest_macro::itest;
 pub use with_procspawn_tempdir::with_procspawn_tempdir;
 
 // HTTP Server deps
@@ -20,19 +18,7 @@ use hyper_staticfile::Static;
 use tokio::runtime::Runtime;
 
 pub(crate) type TestFn = fn() -> Result<()>;
-
+pub(crate) type TestImpl = libtest_mimic::Test<TestFn>;
-#[derive(Debug)]
-pub(crate) struct Test {
-    pub(crate) name: &'static str,
-    pub(crate) f: TestFn,
-}
-
-pub(crate) type TestImpl = libtest_mimic::Test<&'static Test>;
-
-#[distributed_slice]
-pub(crate) static NONDESTRUCTIVE_TESTS: [Test] = [..];
-#[distributed_slice]
-pub(crate) static DESTRUCTIVE_TESTS: [Test] = [..];
 
 /// Run command and assert that its stderr contains pat
 pub(crate) fn cmd_fails_with<C: BorrowMut<Command>>(mut c: C, pat: &str) -> Result<()> {
@@ -105,7 +91,7 @@ pub(crate) async fn http_server<P: AsRef<Path>>(
     ) -> Result<Response<Body>> {
         if let Some(random_delay) = opts.random_delay {
             let slices = 100u32;
-            let n: u32 = rand::thread_rng().gen_range(0, slices);
+            let n: u32 = rand::thread_rng().gen_range(0..slices);
             std::thread::sleep((random_delay / slices) * n);
         }
         if opts.basicauth {

tests/inst/src/treegen.rs

@@ -62,9 +62,9 @@ pub(crate) fn mutate_one_executable_to(
     let extra = rand::thread_rng()
         .sample_iter(&rand::distributions::Alphanumeric)
         .take(10)
-        .collect::<String>();
+        .collect::<Vec<u8>>();
     destf
-        .write_all(extra.as_bytes())
+        .write_all(&extra)
         .context("Failed to append extra data")?;
     Ok(())
 }
@@ -140,7 +140,7 @@ pub(crate) fn update_os_tree<P: AsRef<Path>>(
     }
     assert!(mutated > 0);
     println!("Mutated ELF files: {}", mutated);
-    bash!("ostree --repo={repo} commit --consume -b {ostref} --base={ostref} --tree=dir={tempdir} --owner-uid 0 --owner-gid 0 --selinux-policy-from-base --link-checkout-speedup --no-bindings --no-xattrs",
+    bash!("ostree --repo=${repo} commit --consume -b ${ostref} --base=${ostref} --tree=dir=${tempdir} --owner-uid 0 --owner-gid 0 --selinux-policy-from-base --link-checkout-speedup --no-bindings --no-xattrs",
         repo = repo_path,
         ostref = ostref,
         tempdir = tempdir.path()).context("Failed to commit updated content")?;