fix tests

tag1consulting · Apr 26, 2022 · 41e8f35 · 41e8f35
1 parent 0acaaf5
commit 41e8f35
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,7 +20,7 @@
     o use [FromStr] to auto convert --test-plan "{users},{timespan};{users},{timespan}", where {users} must be an integer, ie "100", and {timespan} can be integer seconds or "30s", "20m", "3h", "1h30m", etc, to internal Vec<(usize, usize)> representation
     o don't allow `--test-plan` together with `--users`, `--startup-time`, `--hatch-rate`, `--run-time`, `--no-reset-metrics`, `--manager` and `--worker`
     o internal `AttackPhase`s renamed: `Starting` -> `Increase`, `Running` -> `Maintain`, `Stopping` -> `Decrease`
- - []() add support for variable speed and multiple decrease AttackPhases
+ - [#450](https://github.com/tag1consulting/goose/pull/450) add support for variable speed and multiple decrease AttackPhases
 
 ## 0.15.2 December 13, 2021
  - [#391](https://github.com/tag1consulting/goose/pull/391) properly sleep for configured `set_wait_time()` walking regularly to exit quickly if the load test ends

diff --git a/src/lib.rs b/src/lib.rs
@@ -306,7 +306,7 @@ struct GooseAttackRunState {
     /// A counter tracking how many [`GooseUser`](./goose/struct.GooseUser.html)s are running.
     active_users: usize,
     /// A counter tracking many users have been stopped.
-    stopped_users: usize,
+    completed_users: usize,
     /// This variable accounts for time spent doing things which is then subtracted from
     /// the time sleeping to avoid an unintentional drift in events that are supposed to
     /// happen regularly.
@@ -1275,7 +1275,7 @@ impl GooseAttack {
             adjust_user_timer: std_now,
             adjust_user_in_ms: 0,
             active_users: 0,
-            stopped_users: 0,
+            completed_users: 0,
             drift_timer: tokio::time::Instant::now(),
             all_threads_metrics_tx,
             metrics_rx,
@@ -1306,6 +1306,31 @@ impl GooseAttack {
         Ok(goose_attack_run_state)
     }
 
+    // Add delay before starting next step if there's time remaining.
+    async fn end_of_step_delay(&mut self) {
+        // Determine how long has elapsed since this step started.
+        let elapsed = if let Some(step_started) = self.step_started {
+            step_started.elapsed().as_millis() as u64
+        } else if let Some(started) = self.started {
+            started.elapsed().as_millis() as u64
+        } else {
+            unreachable!("the load test had to have started");
+        };
+
+        // Determine if there's remaining time in this step.
+        if elapsed < self.test_plan.steps[self.test_plan.current].1 as u64 {
+            let remainder = self.test_plan.steps[self.test_plan.current].1 as u64 - elapsed;
+            let maximum_sleep = 1_000;
+            let sleep_duration = if remainder > maximum_sleep {
+                // Do not sleep longer than 1s.
+                Duration::from_millis(maximum_sleep)
+            } else {
+                Duration::from_millis(remainder)
+            };
+            tokio::time::sleep(sleep_duration).await
+        }
+    }
+
     // Increase the number of active [`GooseUser`](./goose/struct.GooseUser.html) threads in the
     // active [`GooseAttack`](./struct.GooseAttack.html).
     async fn increase_attack(
@@ -1357,8 +1382,7 @@ impl GooseAttack {
             };
 
             // Remember which task group this user is using.
-            thread_user.weighted_users_index =
-                goose_attack_run_state.active_users + goose_attack_run_state.stopped_users;
+            thread_user.weighted_users_index = self.metrics.total_users;
 
             // Create a per-thread channel allowing parent thread to control child threads.
             let (parent_sender, thread_receiver): (
@@ -1448,20 +1472,16 @@ impl GooseAttack {
         };
 
         if all_users_launched {
-            // Pause a tenth of a second waiting for the final user to fully start up.
-            tokio::time::sleep(Duration::from_millis(100)).await;
+            // Pause up to 1 additional second before moving onto the next step to match
+            // what's defined in the test plan.
+            self.end_of_step_delay().await;
 
             if self.attack_mode == AttackMode::Worker {
                 info!(
                     "[{}] launched {} users...",
                     get_worker_id(),
                     self.test_plan.steps[self.test_plan.current].0
                 );
-            } else {
-                info!(
-                    "launched {} users...",
-                    self.test_plan.steps[self.test_plan.current].0
-                );
             }
 
             // Automatically reset metrics if appropriate.
@@ -1516,10 +1536,6 @@ impl GooseAttack {
         // Retreive the number of users configured in the previous step.
         let previous_users = self.test_plan.steps[self.test_plan.current - 1].0;
 
-        // Sanity check: decrease_attack can only be called if the number of users is decreasing
-        // in the current step.
-        debug_assert!(self.test_plan.steps[self.test_plan.current].0 < previous_users);
-
         // Divide the number of users to decrease by the time configured to decrease them.
         let decrease_rate: f32 = (previous_users - self.test_plan.steps[self.test_plan.current].0)
             as f32
@@ -1546,17 +1562,17 @@ impl GooseAttack {
                     Ok(_) => {
                         debug!(
                             "telling user {} to exit",
-                            goose_attack_run_state.stopped_users
+                            goose_attack_run_state.completed_users
                         );
                     }
                     Err(e) => {
                         info!(
                             "failed to tell user {} to exit: {}",
-                            goose_attack_run_state.stopped_users, e
+                            goose_attack_run_state.completed_users, e
                         );
                     }
                 }
-                goose_attack_run_state.stopped_users += 1;
+                goose_attack_run_state.completed_users += 1;
                 goose_attack_run_state.active_users -= 1;
             }
         } else {
@@ -1634,6 +1650,10 @@ impl GooseAttack {
         } else if goose_attack_run_state.active_users
             <= self.test_plan.steps[self.test_plan.current].0
         {
+            // Pause up to 1 additional second before moving onto the next step to match
+            // what's defined in the test plan.
+            self.end_of_step_delay().await;
+
             // Moving to the next phase, reset adjust_user_in_ms.
             goose_attack_run_state.adjust_user_in_ms = 0;
 
@@ -1818,10 +1838,7 @@ impl GooseAttack {
                     }
                 }
                 // By reaching the Shutdown phase, break out of the GooseAttack loop.
-                AttackPhase::Shutdown => {
-                    self.update_duration();
-                    break;
-                }
+                AttackPhase::Shutdown => break,
             }
 
             // Record current users for users per second graph in HTML report.

diff --git a/src/manager.rs b/src/manager.rs
@@ -317,8 +317,10 @@ pub(crate) async fn manager_main(mut goose_attack: GooseAttack) -> GooseAttack {
     let maximum_hatched = hatch_rate * goose_attack.test_plan.steps[1].1 as f32;
     if maximum_hatched < goose_attack.configuration.users.unwrap() as f32 {
         goose_attack.metrics.maximum_users = maximum_hatched as usize;
+        goose_attack.metrics.total_users = maximum_hatched as usize;
     } else {
         goose_attack.metrics.maximum_users = goose_attack.configuration.users.unwrap();
+        goose_attack.metrics.total_users = goose_attack.configuration.users.unwrap();
     }
 
     // Worker control loop.

diff --git a/tests/closure.rs b/tests/closure.rs
@@ -211,7 +211,7 @@ fn validate_closer_test(
     assert!(difference >= -2 && difference <= 2);
 
     // Verify that Goose started the correct number of users.
-    assert!(goose_metrics.maximum_users == configuration.users.unwrap());
+    assert!(goose_metrics.total_users == configuration.users.unwrap());
 }
 
 // Helper to run the test, takes a flag for indicating if running in standalone

diff --git a/tests/controller.rs b/tests/controller.rs
@@ -138,7 +138,7 @@ fn validate_one_taskset(
     assert!(about_metrics.fail_count == 0);
 
     // Users were correctly configured through the controller.
-    assert!(goose_metrics.maximum_users == USERS);
+    assert!(goose_metrics.total_users == USERS);
 
     // Host was not configured at start time.
     assert!(configuration.host.is_empty());

diff --git a/tests/defaults.rs b/tests/defaults.rs
@@ -25,11 +25,11 @@ const DEBUG_LOG: &str = "debug-test.log";
 const LOG_FORMAT: GooseLogFormat = GooseLogFormat::Raw;
 const THROTTLE_REQUESTS: usize = 10;
 const EXPECT_WORKERS: usize = 2;
-// Increase, Increase, Maintain, Increase, Decrease
-const TEST_PLAN: &str = "4,1;8,1;12,1;12,1;14,1;0,0";
-const TEST_PLAN_MAX_USERS: usize = 14;
-const TEST_PLAN_RUN_TIME: usize = 5;
-const TEST_PLAN_STEPS: usize = 6;
+// Increase, Increase, Decrease, Increase, Maintain, Decrease, Decrease
+const TEST_PLAN: &str = "4,1;8,1;4,2;10,2;10,1;4,1;0,1";
+const TEST_PLAN_MAX_USERS: usize = 10;
+const TEST_PLAN_RUN_TIME: usize = 8;
+const TEST_PLAN_STEPS: usize = 7;
 
 // There are multiple test variations in this file.
 #[derive(Clone)]
@@ -134,7 +134,7 @@ fn validate_test(
     match test_type {
         TestType::NotTestPlan => {
             // Verify that Goose started the correct number of users.
-            assert!(goose_metrics.maximum_users == USERS);
+            assert!(goose_metrics.total_users == USERS);
 
             // Requests are made while GooseUsers are hatched, and then for run_time seconds.
             // Verify that the test ran as long as it was supposed to.
@@ -161,6 +161,10 @@ fn validate_test(
 
             // Requests are made while GooseUsers are increasing or maintaining.
             // Verify that the test ran as long as it was supposed to.
+            eprintln!(
+                "duration: {}, TEST_PLAN_RUN_TIME: {}",
+                goose_metrics.duration, TEST_PLAN_RUN_TIME
+            );
             assert!(goose_metrics.duration == TEST_PLAN_RUN_TIME);
 
             // Be sure there were no more requests made than the throttle should allow.
@@ -730,6 +734,6 @@ async fn test_defaults_no_metrics() {
     // Confirm that we did not track metrics.
     assert!(goose_metrics.requests.is_empty());
     assert!(goose_metrics.tasks.is_empty());
-    assert!(goose_metrics.maximum_users == USERS);
+    assert!(goose_metrics.total_users == USERS);
     assert!(goose_metrics.duration == RUN_TIME);
 }
diff --git a/tests/error.rs b/tests/error.rs
@@ -139,7 +139,7 @@ fn validate_error(
     assert!(a_404_metrics.success_count == 0);
 
     // Verify that Goose started the correct number of users.
-    assert!(goose_metrics.maximum_users == configuration.users.unwrap());
+    assert!(goose_metrics.total_users == configuration.users.unwrap());
 }
 
 // Returns the appropriate taskset needed to build these tests.

diff --git a/tests/one_taskset.rs b/tests/one_taskset.rs
@@ -146,7 +146,7 @@ fn validate_one_taskset(
     assert!(about_metrics.fail_count == 0);
 
     // Verify that Goose started the correct number of users.
-    assert!(goose_metrics.maximum_users == configuration.users.unwrap());
+    assert!(goose_metrics.total_users == configuration.users.unwrap());
 }
 
 // Returns the appropriate taskset needed to build these tests.