diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index ff030b2215..44168562e0 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -37,7 +37,7 @@ strum = "0.18.0" strum_macros = "0.18.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" } -xshell = "0.2.3" +xshell = "0.2.7" # This one I might publish to crates.io, not sure yet with-procspawn-tempdir = { git = "https://github.com/cgwalters/with-procspawn-tempdir" } diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index 5a89bd0d4e..6128eca34c 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -25,7 +25,6 @@ use ostree_ext::ostree; use rand::seq::SliceRandom; use rand::Rng; use serde::{Deserialize, Serialize}; -use sh_inline::bash; use std::collections::BTreeMap; use std::io::Write; use std::path::Path; @@ -132,12 +131,11 @@ impl InterruptStrategy { /// TODO add readonly sysroot handling into base ostree fn testinit() -> Result<()> { + let sh = xshell::Shell::new()?; assert!(std::path::Path::new("/run/ostree-booted").exists()); - bash!( - r"if ! test -w /sysroot; then - mount -o remount,rw /sysroot -fi" - )?; + if cmd!(sh, "test -w /sysroot").run().is_err() { + cmd!(sh, "mount -o remount,rw /sysroot").run()?; + } Ok(()) } @@ -146,16 +144,14 @@ fi" /// reworked the tree mutation to operate on an ostree repo /// rather than a filesystem. fn generate_update(commit: &str) -> Result<()> { + let sh = xshell::Shell::new()?; println!("Generating update from {}", commit); crate::treegen::update_os_tree(SRVREPO, TESTREF, TREEGEN_PERCENTAGE) .context("Failed to generate new content")?; // Amortize the prune across multiple runs; we don't want to leak space, // 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", - srvrepo = SRVREPO - )?; + cmd!(sh, "ostree --repo={SRVREPO} prune --refs-only --depth=1").run()?; } Ok(()) } @@ -165,17 +161,24 @@ fn generate_update(commit: &str) -> Result<()> { /// and then teach our webserver to redirect to the system for objects it doesn't /// have. fn generate_srv_repo(commit: &str) -> Result<()> { - bash!( - r#" - ostree --repo=${srvrepo} init --mode=archive - ostree --repo=${srvrepo} config set archive.zlib-level 1 - ostree --repo=${srvrepo} pull-local /sysroot/ostree/repo ${commit} - ostree --repo=${srvrepo} refs --create=${testref} ${commit} - "#, - srvrepo = SRVREPO, - commit = commit, - testref = TESTREF + let sh = xshell::Shell::new()?; + + cmd!(sh, "ostree --repo={SRVREPO} init --mode=archive").run()?; + cmd!( + sh, + "ostree --repo={SRVREPO} config set archive.zlib-level 1" ) + .run()?; + cmd!( + sh, + "ostree --repo={SRVREPO} pull-local /sysroot/ostree/repo {commit}" + ) + .run()?; + cmd!( + sh, + "ostree --repo={SRVREPO} refs --create={TESTREF} {commit}" + ) + .run() .context("Failed to generate srv repo")?; generate_update(commit)?; Ok(()) @@ -200,11 +203,14 @@ struct RebootStats { } fn upgrade_and_finalize() -> Result<()> { - bash!( + let sh = xshell::Shell::new()?; + cmd!( + sh, "rpm-ostree upgrade systemctl start ostree-finalize-staged systemctl stop ostree-finalize-staged" ) + .run() .context("Upgrade and finalize failed")?; Ok(()) } @@ -309,9 +315,10 @@ fn parse_and_validate_reboot_mark>( } fn validate_pending_commit(pending_commit: &str, commitstates: &CommitStates) -> Result<()> { + let sh = xshell::Shell::new()?; if pending_commit != commitstates.target { - bash!("rpm-ostree status -v")?; - bash!("ostree show ${pending_commit}", pending_commit)?; + cmd!(sh, "rpm-ostree status -v").run()?; + cmd!(sh, "ostree show {pending_commit}").run()?; anyhow::bail!( "Expected target commit={} but pending={} ({:?})", commitstates.target, @@ -622,17 +629,10 @@ pub(crate) fn itest_transactionality() -> Result<()> { // FIXME: make this saner let origref = ORIGREF; let testref = TESTREF; - bash!( - " - ostree admin set-origin testrepo ${url} ${testref} - ostree refs --create testrepo:${testref} ${commit} - ostree refs --create=${origref} ${commit} - ", - url, - origref, - testref, - commit - )?; + cmd!(sh, "ostree admin set-origin testrepo {url} {testref}").run()?; + cmd!(sh, "ostree refs --create testrepo:{testref} {commit}").run()?; + cmd!(sh, "ostree refs --create={origref} {commit}").run()?; + // We gather a single "cycle time" at start as a way of gauging how // long an upgrade should take, so we know when to interrupt. This // obviously has some pitfalls, mainly when there are e.g. other competing diff --git a/tests/inst/src/repobin.rs b/tests/inst/src/repobin.rs index af2096f2e4..5367b0634a 100644 --- a/tests/inst/src/repobin.rs +++ b/tests/inst/src/repobin.rs @@ -5,28 +5,27 @@ use std::path::Path; use crate::test::*; use anyhow::{Context, Result}; -use sh_inline::{bash, bash_command}; use with_procspawn_tempdir::with_procspawn_tempdir; +use xshell::cmd; pub(crate) fn itest_basic() -> Result<()> { - bash!(r"ostree --help >/dev/null")?; + let sh = xshell::Shell::new()?; + cmd!(sh, "ostree --help >/dev/null").run()?; Ok(()) } #[with_procspawn_tempdir] pub(crate) fn itest_nofifo() -> Result<()> { + let sh = xshell::Shell::new()?; assert!(std::path::Path::new(".procspawn-tmpdir").exists()); - bash!( - r"ostree --repo=repo init --mode=archive - mkdir tmproot - mkfifo tmproot/afile -" - )?; + cmd!(sh, "ostree --repo=repo init --mode=archive").run()?; + cmd!(sh, "mkdir tmproot").run()?; + cmd!(sh, "mkfifo tmproot/afile").run()?; cmd_fails_with( - bash_command!( - r#"ostree --repo=repo commit -b fifotest -s "commit fifo" --tree=dir=./tmproot"# - ) - .unwrap(), + cmd!( + sh, + "ostree --repo=repo commit -b fifotest -s 'commit fifo' --tree=dir=./tmproot" + ), "Not a regular file or symlink", )?; Ok(()) @@ -34,23 +33,30 @@ pub(crate) fn itest_nofifo() -> Result<()> { #[with_procspawn_tempdir] pub(crate) fn itest_mtime() -> Result<()> { - bash!( - r"ostree --repo=repo init --mode=archive - mkdir tmproot - echo afile > tmproot/afile - ostree --repo=repo commit -b test --tree=dir=tmproot >/dev/null -" - )?; + let sh = xshell::Shell::new()?; + cmd!(sh, "ostree --repo=repo init --mode=archive").run()?; + cmd!(sh, "mkdir tmproot").run()?; + cmd!(sh, "echo afile > tmproot/afile").run()?; + cmd!( + sh, + "ostree --repo=repo commit -b test --tree=dir=tmproot >/dev/null" + ) + .run()?; let ts = Path::new("repo").metadata()?.modified().unwrap(); std::thread::sleep(std::time::Duration::from_secs(1)); - bash!(r#"ostree --repo=repo commit -b test -s "bump mtime" --tree=dir=tmproot >/dev/null"#)?; + cmd!( + sh, + "ostree --repo=repo commit -b test -s 'bump mtime' --tree=dir=tmproot >/dev/null" + ) + .run()?; assert_ne!(ts, Path::new("repo").metadata()?.modified().unwrap()); Ok(()) } #[with_procspawn_tempdir] pub(crate) fn itest_extensions() -> Result<()> { - bash!(r"ostree --repo=repo init --mode=bare")?; + let sh = xshell::Shell::new()?; + cmd!(sh, "ostree --repo=repo init --mode=bare").run()?; assert!(Path::new("repo/extensions").exists()); Ok(()) } @@ -62,44 +68,59 @@ pub(crate) fn itest_pull_basicauth() -> Result<()> { ..Default::default() }; let serverrepo = Path::new("server/repo"); + let sh = xshell::Shell::new()?; std::fs::create_dir_all(&serverrepo)?; with_webserver_in(&serverrepo, &opts, move |addr| { - let baseuri = http::Uri::from_maybe_shared(format!("http://{}/", addr).into_bytes())?; + let baseuri = + http::Uri::from_maybe_shared(format!("http://{}/", addr).into_bytes())?.to_string(); let unauthuri = - http::Uri::from_maybe_shared(format!("http://unknown:badpw@{}/", addr).into_bytes())?; + http::Uri::from_maybe_shared(format!("http://unknown:badpw@{}/", addr).into_bytes())? + .to_string(); let authuri = http::Uri::from_maybe_shared( format!("http://{}@{}/", TEST_HTTP_BASIC_AUTH, addr).into_bytes(), - )?; + )? + .to_string(); let osroot = Path::new("osroot"); crate::treegen::mkroot(&osroot)?; - bash!( - r#"ostree --repo=${serverrepo} init --mode=archive - 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-badauth ${unauthuri} - ostree --repo=repo remote add --set=gpg-verify=false origin-goodauth ${authuri} - "#, - osroot = osroot, - serverrepo = serverrepo, - baseuri = baseuri.to_string(), - unauthuri = unauthuri.to_string(), - authuri = authuri.to_string() - )?; + cmd!(sh, "ostree --repo={serverrepo} init --mode=archive").run()?; + cmd!( + sh, + "ostree --repo={serverrepo} commit -b os --tree=dir={osroot} >/dev/null" + ) + .run()?; + let basedir = sh.current_dir(); + let dir = sh.create_dir("client")?; + let _p = sh.push_dir(dir); + cmd!(sh, "ostree --repo=repo init --mode=archive").run()?; + cmd!( + sh, + "ostree --repo=repo remote add --set=gpg-verify=false origin-unauth {baseuri}" + ) + .run()?; + cmd!( + sh, + "ostree --repo=repo remote add --set=gpg-verify=false origin-badauth {unauthuri}" + ) + .run()?; + cmd!( + sh, + "ostree --repo=repo remote add --set=gpg-verify=false origin-goodauth {authuri}" + ) + .run()?; + + let _p = sh.push_dir(basedir); for rem in &["unauth", "badauth"] { cmd_fails_with( - bash_command!( - r#"ostree --repo=client/repo pull origin-${rem} os >/dev/null"#, - rem = *rem - ) - .unwrap(), + cmd!(sh, "ostree --repo=client/repo pull origin-{rem} os") + .quiet() + .ignore_stdout(), "HTTP 403", ) .context(rem)?; } - bash!(r#"ostree --repo=client/repo pull origin-goodauth os >/dev/null"#,)?; + cmd!(sh, "ostree --repo=client/repo pull origin-goodauth os") + .ignore_stdout() + .run()?; Ok(()) })?; Ok(()) diff --git a/tests/inst/src/sysroot.rs b/tests/inst/src/sysroot.rs index 9f68f15f92..dc09cf13ab 100644 --- a/tests/inst/src/sysroot.rs +++ b/tests/inst/src/sysroot.rs @@ -50,7 +50,8 @@ pub(crate) fn itest_immutable_bit() -> Result<()> { return Ok(()); } // https://bugzilla.redhat.com/show_bug.cgi?id=1867601 - cmd_has_output(sh_inline::bash_command!("lsattr -d /").unwrap(), "-i-")?; + cmd_has_output(cmd!(sh, "lsattr -d /"), "-i-")?; + Ok(()) } diff --git a/tests/inst/src/test.rs b/tests/inst/src/test.rs index 9eafc56132..c98339b436 100644 --- a/tests/inst/src/test.rs +++ b/tests/inst/src/test.rs @@ -1,12 +1,11 @@ -use std::borrow::BorrowMut; use std::fs::File; use std::io::prelude::*; use std::path::Path; -use std::process::Command; use std::time; use anyhow::{bail, Context, Result}; use rand::Rng; +use xshell::Cmd; // HTTP Server deps use futures_util::future; @@ -16,30 +15,34 @@ use hyper_staticfile::Static; use tokio::runtime::Runtime; /// Run command and assert that its stderr contains pat -pub(crate) fn cmd_fails_with>(mut c: C, pat: &str) -> Result<()> { - let c = c.borrow_mut(); - let o = c.output()?; - if o.status.success() { - bail!("Command {:?} unexpectedly succeeded", c); +pub fn cmd_fails_with(cmd: Cmd, pat: &str) -> Result<()> { + let cmd_str = cmd.to_string(); + let output = cmd.ignore_status().output()?; + + if output.status.success() { + bail!("Command {:?} unexpectedly succeeded", cmd_str); } - if twoway::find_bytes(&o.stderr, pat.as_bytes()).is_none() { - dbg!(String::from_utf8_lossy(&o.stdout)); - dbg!(String::from_utf8_lossy(&o.stderr)); - bail!("Command {:?} stderr did not match: {}", c, pat); + + if twoway::find_bytes(&output.stderr, pat.as_bytes()).is_none() { + dbg!(String::from_utf8_lossy(&output.stdout)); + dbg!(String::from_utf8_lossy(&output.stderr)); + bail!("Command {:?} stderr did not match: {}", cmd_str, pat); } + Ok(()) } -/// Run command and assert that its stdout contains pat -pub(crate) fn cmd_has_output>(mut c: C, pat: &str) -> Result<()> { - let c = c.borrow_mut(); - let o = c.output()?; - if !o.status.success() { - bail!("Command {:?} failed", c); +/// Run command and assert that its stdout contains `pat` +pub(crate) fn cmd_has_output(cmd: Cmd, pat: &str) -> Result<()> { + let cmd_str = cmd.to_string(); + let output = cmd.output()?; // run, error if command fails + + if !output.status.success() { + bail!("Command {} failed", cmd_str); } - if twoway::find_bytes(&o.stdout, pat.as_bytes()).is_none() { - dbg!(String::from_utf8_lossy(&o.stdout)); - bail!("Command {:?} stdout did not match: {}", c, pat); + if twoway::find_bytes(&output.stdout, pat.as_bytes()).is_none() { + dbg!(String::from_utf8_lossy(&output.stdout)); + bail!("Command {} stdout did not match: {}", cmd_str, pat); } Ok(()) } @@ -201,36 +204,30 @@ pub(crate) fn prepare_reboot>(mark: M) -> Result<()> { mod tests { use super::*; - fn oops() -> Command { - let mut c = Command::new("/bin/bash"); - c.args(&["-c", "echo oops 1>&2; exit 1"]); - c + fn oops(sh: &Shell) -> Cmd { + cmd!(sh, "bash -c 'echo oops 1>&2; exit 1'") } #[test] fn test_fails_with_matches() -> Result<()> { - cmd_fails_with(Command::new("false"), "")?; - cmd_fails_with(oops(), "oops")?; + cmd_fails_with(cmd!(sh, "false"), "")?; + cmd_fails_with(oops(&sh), "oops")?; Ok(()) } #[test] - fn test_fails_with_fails() { - cmd_fails_with(Command::new("true"), "somepat").expect_err("true"); - cmd_fails_with(oops(), "nomatch").expect_err("nomatch"); + fn test_fails_with_fails() -> Result<()> { + let sh = Shell::new()?; + cmd_fails_with(cmd!(sh, "true"), "somepat").expect_err("true"); + cmd_fails_with(oops(&sh), "nomatch").expect_err("nomatch"); + Ok(()) } #[test] fn test_output() -> Result<()> { - cmd_has_output(Command::new("true"), "")?; - assert!(cmd_has_output(Command::new("true"), "foo").is_err()); - cmd_has_output( - sh_inline::bash_command!("echo foobarbaz; echo fooblahbaz").unwrap(), - "blah", - )?; - assert!( - cmd_has_output(sh_inline::bash_command!("echo foobarbaz").unwrap(), "blah").is_err() - ); + let sh = Shell::new()?; + cmd_has_output(cmd!(sh, "echo foobarbaz; echo fooblahbaz"), "blah")?; + assert!(cmd_has_output(cmd!(sh, "echo foobarbaz"), "blah").is_err()); Ok(()) }