From 00ead3f73d7698259a73b7f63a1cbc11429c9ad3 Mon Sep 17 00:00:00 2001 From: David Rousselie Date: Mon, 17 Jan 2022 17:09:54 +0100 Subject: [PATCH] Fix concurrency bug while testing --- .github/workflows/ci.yml | 2 +- Cargo.lock | 15 ++++++++++++++- Cargo.toml | 2 +- src/contextswitch.rs | 4 ++-- src/lib.rs | 4 ++-- src/taskwarrior.rs | 12 ++++++++---- tests/health_check.rs | 12 ++++++------ tests/task.rs | 24 ++++++++++++++---------- tests/test_helper.rs | 36 +++++++++++++++++++----------------- 9 files changed, 67 insertions(+), 44 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b590c5c..533430d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,7 +38,7 @@ jobs: - uses: actions-rs/clippy-check@v1 with: token: ${{ secrets.GITHUB_TOKEN }} - args: -- -D warnings + args: --tests -- -D warnings - name: Run cargo-tarpaulin uses: ./.github/actions/cargo-tarpaulin-action/ diff --git a/Cargo.lock b/Cargo.lock index ef17c87..44f4cef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -332,9 +332,9 @@ dependencies = [ "lazy_static", "listenfd", "mktemp", - "once_cell", "regex", "reqwest", + "rstest", "serde", "serde_json", "tokio", @@ -1214,6 +1214,19 @@ dependencies = [ "winreg", ] +[[package]] +name = "rstest" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d912f35156a3f99a66ee3e11ac2e0b3f34ac85a07e05263d05a7e2c8810d616f" +dependencies = [ + "cfg-if", + "proc-macro2", + "quote", + "rustc_version", + "syn", +] + [[package]] name = "rustc_version" version = "0.4.0" diff --git a/Cargo.toml b/Cargo.toml index f5840aa..01fe6be 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,5 +33,5 @@ lazy_static = "1.4.0" tracing-bunyan-formatter = "0.3.2" [dev-dependencies] -once_cell = "1.0" reqwest = { version = "0.11.0", features = ["json"] } +rstest = "0.12.0" diff --git a/src/contextswitch.rs b/src/contextswitch.rs index 25f4ff7..57d0ece 100644 --- a/src/contextswitch.rs +++ b/src/contextswitch.rs @@ -44,6 +44,6 @@ pub fn export(filters: Vec<&str>) -> Result, Error> { tasks } -pub fn add(add_args: Vec<&str>) -> Result { - taskwarrior::add(add_args) +pub async fn add(add_args: Vec<&str>) -> Result { + taskwarrior::add(add_args).await } diff --git a/src/lib.rs b/src/lib.rs index 2b1b411..deff222 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,7 +1,7 @@ use actix_web::{dev::Server, http, middleware, web, App, HttpResponse, HttpServer}; use contextswitch_types::TaskDefinition; use listenfd::ListenFd; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; use serde_json::json; use std::env; use std::io::Error; @@ -35,7 +35,7 @@ async fn list_tasks(task_query: web::Query) -> Result) -> Result { - let task_id = contextswitch::add(task_definition.definition.split(' ').collect())?; + let task_id = contextswitch::add(task_definition.definition.split(' ').collect()).await?; Ok(HttpResponse::Ok() .content_type("application/json") diff --git a/src/taskwarrior.rs b/src/taskwarrior.rs index ae1d455..cec4dc9 100644 --- a/src/taskwarrior.rs +++ b/src/taskwarrior.rs @@ -8,6 +8,7 @@ use std::io::{Error, ErrorKind}; use std::path::Path; use std::process::Command; use std::str; +use tokio::sync::Mutex; use tracing::debug; use uuid::Uuid; @@ -109,14 +110,17 @@ pub fn export(filters: Vec<&str>) -> Result, Error> { } #[tracing::instrument(level = "debug")] -pub fn add(add_args: Vec<&str>) -> Result { +pub async fn add(add_args: Vec<&str>) -> Result { + lazy_static! { + static ref RE: Regex = Regex::new(r"Created task (?P\d+).").unwrap(); + static ref LOCK: Mutex = Mutex::new(0); + } + let _lock = LOCK.lock().await; + let mut args = vec!["add"]; args.extend(add_args); let add_output = Command::new("task").args(args).output()?; let output = String::from_utf8(add_output.stdout).unwrap(); - lazy_static! { - static ref RE: Regex = Regex::new(r"Created task (?P\d+).").unwrap(); - } let task_id_capture = RE.captures(&output).ok_or_else(|| { Error::new( ErrorKind::Other, diff --git a/tests/health_check.rs b/tests/health_check.rs index b96ba3b..267a852 100644 --- a/tests/health_check.rs +++ b/tests/health_check.rs @@ -1,12 +1,12 @@ pub mod test_helper; +use rstest::*; +use test_helper::app_address; +#[rstest] #[tokio::test] -async fn health_check_works() { - let address = test_helper::spawn_app(); - let client = reqwest::Client::new(); - - let response = client - .get(&format!("{}/ping", &address)) +async fn health_check_works(app_address: &str) { + let response = reqwest::Client::new() + .get(&format!("{}/ping", &app_address)) .send() .await .expect("Failed to execute request."); diff --git a/tests/task.rs b/tests/task.rs index 925bd45..09c29be 100644 --- a/tests/task.rs +++ b/tests/task.rs @@ -3,15 +3,19 @@ pub mod test_helper; use contextswitch_api::taskwarrior; use contextswitch_types::Task; use contextswitch_types::TaskDefinition; +use rstest::*; +use test_helper::app_address; +#[rstest] #[tokio::test] -async fn list_tasks() { - let address = test_helper::spawn_app(); - let task_id = - taskwarrior::add(vec!["test", "list_tasks", "contextswitch:'{\"test\": 1}'"]).unwrap(); +async fn list_tasks(app_address: &str) { + let task_id = taskwarrior::add(vec!["test", "list_tasks", "contextswitch:'{\"test\": 1}'"]) + .await + .unwrap(); + println!("LIST TASKS ID: {}", task_id); let tasks: Vec = reqwest::Client::new() - .get(&format!("{}/tasks?filter={}", &address, task_id)) + .get(&format!("{}/tasks?filter={}", &app_address, task_id)) .send() .await .expect("Failed to execute request") @@ -25,13 +29,11 @@ async fn list_tasks() { assert_eq!(cs_metadata.test, 1); } +#[rstest] #[tokio::test] -async fn add_task() { - let address = test_helper::spawn_app(); - println!("add_task address: {}", address); - +async fn add_task(app_address: &str) { let response: serde_json::Value = reqwest::Client::new() - .post(&format!("{}/tasks", &address)) + .post(&format!("{}/tasks", &app_address)) .json(&TaskDefinition { definition: "test add_task contextswitch:{\"test\":1}".to_string(), }) @@ -41,8 +43,10 @@ async fn add_task() { .json() .await .expect("Cannot parse JSON result"); + println!("ADD RESPONSE: {:?}", response); let new_task_id = response["id"].as_u64().unwrap(); let tasks = taskwarrior::export(vec![&new_task_id.to_string()]).unwrap(); + println!("TASKS={:?}", tasks); assert_eq!(tasks.len(), 1); assert_eq!(tasks[0].id, new_task_id); diff --git a/tests/test_helper.rs b/tests/test_helper.rs index 3004e9b..8f1a119 100644 --- a/tests/test_helper.rs +++ b/tests/test_helper.rs @@ -1,42 +1,44 @@ use contextswitch_api::observability::{get_subscriber, init_subscriber}; use contextswitch_api::taskwarrior; use mktemp::Temp; -use once_cell::sync::Lazy; +use rstest::*; use std::fs; use std::net::TcpListener; +use tracing::info; -static TRACING: Lazy<()> = Lazy::new(|| { - let subscriber = get_subscriber("info".to_string()); +fn setup_tracing() { + info!("Setting up tracing"); + let subscriber = get_subscriber("debug".to_string()); init_subscriber(subscriber); -}); +} -static SERVER_ADDRESS: Lazy = Lazy::new(|| { +fn setup_server() -> String { + info!("Setting up server"); let listener = TcpListener::bind("127.0.0.1:0").expect("Failed to bind random port"); let port = listener.local_addr().unwrap().port(); let server = contextswitch_api::run(listener).expect("Failed to bind address"); let _ = tokio::spawn(server); format!("http://127.0.0.1:{}", port) -}); +} -static TASK_DATA_LOCATION: Lazy = Lazy::new(|| { +fn setup_taskwarrior() -> String { + info!("Setting up TW"); let tmp_dir = Temp::new_dir().unwrap(); let task_data_location = taskwarrior::load_config(tmp_dir.to_str()); tmp_dir.release(); task_data_location -}); - -pub fn spawn_app() -> String { - Lazy::force(&TRACING); - setup_tasks(); - Lazy::force(&SERVER_ADDRESS).to_string() -} - -pub fn setup_tasks() -> String { - Lazy::force(&TASK_DATA_LOCATION).to_string() } pub fn clear_tasks(task_data_location: String) { fs::remove_dir_all(task_data_location).unwrap(); } + +#[fixture] +#[once] +pub fn app_address() -> String { + setup_tracing(); + setup_taskwarrior(); + setup_server() +}