From cbb876c87fc1a24ece4041ddc2187c1000ad04ed Mon Sep 17 00:00:00 2001 From: Jeremy Andrews Date: Mon, 21 Mar 2022 07:01:44 +0100 Subject: [PATCH 1/3] clippy fixes from latest rust verison --- src/config.rs | 2 +- src/goose.rs | 2 +- src/metrics.rs | 40 ++++++++++++++++++---------------------- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/config.rs b/src/config.rs index 7678c862..9c0d6f40 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1742,7 +1742,7 @@ impl GooseConfiguration { // Use GooseDefault if not already set and not Worker. GooseValue { value: defaults.expect_workers, - filter: !self.expect_workers.is_some() && self.worker, + filter: self.expect_workers.is_none() && self.worker, message: "expect_workers", }, ]); diff --git a/src/goose.rs b/src/goose.rs index 503a9fd7..fbcf4b88 100644 --- a/src/goose.rs +++ b/src/goose.rs @@ -1515,7 +1515,7 @@ impl GooseUser { let built_request = request_builder.build()?; // Get a string version of request path for logging. - let path = match Url::parse(&built_request.url().to_string()) { + let path = match Url::parse(built_request.url().as_ref()) { Ok(u) => u.path().to_string(), Err(e) => { error!("failed to parse url: {}", e); diff --git a/src/metrics.rs b/src/metrics.rs index 0e43d3d8..9b144588 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -2122,12 +2122,11 @@ impl Serialize for GooseMetrics { let mut s = serializer.serialize_struct("GooseMetrics", 10)?; s.serialize_field("hash", &self.hash)?; // Convert started field to a unix timestamp. - let timestamp; - if let Some(started) = self.started { - timestamp = started.timestamp(); + let timestamp = if let Some(started) = self.started { + started.timestamp() } else { - timestamp = 0; - } + 0 + }; s.serialize_field("started", ×tamp)?; s.serialize_field("duration", &self.duration)?; s.serialize_field("users", &self.users)?; @@ -2939,24 +2938,22 @@ impl GooseAttack { } // Only build the tasks template if --no-task-metrics isn't enabled. - let errors_template: String; - if !self.metrics.errors.is_empty() { + let errors_template: String = if !self.metrics.errors.is_empty() { let mut error_rows = Vec::new(); for error in self.metrics.errors.values() { error_rows.push(report::error_row(error)); } - errors_template = report::errors_template( + report::errors_template( &error_rows.join("\n"), self.graph_data.get_errors_per_second_graph(), - ); + ) } else { - errors_template = "".to_string(); - } + "".to_string() + }; // Only build the status_code template if --status-codes is enabled. - let status_code_template: String; - if self.configuration.status_codes { + let status_code_template: String = if self.configuration.status_codes { let mut status_code_metrics = Vec::new(); let mut aggregated_status_code_counts: HashMap = HashMap::new(); for (request_key, request) in self.metrics.requests.iter().sorted() { @@ -3000,12 +2997,11 @@ impl GooseAttack { } // Compile the status_code metrics template. - status_code_template = - report::status_code_metrics_template(&status_code_rows.join("\n")); + report::status_code_metrics_template(&status_code_rows.join("\n")) } else { // If --status-codes is not enabled, return an empty template. - status_code_template = "".to_string(); - } + "".to_string() + }; // Compile the report template. let report = report::build_report( @@ -3170,13 +3166,13 @@ pub(crate) fn prepare_status_codes( ); } if let Some(aggregate_status_code_counts) = aggregate_counts.as_mut() { - let new_count; - if let Some(existing_status_code_count) = aggregate_status_code_counts.get(status_code) + let new_count = if let Some(existing_status_code_count) = + aggregate_status_code_counts.get(status_code) { - new_count = *existing_status_code_count + *count; + *existing_status_code_count + *count } else { - new_count = *count; - } + *count + }; aggregate_status_code_counts.insert(*status_code, new_count); } } From 19af3b3dabd392dc18e287339a021dd800793f0b Mon Sep 17 00:00:00 2001 From: Jeremy Andrews Date: Mon, 21 Mar 2022 08:13:33 +0100 Subject: [PATCH 2/3] gaggle clippy fixes --- src/manager.rs | 45 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/manager.rs b/src/manager.rs index a0af5836..36564cc3 100644 --- a/src/manager.rs +++ b/src/manager.rs @@ -127,17 +127,16 @@ fn merge_requests_from_worker( // Only accrue overhead of merging status_code_counts if we're going to display the results if status_codes { for (status_code, count) in &user_request.status_code_counts { - let new_count; // Add user count into global count - if let Some(existing_status_code_count) = + let new_count = if let Some(existing_status_code_count) = merged_request.status_code_counts.get(status_code) { - new_count = *existing_status_code_count + *count; + *existing_status_code_count + *count } // No global count exists yet, so start with user count else { - new_count = *count; - } + *count + }; merged_request .status_code_counts .insert(*status_code, new_count); @@ -193,17 +192,17 @@ fn merge_request_metrics(goose_attack: &mut GooseAttack, requests: GooseRequestM debug!("requests metrics received: {:?}", requests.len()); for (request_key, request) in requests { trace!("request_key: {}", request_key); - let merged_request; - if let Some(parent_request) = goose_attack.metrics.requests.get(&request_key) { - merged_request = merge_requests_from_worker( - parent_request, - &request, - goose_attack.configuration.status_codes, - ); - } else { - // First time seeing this request, simply insert it. - merged_request = request.clone(); - } + let merged_request = + if let Some(parent_request) = goose_attack.metrics.requests.get(&request_key) { + merge_requests_from_worker( + parent_request, + &request, + goose_attack.configuration.status_codes, + ) + } else { + // First time seeing this request, simply insert it. + request.clone() + }; goose_attack .metrics .requests @@ -231,13 +230,13 @@ fn merge_error_metrics(goose_attack: &mut GooseAttack, errors: GooseErrorMetrics debug!("errors received: {:?}", errors.len()); for (error_key, error) in errors { trace!("error_key: {}", error_key); - let merged_error; - if let Some(parent_error) = goose_attack.metrics.errors.get(&error_key) { - merged_error = merge_errors_from_worker(parent_error, &error); - } else { - // First time seeing this error, simply insert it. - merged_error = error.clone(); - } + let merged_error = + if let Some(parent_error) = goose_attack.metrics.errors.get(&error_key) { + merge_errors_from_worker(parent_error, &error) + } else { + // First time seeing this error, simply insert it. + error.clone() + }; goose_attack .metrics .errors From 64708eec96d3271d36a2afceab807662ddad15d2 Mon Sep 17 00:00:00 2001 From: Jeremy Andrews Date: Mon, 21 Mar 2022 08:38:18 +0100 Subject: [PATCH 3/3] clippy fixes to tests --- src/util.rs | 12 ++++++------ tests/defaults.rs | 8 ++++---- tests/scheduler.rs | 46 +++++++++++++++++++++------------------------- tests/sequence.rs | 42 +++++++++++++++++++----------------------- 4 files changed, 50 insertions(+), 58 deletions(-) diff --git a/src/util.rs b/src/util.rs index 081c4797..99dd4bca 100644 --- a/src/util.rs +++ b/src/util.rs @@ -598,17 +598,17 @@ mod tests { #[test] fn valid_host() { assert!(is_valid_host("http://example.com").is_ok()); - assert!(!is_valid_host("example.com").is_ok()); + assert!(is_valid_host("example.com").is_err()); assert!(is_valid_host("http://example.com/").is_ok()); - assert!(!is_valid_host("example.com/").is_ok()); + assert!(is_valid_host("example.com/").is_err()); assert!(is_valid_host("https://www.example.com/and/with/path").is_ok()); - assert!(!is_valid_host("www.example.com/and/with/path").is_ok()); + assert!(is_valid_host("www.example.com/and/with/path").is_err()); assert!(is_valid_host("foo://example.com").is_ok()); assert!(is_valid_host("file:///path/to/file").is_ok()); - assert!(!is_valid_host("/path/to/file").is_ok()); - assert!(!is_valid_host("http://").is_ok()); + assert!(is_valid_host("/path/to/file").is_err()); + assert!(is_valid_host("http://").is_err()); assert!(is_valid_host("http://foo").is_ok()); assert!(is_valid_host("http:///example.com").is_ok()); - assert!(!is_valid_host("http:// example.com").is_ok()); + assert!(is_valid_host("http:// example.com").is_err()); } } diff --git a/tests/defaults.rs b/tests/defaults.rs index a98affe7..37765624 100644 --- a/tests/defaults.rs +++ b/tests/defaults.rs @@ -361,7 +361,7 @@ async fn test_no_defaults() { "--users", &USERS.to_string(), "--hatch-rate", - &HATCH_RATE.to_string(), + HATCH_RATE, "--run-time", &RUN_TIME.to_string(), "--request-log", @@ -444,7 +444,7 @@ async fn test_no_defaults_gaggle() { vec![ "--worker", "--manager-host", - &HOST.to_string(), + HOST, "--manager-port", &PORT.to_string(), "--request-log", @@ -477,13 +477,13 @@ async fn test_no_defaults_gaggle() { "--expect-workers", &EXPECT_WORKERS.to_string(), "--manager-bind-host", - &HOST.to_string(), + HOST, "--manager-bind-port", &PORT.to_string(), "--users", &USERS.to_string(), "--hatch-rate", - &HATCH_RATE.to_string(), + HATCH_RATE, "--run-time", &RUN_TIME.to_string(), "--no-reset-metrics", diff --git a/tests/scheduler.rs b/tests/scheduler.rs index 2e1d4113..d7ef62ad 100644 --- a/tests/scheduler.rs +++ b/tests/scheduler.rs @@ -262,32 +262,31 @@ async fn run_standalone_test(test_type: &TestType, scheduler: &GooseScheduler) { // Build common configuration. let configuration = common_build_configuration(&server, None, None); - let goose_attack; - match test_type { + let goose_attack = match test_type { TestType::TaskSets => { // Get the tasksets, start and stop tasks to build a load test. let (taskset1, taskset2, start_task, stop_task) = get_tasksets(); // Set up the common base configuration. - goose_attack = crate::GooseAttack::initialize_with_config(configuration) + crate::GooseAttack::initialize_with_config(configuration) .unwrap() .register_taskset(taskset1) .register_taskset(taskset2) .test_start(start_task) .test_stop(stop_task) - .set_scheduler(scheduler.clone()); + .set_scheduler(scheduler.clone()) } TestType::Tasks => { // Get the taskset, start and stop tasks to build a load test. let (taskset1, start_task, stop_task) = get_tasks(); // Set up the common base configuration. - goose_attack = crate::GooseAttack::initialize_with_config(configuration) + crate::GooseAttack::initialize_with_config(configuration) .unwrap() .register_taskset(taskset1) .test_start(start_task) .test_stop(stop_task) - .set_scheduler(scheduler.clone()); + .set_scheduler(scheduler.clone()) } - } + }; // Run the Goose Attack. common::run_load_test(goose_attack, None).await; @@ -339,34 +338,31 @@ async fn run_gaggle_test(test_type: &TestType, scheduler: &GooseScheduler) { // Build Manager configuration. let manager_configuration = common_build_configuration(&server, None, Some(EXPECT_WORKERS)); - let manager_goose_attack; - match test_type { + let manager_goose_attack = match test_type { TestType::TaskSets => { // Get the tasksets, start and stop tasks to build a load test. let (taskset1, taskset2, start_task, stop_task) = get_tasksets(); // Build the load test for the Manager. - manager_goose_attack = - crate::GooseAttack::initialize_with_config(manager_configuration) - .unwrap() - .register_taskset(taskset1) - .register_taskset(taskset2) - .test_start(start_task) - .test_stop(stop_task) - .set_scheduler(scheduler.clone()); + crate::GooseAttack::initialize_with_config(manager_configuration) + .unwrap() + .register_taskset(taskset1) + .register_taskset(taskset2) + .test_start(start_task) + .test_stop(stop_task) + .set_scheduler(scheduler.clone()) } TestType::Tasks => { // Get the taskset, start and stop tasks to build a load test. let (taskset1, start_task, stop_task) = get_tasks(); // Build the load test for the Manager. - manager_goose_attack = - crate::GooseAttack::initialize_with_config(manager_configuration) - .unwrap() - .register_taskset(taskset1) - .test_start(start_task) - .test_stop(stop_task) - .set_scheduler(scheduler.clone()); + crate::GooseAttack::initialize_with_config(manager_configuration) + .unwrap() + .register_taskset(taskset1) + .test_start(start_task) + .test_stop(stop_task) + .set_scheduler(scheduler.clone()) } - } + }; // Run the Goose Attack. common::run_load_test(manager_goose_attack, Some(worker_handles)).await; diff --git a/tests/sequence.rs b/tests/sequence.rs index 19ed72c3..a44db70a 100644 --- a/tests/sequence.rs +++ b/tests/sequence.rs @@ -240,11 +240,10 @@ async fn run_standalone_test(test_type: TestType) { // Get the taskset, start and stop tasks to build a load test. let (taskset, start_task, stop_task) = get_tasks(&test_type); - let goose_attack; - match test_type { + let goose_attack = match test_type { TestType::NotSequenced | TestType::SequencedRoundRobin => { // Set up the common base configuration. - goose_attack = crate::GooseAttack::initialize_with_config(configuration) + crate::GooseAttack::initialize_with_config(configuration) .unwrap() .register_taskset(taskset) .test_start(start_task) @@ -253,14 +252,14 @@ async fn run_standalone_test(test_type: TestType) { } TestType::SequencedSerial => { // Set up the common base configuration. - goose_attack = crate::GooseAttack::initialize_with_config(configuration) + crate::GooseAttack::initialize_with_config(configuration) .unwrap() .register_taskset(taskset) .test_start(start_task) .test_stop(stop_task) .set_scheduler(GooseScheduler::Serial) } - } + }; // Run the Goose Attack. common::run_load_test(goose_attack, None).await; @@ -310,30 +309,27 @@ async fn run_gaggle_test(test_type: TestType) { // Build Manager configuration. let manager_configuration = common_build_configuration(&server, None, Some(EXPECT_WORKERS)); - let manager_goose_attack; - match test_type { + let manager_goose_attack = match test_type { TestType::NotSequenced | TestType::SequencedRoundRobin => { // Set up the common base configuration. - manager_goose_attack = - crate::GooseAttack::initialize_with_config(manager_configuration) - .unwrap() - .register_taskset(taskset) - .test_start(start_task) - .test_stop(stop_task) - // Unnecessary as this is the default. - .set_scheduler(GooseScheduler::RoundRobin); + crate::GooseAttack::initialize_with_config(manager_configuration) + .unwrap() + .register_taskset(taskset) + .test_start(start_task) + .test_stop(stop_task) + // Unnecessary as this is the default. + .set_scheduler(GooseScheduler::RoundRobin) } TestType::SequencedSerial => { // Set up the common base configuration. - manager_goose_attack = - crate::GooseAttack::initialize_with_config(manager_configuration) - .unwrap() - .register_taskset(taskset) - .test_start(start_task) - .test_stop(stop_task) - .set_scheduler(GooseScheduler::Serial); + crate::GooseAttack::initialize_with_config(manager_configuration) + .unwrap() + .register_taskset(taskset) + .test_start(start_task) + .test_stop(stop_task) + .set_scheduler(GooseScheduler::Serial) } - } + }; // Run the Goose Attack. common::run_load_test(manager_goose_attack, Some(worker_handles)).await;