From 46aa49e22992e81786431948e5c0a29a6cea177b Mon Sep 17 00:00:00 2001 From: Peter White Date: Sun, 12 May 2024 16:57:49 -0600 Subject: [PATCH 1/4] feat(telemetry): support DO_NOT_TRACK telemetry --- Cargo.lock | 41 ++++++++++++++++++ Cargo.toml | 1 + crates/pop-cli/src/main.rs | 2 +- crates/pop-telemetry/Cargo.toml | 5 ++- crates/pop-telemetry/src/lib.rs | 74 ++++++++++++++++++++++++++------- 5 files changed, 105 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5343b72a..e81698e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5454,6 +5454,7 @@ dependencies = [ "reqwest", "serde", "serde_json", + "serial_test", "tempfile", "thiserror", "tokio", @@ -6424,6 +6425,15 @@ dependencies = [ "yap", ] +[[package]] +name = "scc" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76ad2bbb0ae5100a07b7a6f2ed7ab5fd0045551a4c507989b7a620046ea3efdc" +dependencies = [ + "sdd", +] + [[package]] name = "schannel" version = "0.1.23" @@ -6515,6 +6525,12 @@ dependencies = [ "untrusted", ] +[[package]] +name = "sdd" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b84345e4c9bd703274a082fb80caaa99b7612be48dfaa1dd9266577ec412309d" + [[package]] name = "seahash" version = "4.1.0" @@ -6760,6 +6776,31 @@ dependencies = [ "serde", ] +[[package]] +name = "serial_test" +version = "3.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b4b487fe2acf240a021cf57c6b2b4903b1e78ca0ecd862a71b71d2a51fed77d" +dependencies = [ + "futures", + "log", + "once_cell", + "parking_lot", + "scc", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "82fe9db325bcef1fbcde82e078a5cc4efdf787e96b3b9cf45b50b529f2083d67" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.58", +] + [[package]] name = "sha-1" version = "0.9.8" diff --git a/Cargo.toml b/Cargo.toml index f9e4d4b2..0b000f3a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ duct = "0.13" git2 = "0.18" log = "0.4.20" mockito = "1.4.0" +serial_test = "3.1.1" tempfile = "3.8" thiserror = "1.0.58" diff --git a/crates/pop-cli/src/main.rs b/crates/pop-cli/src/main.rs index fbf4d1cd..efaf3dc4 100644 --- a/crates/pop-cli/src/main.rs +++ b/crates/pop-cli/src/main.rs @@ -145,7 +145,7 @@ fn init() -> Result> { env_logger::init(); let maybe_config_path = config_file_path(); - let maybe_tel = maybe_config_path.ok().map(|path| Telemetry::new(path)); + let maybe_tel = maybe_config_path.ok().map(|path| Telemetry::new(&path)); // Handle for await not used here as telemetry should complete before any of the commands do. // Sends a generic ping saying the CLI was used. diff --git a/crates/pop-telemetry/Cargo.toml b/crates/pop-telemetry/Cargo.toml index 55c7a077..12e1bf58 100644 --- a/crates/pop-telemetry/Cargo.toml +++ b/crates/pop-telemetry/Cargo.toml @@ -15,5 +15,6 @@ tokio.workspace = true url.workspace = true [dev-dependencies] -tempfile.workspace = true -mockito.workspace = true \ No newline at end of file +mockito.workspace = true +serial_test.workspace = true +tempfile.workspace = true \ No newline at end of file diff --git a/crates/pop-telemetry/src/lib.rs b/crates/pop-telemetry/src/lib.rs index aa86d342..179177bc 100644 --- a/crates/pop-telemetry/src/lib.rs +++ b/crates/pop-telemetry/src/lib.rs @@ -3,6 +3,7 @@ use reqwest::Client; use serde::{de::DeserializeOwned, Deserialize, Serialize}; use serde_json::{json, Value}; use std::{ + env, fs::{create_dir_all, File}, io, io::{Read, Write}, @@ -46,7 +47,7 @@ impl Telemetry { /// /// parameters: /// `config_path`: the path to the configuration file (used for opt-out checks) - pub fn new(config_path: PathBuf) -> Self { + pub fn new(config_path: &PathBuf) -> Self { Self::init(ENDPOINT.to_string(), config_path) } @@ -55,8 +56,8 @@ impl Telemetry { /// parameters: /// `endpoint`: the API endpoint that telemetry will call /// `config_path`: the path to the configuration file (used for opt-out checks) - fn init(endpoint: String, config_path: PathBuf) -> Self { - let opt_out = Self::is_opt_out_from_config(&config_path); + fn init(endpoint: String, config_path: &PathBuf) -> Self { + let opt_out = Self::is_opt_out(&config_path); Telemetry { endpoint, opt_out, client: Client::new() } } @@ -73,6 +74,17 @@ impl Telemetry { !config.opt_out.version.is_empty() } + fn is_opt_out_from_env() -> bool { + env::var("DO_NOT_TRACK").is_ok() + } + + /// Check if the user has opted out of telemetry through two methods: + /// 1. Check environment variable DO_NOT_TRACK. If not set check... + /// 2. Configuration file + fn is_opt_out(config_file_path: &PathBuf) -> bool { + Self::is_opt_out_from_env() || Self::is_opt_out_from_config(config_file_path) + } + /// Send JSON payload to saved api endpoint. /// Returns error and will not send anything if opt-out is true. /// Returns error from reqwest if the sending fails. @@ -198,6 +210,7 @@ mod tests { use super::*; use mockito::{Matcher, Mock, Server}; use serde_json::json; + use serial_test::serial; use tempfile::TempDir; fn create_temp_config(temp_dir: &TempDir) -> PathBuf { @@ -229,10 +242,11 @@ mod tests { } #[tokio::test] + #[serial] async fn new_telemetry_works() { let _ = env_logger::try_init(); // assert that invalid config file results in a false opt_in (hence disabling telemetry) - assert!(!Telemetry::init("".to_string(), PathBuf::new()).opt_out); + assert!(!Telemetry::init("".to_string(), &PathBuf::new()).opt_out); // Mock config file path function to return a temporary path let temp_dir = TempDir::new().unwrap(); @@ -240,7 +254,7 @@ mod tests { let _: Config = read_json_file(&config_path).unwrap(); - let tel = Telemetry::init("127.0.0.1".to_string(), config_path); + let tel = Telemetry::init("127.0.0.1".to_string(), &config_path); let expected_telemetry = Telemetry { endpoint: "127.0.0.1".to_string(), opt_out: true, @@ -252,9 +266,10 @@ mod tests { } #[tokio::test] + #[serial] async fn test_record_cli_used() { let _ = env_logger::try_init(); - let mut mock_server = mockito::Server::new_async().await; + let mut mock_server = Server::new_async().await; let mut endpoint = mock_server.url(); endpoint.push_str("/api/send"); @@ -267,16 +282,17 @@ mod tests { let mock = default_mock(&mut mock_server, expected_payload).await; - let tel = Telemetry::init(endpoint.clone(), config_path); + let tel = Telemetry::init(endpoint.clone(), &config_path); assert!(record_cli_used(tel).await.is_ok()); mock.assert_async().await; } #[tokio::test] + #[serial] async fn test_record_cli_command() { let _ = env_logger::try_init(); - let mut mock_server = mockito::Server::new_async().await; + let mut mock_server = Server::new_async().await; let mut endpoint = mock_server.url(); endpoint.push_str("/api/send"); @@ -290,16 +306,17 @@ mod tests { let mock = default_mock(&mut mock_server, expected_payload).await; - let tel = Telemetry::init(endpoint.clone(), config_path); + let tel = Telemetry::init(endpoint.clone(), &config_path); assert!(record_cli_command(tel, "new", json!("parachain")).await.is_ok()); mock.assert_async().await; } #[tokio::test] - async fn opt_out_fails() { + #[serial] + async fn opt_out_with_config_fails() { let _ = env_logger::try_init(); - let mut mock_server = mockito::Server::new_async().await; + let mut mock_server = Server::new_async().await; let endpoint = mock_server.url(); @@ -310,17 +327,16 @@ mod tests { let mock = mock_server.mock("POST", "/").create_async().await; let mock = mock.expect_at_most(0); - let mut tel = Telemetry::init(endpoint.clone(), config_path); + let mut tel = Telemetry::init(endpoint.clone(), &config_path); - assert!(matches!(tel.send_json(json!("foo")).await, Err(TelemetryError::OptedOut))); + assert!(matches!(tel.send_json(Value::Null).await, Err(TelemetryError::OptedOut))); assert!(matches!(record_cli_used(tel.clone()).await, Err(TelemetryError::OptedOut))); assert!(matches!( - record_cli_command(tel.clone(), "foo", json!("bar")).await, + record_cli_command(tel.clone(), "foo", Value::Null).await, Err(TelemetryError::OptedOut) )); mock.assert_async().await; - // test it's set to true and works tel.opt_out = false; let mock = mock.expect_at_most(3); assert!(tel.send_json(json!("foo")).await.is_ok(),); @@ -328,4 +344,32 @@ mod tests { assert!(record_cli_command(tel, "foo", json!("bar")).await.is_ok()); mock.assert_async().await; } + + #[tokio::test] + #[serial] + async fn opt_out_with_env_fails() { + let _ = env_logger::try_init(); + let mut mock_server = Server::new_async().await; + + let endpoint = mock_server.url(); + + let mock = mock_server.mock("POST", "/").create_async().await; + let mock = mock.expect_at_most(0); + + env::set_var("DO_NOT_TRACK", "true"); + + // config file does not exist, so env variable should be used + let mut tel = Telemetry::init(endpoint.clone(), &PathBuf::new()); + + assert!(matches!(tel.send_json(Value::Null).await, Err(TelemetryError::OptedOut))); + assert!(matches!(record_cli_used(tel.clone()).await, Err(TelemetryError::OptedOut))); + assert!(matches!( + record_cli_command(tel.clone(), "foo", Value::Null).await, + Err(TelemetryError::OptedOut) + )); + mock.assert_async().await; + + // IMPORTANT: needs to remove the env variable for the other tests + env::remove_var("DO_NOT_TRACK"); + } } From 93ee57c21290ba70ad7386caf1b1bcd2a8a7cc71 Mon Sep 17 00:00:00 2001 From: Peter White Date: Sun, 12 May 2024 21:52:28 -0600 Subject: [PATCH 2/4] test(telemetry): improve unit tests --- Cargo.lock | 41 ------------------- Cargo.toml | 1 - crates/pop-telemetry/Cargo.toml | 1 - crates/pop-telemetry/src/lib.rs | 70 +++++++++++---------------------- 4 files changed, 23 insertions(+), 90 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e81698e9..5343b72a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5454,7 +5454,6 @@ dependencies = [ "reqwest", "serde", "serde_json", - "serial_test", "tempfile", "thiserror", "tokio", @@ -6425,15 +6424,6 @@ dependencies = [ "yap", ] -[[package]] -name = "scc" -version = "2.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "76ad2bbb0ae5100a07b7a6f2ed7ab5fd0045551a4c507989b7a620046ea3efdc" -dependencies = [ - "sdd", -] - [[package]] name = "schannel" version = "0.1.23" @@ -6525,12 +6515,6 @@ dependencies = [ "untrusted", ] -[[package]] -name = "sdd" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b84345e4c9bd703274a082fb80caaa99b7612be48dfaa1dd9266577ec412309d" - [[package]] name = "seahash" version = "4.1.0" @@ -6776,31 +6760,6 @@ dependencies = [ "serde", ] -[[package]] -name = "serial_test" -version = "3.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4b4b487fe2acf240a021cf57c6b2b4903b1e78ca0ecd862a71b71d2a51fed77d" -dependencies = [ - "futures", - "log", - "once_cell", - "parking_lot", - "scc", - "serial_test_derive", -] - -[[package]] -name = "serial_test_derive" -version = "3.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "82fe9db325bcef1fbcde82e078a5cc4efdf787e96b3b9cf45b50b529f2083d67" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.58", -] - [[package]] name = "sha-1" version = "0.9.8" diff --git a/Cargo.toml b/Cargo.toml index 0b000f3a..f9e4d4b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,6 @@ duct = "0.13" git2 = "0.18" log = "0.4.20" mockito = "1.4.0" -serial_test = "3.1.1" tempfile = "3.8" thiserror = "1.0.58" diff --git a/crates/pop-telemetry/Cargo.toml b/crates/pop-telemetry/Cargo.toml index 12e1bf58..8cf4bc39 100644 --- a/crates/pop-telemetry/Cargo.toml +++ b/crates/pop-telemetry/Cargo.toml @@ -16,5 +16,4 @@ url.workspace = true [dev-dependencies] mockito.workspace = true -serial_test.workspace = true tempfile.workspace = true \ No newline at end of file diff --git a/crates/pop-telemetry/src/lib.rs b/crates/pop-telemetry/src/lib.rs index 179177bc..6f7909c4 100644 --- a/crates/pop-telemetry/src/lib.rs +++ b/crates/pop-telemetry/src/lib.rs @@ -71,11 +71,12 @@ impl Telemetry { }, }; + // if the version is empty, then the user has not opted out !config.opt_out.version.is_empty() } fn is_opt_out_from_env() -> bool { - env::var("DO_NOT_TRACK").is_ok() + env::var("DO_NOT_TRACK").unwrap_or("false".to_string()) == "true" } /// Check if the user has opted out of telemetry through two methods: @@ -210,7 +211,6 @@ mod tests { use super::*; use mockito::{Matcher, Mock, Server}; use serde_json::json; - use serial_test::serial; use tempfile::TempDir; fn create_temp_config(temp_dir: &TempDir) -> PathBuf { @@ -229,6 +229,7 @@ mod tests { .create_async() .await } + #[tokio::test] async fn write_config_opt_out_works() { // Mock config file path function to return a temporary path @@ -242,14 +243,22 @@ mod tests { } #[tokio::test] - #[serial] async fn new_telemetry_works() { let _ = env_logger::try_init(); - // assert that invalid config file results in a false opt_in (hence disabling telemetry) + // assert that no config file and false DO_NOT_TRACK sets opt-out to false + env::set_var("DO_NOT_TRACK", "false"); + assert!(!Telemetry::init("".to_string(), &PathBuf::new()).opt_out); + // assert if DO_NOT_TRACK env var is set to false, opt-out is false assert!(!Telemetry::init("".to_string(), &PathBuf::new()).opt_out); + // check that if DO_NOT_TRACK env var is set, opt-out is true + env::set_var("DO_NOT_TRACK", "true"); + assert!(Telemetry::init("".to_string(), &PathBuf::new()).opt_out); + env::remove_var("DO_NOT_TRACK"); + // Mock config file path function to return a temporary path let temp_dir = TempDir::new().unwrap(); + // write a config file with opt-out set let config_path = create_temp_config(&temp_dir); let _: Config = read_json_file(&config_path).unwrap(); @@ -263,10 +272,17 @@ mod tests { assert_eq!(tel.endpoint, expected_telemetry.endpoint); assert_eq!(tel.opt_out, expected_telemetry.opt_out); + + let tel = Telemetry::new(&config_path); + + let expected_telemetry = + Telemetry { endpoint: ENDPOINT.to_string(), opt_out: true, client: Default::default() }; + + assert_eq!(tel.endpoint, expected_telemetry.endpoint); + assert_eq!(tel.opt_out, expected_telemetry.opt_out); } #[tokio::test] - #[serial] async fn test_record_cli_used() { let _ = env_logger::try_init(); let mut mock_server = Server::new_async().await; @@ -289,7 +305,6 @@ mod tests { } #[tokio::test] - #[serial] async fn test_record_cli_command() { let _ = env_logger::try_init(); let mut mock_server = Server::new_async().await; @@ -313,53 +328,17 @@ mod tests { } #[tokio::test] - #[serial] - async fn opt_out_with_config_fails() { + async fn opt_out_set_fails() { let _ = env_logger::try_init(); let mut mock_server = Server::new_async().await; let endpoint = mock_server.url(); - // Mock config file path function to return a temporary path - let temp_dir = TempDir::new().unwrap(); - let config_path = create_temp_config(&temp_dir); - let mock = mock_server.mock("POST", "/").create_async().await; let mock = mock.expect_at_most(0); - let mut tel = Telemetry::init(endpoint.clone(), &config_path); - - assert!(matches!(tel.send_json(Value::Null).await, Err(TelemetryError::OptedOut))); - assert!(matches!(record_cli_used(tel.clone()).await, Err(TelemetryError::OptedOut))); - assert!(matches!( - record_cli_command(tel.clone(), "foo", Value::Null).await, - Err(TelemetryError::OptedOut) - )); - mock.assert_async().await; - - tel.opt_out = false; - let mock = mock.expect_at_most(3); - assert!(tel.send_json(json!("foo")).await.is_ok(),); - assert!(record_cli_used(tel.clone()).await.is_ok()); - assert!(record_cli_command(tel, "foo", json!("bar")).await.is_ok()); - mock.assert_async().await; - } - - #[tokio::test] - #[serial] - async fn opt_out_with_env_fails() { - let _ = env_logger::try_init(); - let mut mock_server = Server::new_async().await; - - let endpoint = mock_server.url(); - - let mock = mock_server.mock("POST", "/").create_async().await; - let mock = mock.expect_at_most(0); - - env::set_var("DO_NOT_TRACK", "true"); - - // config file does not exist, so env variable should be used let mut tel = Telemetry::init(endpoint.clone(), &PathBuf::new()); + tel.opt_out = true; assert!(matches!(tel.send_json(Value::Null).await, Err(TelemetryError::OptedOut))); assert!(matches!(record_cli_used(tel.clone()).await, Err(TelemetryError::OptedOut))); @@ -368,8 +347,5 @@ mod tests { Err(TelemetryError::OptedOut) )); mock.assert_async().await; - - // IMPORTANT: needs to remove the env variable for the other tests - env::remove_var("DO_NOT_TRACK"); } } From 4dda8461a654bf1b79723798b18fca73da192760 Mon Sep 17 00:00:00 2001 From: Peter White Date: Sun, 12 May 2024 23:54:31 -0600 Subject: [PATCH 3/4] feat(telemetry): add support for CI env variable --- crates/pop-telemetry/README.md | 11 +++++++-- crates/pop-telemetry/src/lib.rs | 40 ++++++++++++++++++++++++--------- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/crates/pop-telemetry/README.md b/crates/pop-telemetry/README.md index e8500e3f..d8d4dc95 100644 --- a/crates/pop-telemetry/README.md +++ b/crates/pop-telemetry/README.md @@ -54,8 +54,15 @@ We take privacy seriously and are committed to protecting the anonymity of our u ## How to Opt-Out -If you prefer not to participate in anonymous usage metrics collection, you can completely disable telemetry by -installing Pop CLI with it disabled. +If you prefer not to participate in anonymous usage metrics collection, there are a +few ways you can opt out. We support the [DO_NOT_TRACK](https://consoledonottrack.com/) and CI environment variable +standards. + +1. Set the `DO_NOT_TRACK` environment variable to `true` or `1`: +2. Set the `CI` environment variable to `true` or `1`: +3. Completely disable telemetry + +Install Pop CLI with telemetry compiled out ```bash cargo install --locked --no-default-features --features contract,parachain --git "https://github.com/r0gue-io/pop-cli" diff --git a/crates/pop-telemetry/src/lib.rs b/crates/pop-telemetry/src/lib.rs index 6f7909c4..bdefdcb7 100644 --- a/crates/pop-telemetry/src/lib.rs +++ b/crates/pop-telemetry/src/lib.rs @@ -75,8 +75,13 @@ impl Telemetry { !config.opt_out.version.is_empty() } + // Checks two env variables, CI & DO_NOT_TRACK. If either are set to true, disable telemetry fn is_opt_out_from_env() -> bool { - env::var("DO_NOT_TRACK").unwrap_or("false".to_string()) == "true" + // CI first as it is more likely to be set + env::var("CI").unwrap_or_default() == "true" + || env::var("CI").unwrap_or_default() == "1" + || env::var("DO_NOT_TRACK").unwrap_or_default() == "true" + || env::var("DO_NOT_TRACK").unwrap_or_default() == "1" } /// Check if the user has opted out of telemetry through two methods: @@ -245,16 +250,6 @@ mod tests { #[tokio::test] async fn new_telemetry_works() { let _ = env_logger::try_init(); - // assert that no config file and false DO_NOT_TRACK sets opt-out to false - env::set_var("DO_NOT_TRACK", "false"); - assert!(!Telemetry::init("".to_string(), &PathBuf::new()).opt_out); - // assert if DO_NOT_TRACK env var is set to false, opt-out is false - assert!(!Telemetry::init("".to_string(), &PathBuf::new()).opt_out); - - // check that if DO_NOT_TRACK env var is set, opt-out is true - env::set_var("DO_NOT_TRACK", "true"); - assert!(Telemetry::init("".to_string(), &PathBuf::new()).opt_out); - env::remove_var("DO_NOT_TRACK"); // Mock config file path function to return a temporary path let temp_dir = TempDir::new().unwrap(); @@ -282,6 +277,29 @@ mod tests { assert_eq!(tel.opt_out, expected_telemetry.opt_out); } + #[test] + #[ignore] + /// ignore by default as to not mess up env variables for + /// remaining tests when running in CI + fn new_telemetry_env_vars_works() { + let _ = env_logger::try_init(); + + // assert that no config file, and env vars not existing sets opt-out to false + env::remove_var("DO_NOT_TRACK"); + env::set_var("CI", "false"); + assert!(!Telemetry::init("".to_string(), &PathBuf::new()).opt_out); + + // assert that if DO_NOT_TRACK env var is set, opt-out is true + env::set_var("DO_NOT_TRACK", "true"); + assert!(Telemetry::init("".to_string(), &PathBuf::new()).opt_out); + env::remove_var("DO_NOT_TRACK"); + + // assert that if CI env var is set, opt-out is true + env::set_var("CI", "true"); + assert!(Telemetry::init("".to_string(), &PathBuf::new()).opt_out); + env::remove_var("CI"); + } + #[tokio::test] async fn test_record_cli_used() { let _ = env_logger::try_init(); From f2d42579143e814c3a13b1986a14bf17a943aab7 Mon Sep 17 00:00:00 2001 From: Frank Bell <60948618+evilrobot-01@users.noreply.github.com> Date: Mon, 13 May 2024 15:09:14 +0100 Subject: [PATCH 4/4] test: fix test failures (#163) * test: propagate errors * test: override telemetry opt-out when mocked * docs: adjust formatting * refactor: dry * test: remove ignore attribute --- crates/pop-telemetry/README.md | 14 +++++------ crates/pop-telemetry/src/lib.rs | 42 +++++++++++++++++---------------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/crates/pop-telemetry/README.md b/crates/pop-telemetry/README.md index d8d4dc95..69b78b35 100644 --- a/crates/pop-telemetry/README.md +++ b/crates/pop-telemetry/README.md @@ -58,15 +58,13 @@ If you prefer not to participate in anonymous usage metrics collection, there ar few ways you can opt out. We support the [DO_NOT_TRACK](https://consoledonottrack.com/) and CI environment variable standards. -1. Set the `DO_NOT_TRACK` environment variable to `true` or `1`: -2. Set the `CI` environment variable to `true` or `1`: -3. Completely disable telemetry +1. Set the `DO_NOT_TRACK` environment variable to `true` or `1` +2. Set the `CI` environment variable to `true` or `1` +3. Completely disable telemetry, by installing with telemetry compiled out: -Install Pop CLI with telemetry compiled out - -```bash -cargo install --locked --no-default-features --features contract,parachain --git "https://github.com/r0gue-io/pop-cli" -``` + ```bash + cargo install --locked --no-default-features --features contract,parachain --git "https://github.com/r0gue-io/pop-cli" + ``` ## Questions or Concerns? diff --git a/crates/pop-telemetry/src/lib.rs b/crates/pop-telemetry/src/lib.rs index bdefdcb7..1c66f3a4 100644 --- a/crates/pop-telemetry/src/lib.rs +++ b/crates/pop-telemetry/src/lib.rs @@ -78,10 +78,9 @@ impl Telemetry { // Checks two env variables, CI & DO_NOT_TRACK. If either are set to true, disable telemetry fn is_opt_out_from_env() -> bool { // CI first as it is more likely to be set - env::var("CI").unwrap_or_default() == "true" - || env::var("CI").unwrap_or_default() == "1" - || env::var("DO_NOT_TRACK").unwrap_or_default() == "true" - || env::var("DO_NOT_TRACK").unwrap_or_default() == "1" + let ci = env::var("CI").unwrap_or_default(); + let do_not_track = env::var("DO_NOT_TRACK").unwrap_or_default(); + ci == "true" || ci == "1" || do_not_track == "true" || do_not_track == "1" } /// Check if the user has opted out of telemetry through two methods: @@ -218,10 +217,10 @@ mod tests { use serde_json::json; use tempfile::TempDir; - fn create_temp_config(temp_dir: &TempDir) -> PathBuf { + fn create_temp_config(temp_dir: &TempDir) -> Result { let config_path = temp_dir.path().join("config.json"); - assert!(write_config_opt_out(&config_path).is_ok()); - config_path + write_config_opt_out(&config_path)?; + Ok(config_path) } async fn default_mock(mock_server: &mut Server, payload: String) -> Mock { mock_server @@ -236,25 +235,26 @@ mod tests { } #[tokio::test] - async fn write_config_opt_out_works() { + async fn write_config_opt_out_works() -> Result<()> { // Mock config file path function to return a temporary path let temp_dir = TempDir::new().unwrap(); - let config_path = create_temp_config(&temp_dir); + let config_path = create_temp_config(&temp_dir)?; let actual_config: Config = read_json_file(&config_path).unwrap(); let expected_config = Config { opt_out: OptOut { version: CARGO_PKG_VERSION.to_string() } }; assert_eq!(actual_config, expected_config); + Ok(()) } #[tokio::test] - async fn new_telemetry_works() { + async fn new_telemetry_works() -> Result<()> { let _ = env_logger::try_init(); // Mock config file path function to return a temporary path let temp_dir = TempDir::new().unwrap(); // write a config file with opt-out set - let config_path = create_temp_config(&temp_dir); + let config_path = create_temp_config(&temp_dir)?; let _: Config = read_json_file(&config_path).unwrap(); @@ -275,12 +275,10 @@ mod tests { assert_eq!(tel.endpoint, expected_telemetry.endpoint); assert_eq!(tel.opt_out, expected_telemetry.opt_out); + Ok(()) } #[test] - #[ignore] - /// ignore by default as to not mess up env variables for - /// remaining tests when running in CI fn new_telemetry_env_vars_works() { let _ = env_logger::try_init(); @@ -301,7 +299,7 @@ mod tests { } #[tokio::test] - async fn test_record_cli_used() { + async fn test_record_cli_used() -> Result<()> { let _ = env_logger::try_init(); let mut mock_server = Server::new_async().await; @@ -316,14 +314,16 @@ mod tests { let mock = default_mock(&mut mock_server, expected_payload).await; - let tel = Telemetry::init(endpoint.clone(), &config_path); + let mut tel = Telemetry::init(endpoint.clone(), &config_path); + tel.opt_out = false; // override as endpoint is mocked - assert!(record_cli_used(tel).await.is_ok()); + record_cli_used(tel).await?; mock.assert_async().await; + Ok(()) } #[tokio::test] - async fn test_record_cli_command() { + async fn test_record_cli_command() -> Result<()> { let _ = env_logger::try_init(); let mut mock_server = Server::new_async().await; @@ -339,10 +339,12 @@ mod tests { let mock = default_mock(&mut mock_server, expected_payload).await; - let tel = Telemetry::init(endpoint.clone(), &config_path); + let mut tel = Telemetry::init(endpoint.clone(), &config_path); + tel.opt_out = false; // override as endpoint is mocked - assert!(record_cli_command(tel, "new", json!("parachain")).await.is_ok()); + record_cli_command(tel, "new", json!("parachain")).await?; mock.assert_async().await; + Ok(()) } #[tokio::test]