update database test exporting & update sorting function

Author: Jamesbarford

database/Cargo.toml

@@ -26,6 +26,7 @@ csv = "1"
 x509-cert = { version = "0.2.5", features = ["pem"] }
 
 intern = { path = "../intern" }
+uuid = "1.16.0"
 
 [dev-dependencies]
 uuid = { version = "1.16.0", features = ["v4"] }

database/src/lib.rs

@@ -13,8 +13,7 @@ pub mod interpolate;
 pub mod metric;
 pub mod pool;
 pub mod selector;
-
-mod tests;
+pub mod tests;
 
 pub use pool::{Connection, Pool};
 

database/src/tests/mod.rs

@@ -23,11 +23,12 @@ pub struct TestContext {
 }
 
 impl TestContext {
+    #[allow(dead_code)]
     async fn new_postgres(db_url: &str) -> Self {
         let config: Config = db_url.parse().expect("Cannot parse connection string");
 
         // Create a new database that will be used for this specific test
-        let client = make_client(&db_url)
+        let client = make_client(db_url)
             .await
             .expect("Cannot connect to database");
         let db_name = format!("db{}", uuid::Uuid::new_v4().to_string().replace("-", ""));
@@ -77,6 +78,7 @@ impl TestContext {
         }
     }
 
+    #[allow(dead_code)]
     async fn new_sqlite() -> Self {
         let pool = Pool::open(":memory:");
         Self {
@@ -85,10 +87,12 @@ impl TestContext {
         }
     }
 
+    #[allow(dead_code)]
     pub fn db_client(&self) -> &Pool {
         &self.client
     }
 
+    #[allow(dead_code)]
     async fn finish(self) {
         // Cleanup the test database
         // First, we need to stop using the database
@@ -114,6 +118,7 @@ impl TestContext {
 }
 
 /// Runs a test against an actual postgres database.
+#[allow(dead_code)]
 pub async fn run_postgres_test<F, Fut>(f: F)
 where
     F: Fn(TestContext) -> Fut,
@@ -141,6 +146,7 @@ where
 
 /// Runs a test against an actual database.
 /// Checks both Postgres and SQLite.
+#[allow(dead_code)]
 pub async fn run_db_test<F, Fut>(f: F)
 where
     F: Fn(TestContext) -> Fut + Clone,

site/src/job_queue.rs

@@ -37,44 +37,30 @@ async fn create_benchmark_request_master_commits(
     Ok(())
 }
 
-/// For getting the priority of the request type;
-/// - Release
-/// - Master
-/// - Try
-fn benchmark_request_commit_type_rank(commit_type: &BenchmarkRequestType) -> u8 {
-    match commit_type {
-        BenchmarkRequestType::Release { .. } => 0,
-        BenchmarkRequestType::Master { .. } => 1,
-        BenchmarkRequestType::Try { .. } => 2,
-    }
-}
-
-/// Sorts the priority order of the queue. It assumes that the only status
-/// is `ArtifactsReady`
-fn sort_benchmark_requests<'a>(
-    done: &mut HashSet<String>,
-    unordered_queue: &'a mut [BenchmarkRequest],
-) -> &'a [BenchmarkRequest] {
+/// Sorts try and master requests that are in the `ArtifactsReady` status.
+/// Doesn't consider in-progress requests or release artifacts.
+fn sort_benchmark_requests<'a>(done: &HashSet<String>, request_queue: &'a mut [BenchmarkRequest]) {
     // A topological sort, where each "level" is additionally altered such that
     // try commits come first, and then sorted by PR # (as a rough heuristic for
     // earlier requests).
+    let mut done: HashSet<&str> = done.iter().map(|x| x.as_str()).collect();
 
     // Ensure all the items are ready to be sorted, if they are not this is
     // undefined behaviour
-    assert!(unordered_queue
-        .iter()
-        .all(|bmr| bmr.status == BenchmarkRequestStatus::ArtifactsReady));
-
-    // Ensure the queue is ordered by status and the `commit_type`
-    unordered_queue
-        .sort_unstable_by_key(|bmr| (benchmark_request_commit_type_rank(&bmr.commit_type)));
+    assert!(request_queue.iter().all(|bmr| {
+        bmr.status == BenchmarkRequestStatus::ArtifactsReady
+            && matches!(
+                bmr.commit_type,
+                BenchmarkRequestType::Master { .. } | BenchmarkRequestType::Try { .. }
+            )
+    }));
 
     let mut finished = 0;
-    while finished < unordered_queue.len() {
+    while finished < request_queue.len() {
         // The next level is those elements in the unordered queue which
         // are ready to be benchmarked (i.e., those with parent in done or no
         // parent).
-        let level_len = partition_in_place(unordered_queue[finished..].iter_mut(), |bmr| {
+        let level_len = partition_in_place(request_queue[finished..].iter_mut(), |bmr| {
             bmr.parent_sha().is_none_or(|parent| done.contains(parent))
         });
 
@@ -83,33 +69,43 @@ fn sort_benchmark_requests<'a>(
         // let the commits be benchmarked in the current order that we have, these benchmark runs
         // just won't have a parent result available.
         if level_len == 0 {
-            return unordered_queue;
+            log::warn!("No commit is ready for benchmarking");
+            #[cfg(test)]
+            {
+                panic!("No commit is ready for benchmarking");
+            }
+            return;
         }
 
-        let level = &mut unordered_queue[finished..][..level_len];
+        // Everything in level has the same topological order, then we sort based on heuristics
+        let level = &mut request_queue[finished..][..level_len];
         level.sort_unstable_by_key(|bmr| {
             (
-                bmr.parent_sha().is_some(),
+                // Order master commits before try commits
+                match bmr.commit_type {
+                    BenchmarkRequestType::Try { .. } => 1,
+                    BenchmarkRequestType::Master { .. } => 0,
+                    BenchmarkRequestType::Release { .. } => unreachable!(),
+                },
                 *bmr.pr().unwrap_or(&0),
                 bmr.created_at,
-                bmr.tag().to_string(),
             )
         });
         for c in level {
-            done.insert(c.tag().to_string());
+            done.insert(c.tag());
         }
         finished += level_len;
     }
-    unordered_queue
 }
 
 /// Given some pending requests and a list of completed requests determine if
 /// we have another request the we can process
 fn get_next_benchmark_request<'a>(
     pending: &'a mut [BenchmarkRequest],
-    completed_set: &mut HashSet<String>,
+    completed_set: &HashSet<String>,
 ) -> Option<&'a BenchmarkRequest> {
-    sort_benchmark_requests(completed_set, pending).first()
+    sort_benchmark_requests(completed_set, pending);
+    pending.first()
 }
 
 /// Enqueue the job into the job_queue