From 34d6b574c17866f59bd3dd1ab922f3f6a306cb5a Mon Sep 17 00:00:00 2001 From: Chip Senkbeil Date: Sun, 5 Sep 2021 17:20:47 -0500 Subject: [PATCH] Fix clippy warnings, fix rustfmt, refactor proc-run tests to use generated scripts instead of script files --- .github/workflows/ci.yml | 32 +------ Cargo.lock | 17 ++++ Cargo.toml | 1 + core/Cargo.toml | 1 + core/src/client/lsp/mod.rs | 7 +- core/src/net/listener.rs | 6 +- core/src/server/distant/handler.rs | 128 +++++++++++++++++---------- core/src/server/distant/mod.rs | 1 + core/src/server/distant/state.rs | 4 +- core/src/server/relay.rs | 1 + core/src/server/utils.rs | 10 +-- core/tests/transport_test.rs | 2 +- scripts/test/echo_args_to_stderr.sh | 3 - scripts/test/echo_args_to_stdout.sh | 3 - scripts/test/echo_stdin_to_stdout.sh | 3 - scripts/test/exit_code.sh | 3 - scripts/test/sleep.sh | 3 - src/buf.rs | 4 +- src/exit.rs | 4 +- src/opt.rs | 7 +- src/output.rs | 17 ++-- src/session.rs | 2 +- src/subcommand/listen.rs | 8 +- tests/cli/action/dir_read.rs | 2 +- tests/cli/action/proc_run.rs | 62 ++++++++++--- tests/cli/fixtures.rs | 2 +- 26 files changed, 186 insertions(+), 147 deletions(-) delete mode 100755 scripts/test/echo_args_to_stderr.sh delete mode 100755 scripts/test/echo_args_to_stdout.sh delete mode 100755 scripts/test/echo_stdin_to_stdout.sh delete mode 100755 scripts/test/exit_code.sh delete mode 100755 scripts/test/sleep.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 13ba709..fb41c16 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,18 +21,12 @@ jobs: - { rust: stable, os: windows-latest } steps: - uses: actions/checkout@v2 - - uses: actions/cache@v2 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} - name: Install Rust ${{ matrix.rust }} uses: actions-rs/toolchain@v1 with: profile: minimal toolchain: ${{ matrix.rust }} + - uses: Swatinem/rust-cache@v1 - name: Check Cargo availability run: cargo --version - run: cargo test --verbose -p distant-core @@ -49,19 +43,13 @@ jobs: - { rust: stable, os: macos-latest } steps: - uses: actions/checkout@v2 - - uses: actions/cache@v2 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} - name: Install Rust ${{ matrix.rust }} uses: actions-rs/toolchain@v1 with: profile: minimal toolchain: ${{ matrix.rust }} components: rustfmt, clippy + - uses: Swatinem/rust-cache@v1 - name: Check Cargo availability run: cargo --version - run: cargo test --verbose @@ -73,19 +61,13 @@ jobs: RUSTFLAGS: -Dwarnings steps: - uses: actions/checkout@v2 - - uses: actions/cache@v2 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} - name: Install Rust (clippy) uses: actions-rs/toolchain@v1 with: profile: minimal toolchain: stable components: clippy + - uses: Swatinem/rust-cache@v1 - name: Check Cargo availability run: cargo --version - run: cargo clippy --workspace --all-targets --verbose @@ -96,19 +78,13 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - uses: actions/cache@v2 - with: - path: | - ~/.cargo/registry - ~/.cargo/git - target - key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} - name: Install Rust (rustfmt) uses: actions-rs/toolchain@v1 with: profile: minimal toolchain: stable components: rustfmt + - uses: Swatinem/rust-cache@v1 - name: Check Cargo availability run: cargo --version - run: cargo fmt --all -- --check diff --git a/Cargo.lock b/Cargo.lock index 3d57eb6..74c7a4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -242,6 +242,7 @@ dependencies = [ "distant-core", "flexi_logger", "fork", + "indoc", "lazy_static", "log", "predicates", @@ -263,6 +264,7 @@ dependencies = [ "derive_more", "futures", "hex", + "indoc", "k256", "lazy_static", "log", @@ -583,6 +585,15 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "indoc" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5a75aeaaef0ce18b58056d306c27b07436fbb34b8816c53094b76dd81803136" +dependencies = [ + "unindent", +] + [[package]] name = "instant" version = "0.1.10" @@ -1320,6 +1331,12 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3" +[[package]] +name = "unindent" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f14ee04d9415b52b3aeab06258a3f07093182b88ba0f9b8d203f211a7a7d41c7" + [[package]] name = "vec_map" version = "0.8.2" diff --git a/Cargo.toml b/Cargo.toml index 1fd3598..9210790 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,5 +36,6 @@ whoami = "1.1.2" [dev-dependencies] assert_cmd = "2.0.0" assert_fs = "1.0.4" +indoc = "1.0.3" predicates = "2.0.2" rstest = "0.11.0" diff --git a/core/Cargo.toml b/core/Cargo.toml index e0767fa..9a97c1a 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -33,4 +33,5 @@ structopt = { version = "0.3.22", optional = true } [dev-dependencies] assert_fs = "1.0.4" +indoc = "1.0.3" predicates = "2.0.2" diff --git a/core/src/client/lsp/mod.rs b/core/src/client/lsp/mod.rs index bd5dd26..9f4ec89 100644 --- a/core/src/client/lsp/mod.rs +++ b/core/src/client/lsp/mod.rs @@ -184,12 +184,7 @@ where let read_task = tokio::spawn(async move { let mut task_buf: Option = None; - loop { - let data = match stream.next().await { - Some(data) => data, - None => break, - }; - + while let Some(data) = stream.next().await { // Create or insert into our buffer match &mut task_buf { Some(buf) => buf.push_str(&data), diff --git a/core/src/net/listener.rs b/core/src/net/listener.rs index ba814b3..4dbf413 100644 --- a/core/src/net/listener.rs +++ b/core/src/net/listener.rs @@ -145,7 +145,7 @@ impl Listener for TcpListener { where Self: Sync + 'a, { - async fn accept<'a>(_self: &'a TcpListener) -> io::Result { + async fn accept(_self: &TcpListener) -> io::Result { _self.accept().await.map(|(stream, _)| stream) } @@ -180,8 +180,8 @@ where where Self: Sync + 'a, { - async fn accept<'a, T>( - _self: &'a tokio::sync::Mutex>, + async fn accept( + _self: &tokio::sync::Mutex>, ) -> io::Result where T: DataStream + Send + Sync + 'static, diff --git a/core/src/server/distant/handler.rs b/core/src/server/distant/handler.rs index 6c793b2..4bec09b 100644 --- a/core/src/server/distant/handler.rs +++ b/core/src/server/distant/handler.rs @@ -294,15 +294,15 @@ async fn copy(src: PathBuf, dst: PathBuf) -> Result { // Create the destination directory for the file when copying tokio::fs::create_dir_all(dst_parent_dir.as_path()).await?; + let dst_path = dst_parent_dir.join(local_src_file_name); + // Perform copying from entry to destination (if a file/symlink) if !entry.file_type().is_dir() { - let dst_file = dst_parent_dir.join(local_src_file_name); - tokio::fs::copy(entry.path(), dst_file).await?; + tokio::fs::copy(entry.path(), dst_path).await?; // Otherwise, if a directory, create it } else { - let dst_dir = dst_parent_dir.join(local_src_file_name); - tokio::fs::create_dir(dst_dir).await?; + tokio::fs::create_dir(dst_path).await?; } } } else { @@ -409,7 +409,7 @@ async fn proc_run( None, vec![ResponseData::ProcStdout { id, data }], ); - if let Err(_) = tx_2.send(res).await { + if tx_2.send(res).await.is_err() { error!(" Stdout channel closed", conn_id); break; } @@ -445,7 +445,7 @@ async fn proc_run( None, vec![ResponseData::ProcStderr { id, data }], ); - if let Err(_) = tx_2.send(res).await { + if tx_2.send(res).await.is_err() { error!(" Stderr channel closed", conn_id); break; } @@ -513,7 +513,7 @@ async fn proc_run( None, vec![ResponseData::ProcDone { id, success, code }] ); - if let Err(_) = tx.send(res).await { + if tx.send(res).await.is_err() { error!( " Failed to send done for process {}!", conn_id, @@ -523,7 +523,7 @@ async fn proc_run( } Err(x) => { let res = Response::new(tenant.as_str(), None, vec![ResponseData::from(x)]); - if let Err(_) = tx.send(res).await { + if tx.send(res).await.is_err() { error!( " Failed to send error for waiting on process {}!", conn_id, @@ -558,10 +558,7 @@ async fn proc_run( let res = Response::new(tenant.as_str(), None, vec![ResponseData::ProcDone { id, success: false, code: None }]); - if let Err(_) = tx - .send(res) - .await - { + if tx.send(res).await.is_err() { error!(" Failed to send done for process {}!", conn_id, id); } } @@ -642,22 +639,56 @@ mod tests { use std::time::Duration; lazy_static::lazy_static! { - // TODO: This is a workaround to get the workspace root directory from within a specific - // workspace member as there is no environment variable to support this. - // - // See https://github.com/rust-lang/cargo/issues/3946 - static ref ROOT_DIR: PathBuf = { - PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("..").canonicalize().unwrap() + static ref TEMP_SCRIPT_DIR: assert_fs::TempDir = assert_fs::TempDir::new().unwrap(); + static ref SCRIPT_RUNNER: String = String::from("bash"); + + static ref ECHO_ARGS_TO_STDOUT_SH: assert_fs::fixture::ChildPath = { + let script = TEMP_SCRIPT_DIR.child("echo_args_to_stdout.sh"); + script.write_str(indoc::indoc!(r#" + #/usr/bin/env bash + printf "%s" "$@" + "#)).unwrap(); + script + }; + + static ref ECHO_ARGS_TO_STDERR_SH: assert_fs::fixture::ChildPath = { + let script = TEMP_SCRIPT_DIR.child("echo_args_to_stderr.sh"); + script.write_str(indoc::indoc!(r#" + #/usr/bin/env bash + printf "%s" "$@" 1>&2 + "#)).unwrap(); + script + }; + + static ref ECHO_STDIN_TO_STDOUT_SH: assert_fs::fixture::ChildPath = { + let script = TEMP_SCRIPT_DIR.child("echo_stdin_to_stdout.sh"); + script.write_str(indoc::indoc!(r#" + #/usr/bin/env bash + while IFS= read; do echo "$REPLY"; done + "#)).unwrap(); + script + }; + + static ref EXIT_CODE_SH: assert_fs::fixture::ChildPath = { + let script = TEMP_SCRIPT_DIR.child("exit_code.sh"); + script.write_str(indoc::indoc!(r#" + #!/usr/bin/env bash + exit "$1" + "#)).unwrap(); + script }; - static ref SCRIPT_DIR: PathBuf = ROOT_DIR.join("scripts").join("test"); - static ref ECHO_ARGS_TO_STDOUT_SH: PathBuf = SCRIPT_DIR.join("echo_args_to_stdout.sh"); - static ref ECHO_ARGS_TO_STDERR_SH: PathBuf = SCRIPT_DIR.join("echo_args_to_stderr.sh"); - static ref ECHO_STDIN_TO_STDOUT_SH: PathBuf = SCRIPT_DIR.join("echo_stdin_to_stdout.sh"); - static ref EXIT_CODE_SH: PathBuf = SCRIPT_DIR.join("exit_code.sh"); - static ref SLEEP_SH: PathBuf = SCRIPT_DIR.join("sleep.sh"); + static ref SLEEP_SH: assert_fs::fixture::ChildPath = { + let script = TEMP_SCRIPT_DIR.child("sleep.sh"); + script.write_str(indoc::indoc!(r#" + #!/usr/bin/env bash + sleep "$1" + "#)).unwrap(); + script + }; - static ref DOES_NOT_EXIST_BIN: PathBuf = SCRIPT_DIR.join("does_not_exist_bin"); + static ref DOES_NOT_EXIST_BIN: assert_fs::fixture::ChildPath = + TEMP_SCRIPT_DIR.child("does_not_exist_bin"); } fn setup( @@ -729,10 +760,7 @@ mod tests { let temp = assert_fs::TempDir::new().unwrap(); let path = temp.child("missing-file").path().to_path_buf(); - let req = Request::new( - "test-tenant", - vec![RequestData::FileReadText { path: path }], - ); + let req = Request::new("test-tenant", vec![RequestData::FileReadText { path }]); process(conn_id, state, req, tx).await.unwrap(); @@ -2043,8 +2071,8 @@ mod tests { let req = Request::new( "test-tenant", vec![RequestData::ProcRun { - cmd: ECHO_ARGS_TO_STDOUT_SH.to_str().unwrap().to_string(), - args: Vec::new(), + cmd: SCRIPT_RUNNER.to_string(), + args: vec![ECHO_ARGS_TO_STDOUT_SH.to_str().unwrap().to_string()], }], ); @@ -2069,8 +2097,11 @@ mod tests { let req = Request::new( "test-tenant", vec![RequestData::ProcRun { - cmd: ECHO_ARGS_TO_STDOUT_SH.to_str().unwrap().to_string(), - args: vec![String::from("some stdout")], + cmd: SCRIPT_RUNNER.to_string(), + args: vec![ + ECHO_ARGS_TO_STDOUT_SH.to_str().unwrap().to_string(), + String::from("some stdout"), + ], }], ); @@ -2128,8 +2159,11 @@ mod tests { let req = Request::new( "test-tenant", vec![RequestData::ProcRun { - cmd: ECHO_ARGS_TO_STDERR_SH.to_str().unwrap().to_string(), - args: vec![String::from("some stderr")], + cmd: SCRIPT_RUNNER.to_string(), + args: vec![ + ECHO_ARGS_TO_STDERR_SH.to_str().unwrap().to_string(), + String::from("some stderr"), + ], }], ); @@ -2187,8 +2221,8 @@ mod tests { let req = Request::new( "test-tenant", vec![RequestData::ProcRun { - cmd: SLEEP_SH.to_str().unwrap().to_string(), - args: vec![String::from("0.1")], + cmd: SCRIPT_RUNNER.to_string(), + args: vec![SLEEP_SH.to_str().unwrap().to_string(), String::from("0.1")], }], ); @@ -2229,8 +2263,8 @@ mod tests { let req = Request::new( "test-tenant", vec![RequestData::ProcRun { - cmd: SLEEP_SH.to_str().unwrap().to_string(), - args: vec![String::from("1")], + cmd: SCRIPT_RUNNER.to_string(), + args: vec![SLEEP_SH.to_str().unwrap().to_string(), String::from("1")], }], ); @@ -2303,8 +2337,8 @@ mod tests { let req = Request::new( "test-tenant", vec![RequestData::ProcRun { - cmd: SLEEP_SH.to_str().unwrap().to_string(), - args: vec![String::from("1")], + cmd: SCRIPT_RUNNER.to_string(), + args: vec![SLEEP_SH.to_str().unwrap().to_string(), String::from("1")], }], ); @@ -2395,8 +2429,8 @@ mod tests { let req = Request::new( "test-tenant", vec![RequestData::ProcRun { - cmd: ECHO_STDIN_TO_STDOUT_SH.to_str().unwrap().to_string(), - args: Vec::new(), + cmd: SCRIPT_RUNNER.to_string(), + args: vec![ECHO_STDIN_TO_STDOUT_SH.to_str().unwrap().to_string()], }], ); @@ -2467,8 +2501,8 @@ mod tests { "test-tenant", vec![ RequestData::ProcRun { - cmd: SLEEP_SH.to_str().unwrap().to_string(), - args: vec![String::from("1")], + cmd: SCRIPT_RUNNER.to_string(), + args: vec![SLEEP_SH.to_str().unwrap().to_string(), String::from("1")], }, RequestData::ProcList {}, ], @@ -2490,8 +2524,8 @@ mod tests { res.payload[1], ResponseData::ProcEntries { entries: vec![RunningProcess { - cmd: SLEEP_SH.to_str().unwrap().to_string(), - args: vec![String::from("1")], + cmd: SCRIPT_RUNNER.to_string(), + args: vec![SLEEP_SH.to_str().unwrap().to_string(), String::from("1")], id, }], }, diff --git a/core/src/server/distant/mod.rs b/core/src/server/distant/mod.rs index 91dff63..fe46378 100644 --- a/core/src/server/distant/mod.rs +++ b/core/src/server/distant/mod.rs @@ -275,6 +275,7 @@ mod tests { }; use std::pin::Pin; + #[allow(clippy::type_complexity)] fn make_transport_stream() -> ( mpsc::Sender>, Pin> + Send>>, diff --git a/core/src/server/distant/state.rs b/core/src/server/distant/state.rs index 7f78837..59485d5 100644 --- a/core/src/server/distant/state.rs +++ b/core/src/server/distant/state.rs @@ -25,7 +25,7 @@ impl State { pub fn push_process(&mut self, conn_id: usize, process: Process) { self.client_processes .entry(conn_id) - .or_insert(Vec::new()) + .or_insert_with(Vec::new) .push(process.id); self.processes.insert(process.id, process); } @@ -69,7 +69,7 @@ impl State { conn_id, process.id ); - if let Err(_) = process.kill_tx.send(()) { + if process.kill_tx.send(()).is_err() { error!( "Conn {} failed to send process {} kill signal", id, process.id diff --git a/core/src/server/relay.rs b/core/src/server/relay.rs index 404de3d..a3829a7 100644 --- a/core/src/server/relay.rs +++ b/core/src/server/relay.rs @@ -420,6 +420,7 @@ mod tests { (t1, Session::initialize(t2).unwrap()) } + #[allow(clippy::type_complexity)] fn make_transport_stream() -> ( mpsc::Sender>, Pin> + Send>>, diff --git a/core/src/server/utils.rs b/core/src/server/utils.rs index 53b539c..89615dd 100644 --- a/core/src/server/utils.rs +++ b/core/src/server/utils.rs @@ -148,7 +148,7 @@ mod tests { "Shutdown task unexpectedly completed" ); - time::sleep(Duration::from_millis(15)).await; + time::sleep(Duration::from_millis(50)).await; assert!( futures::poll!(task).is_pending(), @@ -164,7 +164,7 @@ mod tests { "Shutdown task unexpectedly completed" ); - time::sleep(Duration::from_millis(15)).await; + time::sleep(Duration::from_millis(50)).await; assert!( futures::poll!(task).is_ready(), @@ -182,14 +182,14 @@ mod tests { "Shutdown task unexpectedly completed" ); - time::sleep(Duration::from_millis(15)).await; + time::sleep(Duration::from_millis(50)).await; assert!( futures::poll!(&mut task).is_pending(), "Shutdown task unexpectedly completed" ); task.tracker().lock().await.decrement(); - time::sleep(Duration::from_millis(15)).await; + time::sleep(Duration::from_millis(50)).await; assert!( futures::poll!(task).is_ready(), @@ -199,7 +199,7 @@ mod tests { #[tokio::test] async fn shutdown_task_should_not_resolve_before_minimum_duration() { - let mut task = ShutdownTask::initialize(Duration::from_millis(10)); + let mut task = ShutdownTask::initialize(Duration::from_millis(50)); assert!( futures::poll!(&mut task).is_pending(), "Shutdown task unexpectedly completed" diff --git a/core/tests/transport_test.rs b/core/tests/transport_test.rs index 32a4f2c..d556e20 100644 --- a/core/tests/transport_test.rs +++ b/core/tests/transport_test.rs @@ -1,4 +1,4 @@ -use distant_core::{Transport, TransportError, InmemoryStream, SecretKey}; +use distant_core::{InmemoryStream, SecretKey, Transport, TransportError}; use std::{io, sync::Arc}; const BUFFER_SIZE: usize = 100; diff --git a/scripts/test/echo_args_to_stderr.sh b/scripts/test/echo_args_to_stderr.sh deleted file mode 100755 index dc9ee53..0000000 --- a/scripts/test/echo_args_to_stderr.sh +++ /dev/null @@ -1,3 +0,0 @@ -#/usr/bin/env bash - -printf "%s" "$@" 1>&2 diff --git a/scripts/test/echo_args_to_stdout.sh b/scripts/test/echo_args_to_stdout.sh deleted file mode 100755 index ae3ace5..0000000 --- a/scripts/test/echo_args_to_stdout.sh +++ /dev/null @@ -1,3 +0,0 @@ -#/usr/bin/env bash - -printf "%s" "$@" diff --git a/scripts/test/echo_stdin_to_stdout.sh b/scripts/test/echo_stdin_to_stdout.sh deleted file mode 100755 index c22cb1f..0000000 --- a/scripts/test/echo_stdin_to_stdout.sh +++ /dev/null @@ -1,3 +0,0 @@ -#/usr/bin/env bash - -while IFS= read; do echo "$REPLY"; done diff --git a/scripts/test/exit_code.sh b/scripts/test/exit_code.sh deleted file mode 100755 index 6411de8..0000000 --- a/scripts/test/exit_code.sh +++ /dev/null @@ -1,3 +0,0 @@ -#!/usr/bin/env bash - -exit "$1" diff --git a/scripts/test/sleep.sh b/scripts/test/sleep.sh deleted file mode 100755 index bfd6524..0000000 --- a/scripts/test/sleep.sh +++ /dev/null @@ -1,3 +0,0 @@ -#!/usr/bin/env bash - -sleep "$1" diff --git a/src/buf.rs b/src/buf.rs index e2de6e8..e0bce79 100644 --- a/src/buf.rs +++ b/src/buf.rs @@ -1,12 +1,12 @@ use std::ops::{Deref, DerefMut}; /// Wraps a string to provide some friendly read and write methods -#[derive(Clone, Debug, PartialEq, Eq, Hash)] +#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)] pub struct StringBuf(String); impl StringBuf { pub fn new() -> Self { - Self(String::new()) + Self::default() } /// Consumes data within the buffer that represent full lines (end with a newline) and returns diff --git a/src/exit.rs b/src/exit.rs index f8967ff..5196d28 100644 --- a/src/exit.rs +++ b/src/exit.rs @@ -41,8 +41,8 @@ pub enum ExitCode { impl ExitCode { /// Convert into numeric exit code - pub fn to_i32(&self) -> i32 { - match *self { + pub fn to_i32(self) -> i32 { + match self { Self::Usage => 64, Self::DataErr => 65, Self::NoInput => 66, diff --git a/src/opt.rs b/src/opt.rs index 3a5a734..23b41ce 100644 --- a/src/opt.rs +++ b/src/opt.rs @@ -197,21 +197,20 @@ pub enum ConvertToIpAddrError { impl BindAddress { /// Converts address into valid IP; in the case of "any", will leverage the /// `use_ipv6` flag to determine if binding should use ipv4 or ipv6 - pub fn to_ip_addr(&self, use_ipv6: bool) -> Result { + pub fn to_ip_addr(self, use_ipv6: bool) -> Result { match self { Self::Ssh => { let ssh_connection = env::var("SSH_CONNECTION")?; let ip_str = ssh_connection .split(' ') - .skip(2) - .next() + .nth(2) .ok_or(ConvertToIpAddrError::MissingSshAddr)?; let ip = ip_str.parse::()?; Ok(ip) } Self::Any if use_ipv6 => Ok(IpAddr::V6(Ipv6Addr::UNSPECIFIED)), Self::Any => Ok(IpAddr::V4(Ipv4Addr::UNSPECIFIED)), - Self::Ip(addr) => Ok(*addr), + Self::Ip(addr) => Ok(addr), } } } diff --git a/src/output.rs b/src/output.rs index a24ed79..45065be 100644 --- a/src/output.rs +++ b/src/output.rs @@ -18,11 +18,10 @@ impl ResponseOut { let payload_cnt = res.payload.len(); Ok(match format { - Format::Json => ResponseOut::StdoutLine(format!( - "{}", + Format::Json => ResponseOut::StdoutLine( serde_json::to_string(&res) - .map_err(|x| io::Error::new(io::ErrorKind::InvalidData, x))? - )), + .map_err(|x| io::Error::new(io::ErrorKind::InvalidData, x))?, + ), // NOTE: For shell, we assume a singular entry in the response's payload Format::Shell if payload_cnt != 1 => { @@ -77,8 +76,7 @@ fn format_shell(data: ResponseData) -> ResponseOut { ResponseOut::StdoutLine(String::from_utf8_lossy(&data).to_string()) } ResponseData::Text { data } => ResponseOut::StdoutLine(data), - ResponseData::DirEntries { entries, .. } => ResponseOut::StdoutLine(format!( - "{}", + ResponseData::DirEntries { entries, .. } => ResponseOut::StdoutLine( entries .into_iter() .map(|entry| { @@ -100,7 +98,7 @@ fn format_shell(data: ResponseData) -> ResponseOut { }) .collect::>() .join("\n"), - )), + ), ResponseData::Exists(exists) => { if exists { ResponseOut::StdoutLine("true".to_string()) @@ -136,14 +134,13 @@ fn format_shell(data: ResponseData) -> ResponseOut { accessed.unwrap_or_default(), modified.unwrap_or_default(), )), - ResponseData::ProcEntries { entries } => ResponseOut::StdoutLine(format!( - "{}", + ResponseData::ProcEntries { entries } => ResponseOut::StdoutLine( entries .into_iter() .map(|entry| format!("{}: {} {}", entry.id, entry.cmd, entry.args.join(" "))) .collect::>() .join("\n"), - )), + ), ResponseData::ProcStart { .. } => ResponseOut::None, ResponseData::ProcStdout { data, .. } => ResponseOut::Stdout(data), ResponseData::ProcStderr { data, .. } => ResponseOut::Stderr(data), diff --git a/src/session.rs b/src/session.rs index 1c3e4e2..a61e2de 100644 --- a/src/session.rs +++ b/src/session.rs @@ -113,7 +113,7 @@ async fn process_outgoing_requests( } else if line == "exit" { debug!("Got exit request, so closing cli session"); stdin_rx.close(); - if let Err(_) = exit_tx.send(()).await { + if exit_tx.send(()).await.is_err() { error!("Failed to close cli session"); } continue; diff --git a/src/subcommand/listen.rs b/src/subcommand/listen.rs index 0222b4c..c8ba818 100644 --- a/src/subcommand/listen.rs +++ b/src/subcommand/listen.rs @@ -38,7 +38,7 @@ pub fn run(cmd: ListenSubcommand, opt: CommonOpt) -> Result<(), Error> { } Ok(Fork::Parent(pid)) => { info!("[distant detached, pid = {}]", pid); - if let Err(_) = fork::close_fd() { + if fork::close_fd().is_err() { return Err(Error::ForkError); } } @@ -83,10 +83,8 @@ async fn run_async(cmd: ListenSubcommand, _opt: CommonOpt, is_forked: bool) -> R ); // For the child, we want to fully disconnect it from pipes, which we do now - if is_forked { - if let Err(_) = fork::close_fd() { - return Err(Error::ForkError); - } + if is_forked && fork::close_fd().is_err() { + return Err(Error::ForkError); } // Let our server run to completion diff --git a/tests/cli/action/dir_read.rs b/tests/cli/action/dir_read.rs index 52bdcdf..0cb3488 100644 --- a/tests/cli/action/dir_read.rs +++ b/tests/cli/action/dir_read.rs @@ -449,7 +449,7 @@ fn should_support_json_including_root_directory_if_specified(mut action_cmd: Com ResponseData::DirEntries { entries: vec![ DirEntry { - path: root_path.to_path_buf(), + path: root_path, file_type: FileType::Dir, depth: 0 }, diff --git a/tests/cli/action/proc_run.rs b/tests/cli/action/proc_run.rs index 4140287..02cf0cd 100644 --- a/tests/cli/action/proc_run.rs +++ b/tests/cli/action/proc_run.rs @@ -1,26 +1,55 @@ -use crate::cli::{ - fixtures::*, - utils::{random_tenant, regex_pred, FAILURE_LINE}, -}; +use crate::cli::{fixtures::*, utils::random_tenant}; use assert_cmd::Command; +use assert_fs::prelude::*; use distant::ExitCode; use distant_core::{ data::{Error, ErrorKind}, Request, RequestData, Response, ResponseData, }; use rstest::*; -use std::path::PathBuf; lazy_static::lazy_static! { - static ref ROOT_DIR: PathBuf = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - static ref SCRIPT_DIR: PathBuf = ROOT_DIR.join("scripts").join("test"); + static ref TEMP_SCRIPT_DIR: assert_fs::TempDir = assert_fs::TempDir::new().unwrap(); + static ref SCRIPT_RUNNER: String = String::from("bash"); - static ref ECHO_ARGS_TO_STDOUT_SH: PathBuf = SCRIPT_DIR.join("echo_args_to_stdout.sh"); - static ref ECHO_ARGS_TO_STDERR_SH: PathBuf = SCRIPT_DIR.join("echo_args_to_stderr.sh"); - static ref ECHO_STDIN_TO_STDOUT_SH: PathBuf = SCRIPT_DIR.join("echo_stdin_to_stdout.sh"); - static ref EXIT_CODE_SH: PathBuf = SCRIPT_DIR.join("exit_code.sh"); + static ref ECHO_ARGS_TO_STDOUT_SH: assert_fs::fixture::ChildPath = { + let script = TEMP_SCRIPT_DIR.child("echo_args_to_stdout.sh"); + script.write_str(indoc::indoc!(r#" + #/usr/bin/env bash + printf "%s" "$@" + "#)).unwrap(); + script + }; - static ref DOES_NOT_EXIST_BIN: PathBuf = SCRIPT_DIR.join("does_not_exist_bin"); + static ref ECHO_ARGS_TO_STDERR_SH: assert_fs::fixture::ChildPath = { + let script = TEMP_SCRIPT_DIR.child("echo_args_to_stderr.sh"); + script.write_str(indoc::indoc!(r#" + #/usr/bin/env bash + printf "%s" "$@" 1>&2 + "#)).unwrap(); + script + }; + + static ref ECHO_STDIN_TO_STDOUT_SH: assert_fs::fixture::ChildPath = { + let script = TEMP_SCRIPT_DIR.child("echo_stdin_to_stdout.sh"); + script.write_str(indoc::indoc!(r#" + #/usr/bin/env bash + while IFS= read; do echo "$REPLY"; done + "#)).unwrap(); + script + }; + + static ref EXIT_CODE_SH: assert_fs::fixture::ChildPath = { + let script = TEMP_SCRIPT_DIR.child("exit_code.sh"); + script.write_str(indoc::indoc!(r#" + #!/usr/bin/env bash + exit "$1" + "#)).unwrap(); + script + }; + + static ref DOES_NOT_EXIST_BIN: assert_fs::fixture::ChildPath = + TEMP_SCRIPT_DIR.child("does_not_exist_bin"); } #[rstest] @@ -28,6 +57,7 @@ fn should_execute_program_and_return_exit_status(mut action_cmd: Command) { // distant action proc-run -- {cmd} [args] action_cmd .args(&["proc-run", "--"]) + .arg(SCRIPT_RUNNER.as_str()) .arg(EXIT_CODE_SH.to_str().unwrap()) .arg("0") .assert() @@ -41,6 +71,7 @@ fn should_capture_and_print_stdout(mut action_cmd: Command) { // distant action proc-run {cmd} [args] action_cmd .args(&["proc-run", "--"]) + .arg(SCRIPT_RUNNER.as_str()) .arg(ECHO_ARGS_TO_STDOUT_SH.to_str().unwrap()) .arg("hello world") .assert() @@ -54,6 +85,7 @@ fn should_capture_and_print_stderr(mut action_cmd: Command) { // distant action proc-run {cmd} [args] action_cmd .args(&["proc-run", "--"]) + .arg(SCRIPT_RUNNER.as_str()) .arg(ECHO_ARGS_TO_STDERR_SH.to_str().unwrap()) .arg("hello world") .assert() @@ -67,6 +99,7 @@ fn should_forward_stdin_to_remote_process(mut action_cmd: Command) { // distant action proc-run {cmd} [args] action_cmd .args(&["proc-run", "--"]) + .arg(SCRIPT_RUNNER.as_str()) .arg(ECHO_STDIN_TO_STDOUT_SH.to_str().unwrap()) .write_stdin("hello world\n") .assert() @@ -80,6 +113,7 @@ fn reflect_the_exit_code_of_the_process(mut action_cmd: Command) { // distant action proc-run {cmd} [args] action_cmd .args(&["proc-run", "--"]) + .arg(SCRIPT_RUNNER.as_str()) .arg(EXIT_CODE_SH.to_str().unwrap()) .arg("99") .assert() @@ -106,8 +140,8 @@ fn should_support_json_to_execute_program_and_return_exit_status(mut action_cmd: id: rand::random(), tenant: random_tenant(), payload: vec![RequestData::ProcRun { - cmd: ECHO_ARGS_TO_STDOUT_SH.to_str().unwrap().to_string(), - args: Vec::new(), + cmd: SCRIPT_RUNNER.to_string(), + args: vec![ECHO_ARGS_TO_STDOUT_SH.to_str().unwrap().to_string()], }], }; diff --git a/tests/cli/fixtures.rs b/tests/cli/fixtures.rs index d895622..51d47ca 100644 --- a/tests/cli/fixtures.rs +++ b/tests/cli/fixtures.rs @@ -5,7 +5,7 @@ use rstest::*; use std::{ffi::OsStr, net::SocketAddr, sync::Arc, thread}; use tokio::{runtime::Runtime, sync::mpsc}; -const LOG_PATH: &'static str = "/tmp/test.distant.server.log"; +const LOG_PATH: &str = "/tmp/test.distant.server.log"; /// Context for some listening distant server pub struct DistantServerCtx {