Merge pull request #673 from Szymongib/cleanup/remove-cmd-dup-in-tests

Remove duplication from commands execution in Integration tests

crates/integration_test/src/tests/lifecycle/delete.rs

@@ -1,18 +1,10 @@
 use super::get_result_from_output;
-use crate::utils::get_runtime_path;
+use crate::utils::delete_container;
 use std::path::Path;
-use std::process::{Command, Stdio};
 use test_framework::TestResult;
 
 pub fn delete(project_path: &Path, id: &str) -> TestResult {
-    let res = Command::new(get_runtime_path())
-        .stdout(Stdio::piped())
-        .stderr(Stdio::piped())
-        .arg("--root")
-        .arg(project_path.join("runtime"))
-        .arg("delete")
-        .arg(id)
-        .spawn()
+    let res = delete_container(id, project_path)
         .expect("failed to execute delete command")
         .wait_with_output();
     get_result_from_output(res)

crates/integration_test/src/tests/lifecycle/kill.rs

@@ -1,19 +1,10 @@
 use super::get_result_from_output;
-use crate::utils::get_runtime_path;
+use crate::utils::kill_container;
 use std::path::Path;
-use std::process::{Command, Stdio};
 use test_framework::TestResult;
 
 pub fn kill(project_path: &Path, id: &str) -> TestResult {
-    let res = Command::new(get_runtime_path())
-        .stdout(Stdio::piped())
-        .stderr(Stdio::piped())
-        .arg("--root")
-        .arg(project_path.join("runtime"))
-        .arg("kill")
-        .arg(id)
-        .arg("9")
-        .spawn()
+    let res = kill_container(id, project_path)
         .expect("failed to execute kill command")
         .wait_with_output();
     get_result_from_output(res)

crates/integration_test/src/tests/lifecycle/start.rs

@@ -1,18 +1,10 @@
 use super::get_result_from_output;
-use crate::utils::get_runtime_path;
+use crate::utils::test_utils::start_container;
 use std::path::Path;
-use std::process::{Command, Stdio};
 use test_framework::TestResult;
 
 pub fn start(project_path: &Path, id: &str) -> TestResult {
-    let res = Command::new(get_runtime_path())
-        .stdout(Stdio::piped())
-        .stderr(Stdio::piped())
-        .arg("--root")
-        .arg(project_path.join("runtime"))
-        .arg("start")
-        .arg(id)
-        .spawn()
+    let res = start_container(id, project_path)
         .expect("failed to execute start command")
         .wait_with_output();
     get_result_from_output(res)

crates/integration_test/src/tests/lifecycle/state.rs

@@ -1,41 +1,24 @@
-use crate::utils::get_runtime_path;
-use std::io;
+use crate::utils::get_state;
+use anyhow::{anyhow, Result};
 use std::path::Path;
-use std::process::{Command, Stdio};
 use test_framework::TestResult;
 
 pub fn state(project_path: &Path, id: &str) -> TestResult {
-    let res = Command::new(get_runtime_path())
-        .stdout(Stdio::piped())
-        .stderr(Stdio::piped())
-        .arg("--root")
-        .arg(project_path.join("runtime"))
-        .arg("state")
-        .arg(id)
-        .spawn()
-        .expect("failed to execute state command")
-        .wait_with_output();
-    match res {
-        io::Result::Ok(output) => {
-            let stderr = String::from_utf8(output.stderr).unwrap();
-            let stdout = String::from_utf8(output.stdout).unwrap();
+    match get_state(id, project_path) {
+        Result::Ok((stdout, stderr)) => {
             if stderr.contains("Error") || stderr.contains("error") {
-                TestResult::Failed(anyhow::anyhow!(
-                    "Error :\nstdout : {}\nstderr : {}",
-                    stdout,
-                    stderr
-                ))
+                TestResult::Failed(anyhow!("Error :\nstdout : {}\nstderr : {}", stdout, stderr))
             } else {
                 // confirm that the status is stopped, as this is executed after the kill command
                 if !(stdout.contains(&format!(r#""id": "{}""#, id))
                     && stdout.contains(r#""status": "stopped""#))
                 {
-                    TestResult::Failed(anyhow::anyhow!("Expected state stopped, got : {}", stdout))
+                    TestResult::Failed(anyhow!("Expected state stopped, got : {}", stdout))
                 } else {
                     TestResult::Passed
                 }
             }
         }
-        io::Result::Err(e) => TestResult::Failed(anyhow::Error::new(e)),
+        Result::Err(e) => TestResult::Failed(e.context("failed to get container state")),
     }
 }

crates/integration_test/src/tests/pidfile/pidfile_test.rs

@@ -9,8 +9,9 @@ use uuid::Uuid;
 
 #[inline]
 fn cleanup(id: &Uuid, bundle: &TempDir) {
-    kill_container(id, bundle).unwrap().wait().unwrap();
-    delete_container(id, bundle).unwrap().wait().unwrap();
+    let str_id = id.to_string();
+    kill_container(&str_id, bundle).unwrap().wait().unwrap();
+    delete_container(&str_id, bundle).unwrap().wait().unwrap();
 }
 
 // here we have to manually create and manage the container
@@ -42,7 +43,7 @@ fn test_pidfile() -> TestResult {
         .wait()
         .unwrap();
 
-    let (out, err) = get_state(&container_id, &bundle).unwrap();
+    let (out, err) = get_state(&container_id.to_string(), &bundle).unwrap();
 
     if !err.is_empty() {
         cleanup(&container_id, &bundle);

crates/integration_test/src/utils/support.rs

@@ -78,7 +78,7 @@ pub fn prepare_bundle(id: &Uuid) -> Result<TempDir> {
 
     let mut spec = Spec::default();
     let mut process = Process::default();
-    process.set_args(Some(vec!["sleep".into(), "5".into()]));
+    process.set_args(Some(vec!["sleep".into(), "10".into()]));
     spec.set_process(Some(process));
     set_config(&temp_dir, &spec).unwrap();
 

crates/integration_test/src/utils/test_utils.rs

@@ -11,7 +11,6 @@ use std::process::{Child, Command, ExitStatus, Stdio};
 use std::thread::sleep;
 use std::time::Duration;
 use test_framework::{test_result, TestResult};
-use uuid::Uuid;
 
 const SLEEP_TIME: Duration = Duration::from_millis(150);
 pub const CGROUP_ROOT: &str = "/sys/fs/cgroup";
@@ -43,7 +42,7 @@ pub struct ContainerData {
 }
 
 /// Starts the runtime with given directory as root directory
-pub fn create_container<P: AsRef<Path>>(id: &Uuid, dir: P) -> Result<Child> {
+pub fn create_container<P: AsRef<Path>>(id: &str, dir: P) -> Result<Child> {
     let res = Command::new(get_runtime_path())
         // set stdio so that we can get o/p of runtimetest
         // in test_inside_container function
@@ -55,7 +54,7 @@ pub fn create_container<P: AsRef<Path>>(id: &Uuid, dir: P) -> Result<Child> {
         .arg("--root")
         .arg(dir.as_ref().join("runtime"))
         .arg("create")
-        .arg(id.to_string())
+        .arg(id)
         .arg("--bundle")
         .arg(dir.as_ref().join("bundle"))
         .spawn()
@@ -64,71 +63,68 @@ pub fn create_container<P: AsRef<Path>>(id: &Uuid, dir: P) -> Result<Child> {
 }
 
 /// Sends a kill command to the given container process
-pub fn kill_container<P: AsRef<Path>>(id: &Uuid, dir: P) -> Result<Child> {
-    let res = Command::new(get_runtime_path())
-        .stdout(Stdio::piped())
-        .stderr(Stdio::piped())
-        .arg("--root")
-        .arg(dir.as_ref().join("runtime"))
+pub fn kill_container<P: AsRef<Path>>(id: &str, dir: P) -> Result<Child> {
+    let res = runtime_command(dir)
         .arg("kill")
-        .arg(id.to_string())
+        .arg(id)
         .arg("9")
         .spawn()
         .context("could not kill container")?;
     Ok(res)
 }
 
-pub fn delete_container<P: AsRef<Path>>(id: &Uuid, dir: P) -> Result<Child> {
-    let res = Command::new(get_runtime_path())
-        .stdout(Stdio::piped())
-        .stderr(Stdio::piped())
-        .arg("--root")
-        .arg(dir.as_ref().join("runtime"))
+pub fn delete_container<P: AsRef<Path>>(id: &str, dir: P) -> Result<Child> {
+    let res = runtime_command(dir)
         .arg("delete")
-        .arg(id.to_string())
+        .arg(id)
         .spawn()
         .context("could not delete container")?;
     Ok(res)
 }
 
-pub fn get_state<P: AsRef<Path>>(id: &Uuid, dir: P) -> Result<(String, String)> {
+pub fn get_state<P: AsRef<Path>>(id: &str, dir: P) -> Result<(String, String)> {
     sleep(SLEEP_TIME);
-    let output = Command::new(get_runtime_path())
-        .stdout(Stdio::piped())
-        .stderr(Stdio::piped())
-        .arg("--root")
-        .arg(dir.as_ref().join("runtime"))
+    let output = runtime_command(dir)
         .arg("state")
         .arg(id.to_string())
-        .spawn()?
-        .wait_with_output()?;
+        .spawn()
+        .context("could not get container state")?
+        .wait_with_output()
+        .context("failed while waiting for state command")?;
     let stderr = String::from_utf8(output.stderr).context("failed to parse std error stream")?;
     let stdout = String::from_utf8(output.stdout).context("failed to parse std output stream")?;
     Ok((stdout, stderr))
 }
 
-pub fn start_container<P: AsRef<Path>>(id: &Uuid, dir: P) -> Result<Child> {
-    let res = Command::new(get_runtime_path())
-        .stderr(Stdio::piped())
-        .stdout(Stdio::piped())
-        .arg("--root")
-        .arg(dir.as_ref().join("runtime"))
+pub fn start_container<P: AsRef<Path>>(id: &str, dir: P) -> Result<Child> {
+    let res = runtime_command(dir)
         .arg("start")
         .arg(id.to_string())
         .spawn()
         .context("could not start container")?;
     Ok(res)
 }
 
+fn runtime_command<P: AsRef<Path>>(dir: P) -> Command {
+    let mut command = Command::new(get_runtime_path());
+    command
+        .stdout(Stdio::piped())
+        .stderr(Stdio::piped())
+        .arg("--root")
+        .arg(dir.as_ref().join("runtime"));
+    command
+}
+
 pub fn test_outside_container(
     spec: Spec,
     execute_test: &dyn Fn(ContainerData) -> TestResult,
 ) -> TestResult {
     let id = generate_uuid();
+    let id_str = id.to_string();
     let bundle = prepare_bundle(&id).unwrap();
     set_config(&bundle, &spec).unwrap();
-    let create_result = create_container(&id, &bundle).unwrap().wait();
-    let (out, err) = get_state(&id, &bundle).unwrap();
+    let create_result = create_container(&id_str, &bundle).unwrap().wait();
+    let (out, err) = get_state(&id_str, &bundle).unwrap();
     let state: Option<State> = match serde_json::from_str(&out) {
         Ok(v) => Some(v),
         Err(_) => None,
@@ -140,8 +136,8 @@ pub fn test_outside_container(
         create_result,
     };
     let test_result = execute_test(data);
-    kill_container(&id, &bundle).unwrap().wait().unwrap();
-    delete_container(&id, &bundle).unwrap().wait().unwrap();
+    kill_container(&id_str, &bundle).unwrap().wait().unwrap();
+    delete_container(&id_str, &bundle).unwrap().wait().unwrap();
     test_result
 }
 
@@ -151,6 +147,7 @@ pub fn test_inside_container(
     setup_for_test: &dyn Fn(&Path) -> Result<()>,
 ) -> TestResult {
     let id = generate_uuid();
+    let id_str = id.to_string();
     let bundle = prepare_bundle(&id).unwrap();
 
     // This will do the required setup for the test
@@ -181,15 +178,18 @@ pub fn test_inside_container(
             .join("runtimetest"),
     )
     .unwrap();
-    let create_process = create_container(&id, &bundle).unwrap();
+    let create_process = create_container(&id_str, &bundle).unwrap();
     // here we do not wait for the process by calling wait() as in the test_outside_container
     // function because we need the output of the runtimetest. If we call wait, it will return
     // and we won't have an easy way of getting the stdio of the runtimetest.
     // Thus to make sure the container is created, we just wait for sometime, and
     // assume that the create command was successful. If it wasn't we can catch that error
     // in the start_container, as we can not start a non-created container anyways
     std::thread::sleep(std::time::Duration::from_millis(1000));
-    match start_container(&id, &bundle).unwrap().wait_with_output() {
+    match start_container(&id_str, &bundle)
+        .unwrap()
+        .wait_with_output()
+    {
         Ok(c) => c,
         Err(e) => return TestResult::Failed(anyhow!("container start failed : {:?}", e)),
     };
@@ -207,23 +207,23 @@ pub fn test_inside_container(
         ));
     }
 
-    let (out, err) = get_state(&id, &bundle).unwrap();
+    let (out, err) = get_state(&id_str, &bundle).unwrap();
     if !err.is_empty() {
         return TestResult::Failed(anyhow!(
             "error in getting state after starting the container : {}",
             err
         ));
     }
 
-    let state:State = match serde_json::from_str(&out) {
+    let state: State = match serde_json::from_str(&out) {
         Ok(v) => v,
         Err(e) => return TestResult::Failed(anyhow!("error in parsing state of container after start in test_inside_container : stdout : {}, parse error : {}",out,e)),
     };
     if state.status != "stopped" {
         return TestResult::Failed(anyhow!("error : unexpected container status in test_inside_runtime : expected stopped, got {}, container state : {:?}",state.status,state));
     }
-    kill_container(&id, &bundle).unwrap().wait().unwrap();
-    delete_container(&id, &bundle).unwrap().wait().unwrap();
+    kill_container(&id_str, &bundle).unwrap().wait().unwrap();
+    delete_container(&id_str, &bundle).unwrap().wait().unwrap();
     TestResult::Passed
 }