diff --git a/src/handlers/assign.rs b/src/handlers/assign.rs index 8519d3ad8..27435d3fc 100644 --- a/src/handlers/assign.rs +++ b/src/handlers/assign.rs @@ -188,7 +188,7 @@ pub(super) async fn handle_input( if event.issue.assignees.is_empty() { let (assignee, from_comment) = determine_assignee(ctx, assign_command, event, config, &diff).await?; - if assignee.as_deref() == Some(GHOST_ACCOUNT) { + if assignee.as_ref().map(|r| r.name.as_str()) == Some(GHOST_ACCOUNT) { // "ghost" is GitHub's placeholder account for deleted accounts. // It is used here as a convenient way to prevent assignment. This // is typically used for rollups or experiments where you don't @@ -205,7 +205,7 @@ pub(super) async fn handle_input( .await { let who_text = match &assignee { - Some(assignee) => WELCOME_WITH_REVIEWER.replace("{assignee}", assignee), + Some(assignee) => WELCOME_WITH_REVIEWER.replace("{assignee}", &assignee.name), None => WELCOME_WITHOUT_REVIEWER.to_string(), }; let mut welcome = NEW_USER_WELCOME_MESSAGE.replace("{who}", &who_text); @@ -221,7 +221,7 @@ pub(super) async fn handle_input( } else if !from_comment { let welcome = match &assignee { Some(assignee) => RETURNING_USER_WELCOME_MESSAGE - .replace("{assignee}", assignee) + .replace("{assignee}", &assignee.name) .replace("{bot}", &ctx.username), None => RETURNING_USER_WELCOME_MESSAGE_NO_REVIEWER .replace("{author}", &event.issue.user.login), @@ -268,37 +268,38 @@ async fn set_assignee( ctx: &Context, issue: &Issue, github: &GithubClient, - username: &str, + reviewer: &ReviewerSelection, ) -> anyhow::Result<()> { let mut db = ctx.db.get().await; let mut state: IssueData<'_, Reviewers> = IssueData::load(&mut db, &issue, PREVIOUS_REVIEWERS_KEY).await?; // Don't re-assign if already assigned, e.g. on comment edit - if issue.contain_assignee(&username) { + if issue.contain_assignee(&reviewer.name) { log::trace!( "ignoring assign PR {} to {}, already assigned", issue.global_id(), - username, + reviewer.name, ); return Ok(()); } - if let Err(err) = issue.set_assignee(github, &username).await { + if let Err(err) = issue.set_assignee(github, &reviewer.name).await { log::warn!( "failed to set assignee of PR {} to {}: {:?}", issue.global_id(), - username, + reviewer.name, err ); if let Err(e) = issue .post_comment( github, &format!( - "Failed to set assignee to `{username}`: {err}\n\ + "Failed to set assignee to `{}`: {err}\n\ \n\ > **Note**: Only org members with at least the repository \"read\" role, \ users with write permissions, or people who have commented on the PR may \ - be assigned." + be assigned.", + reviewer.name ), ) .await @@ -308,9 +309,30 @@ async fn set_assignee( } } - // Record the reviewer in the database + // If an error was suppressed, post a warning on the PR. + if let Some(suppressed_error) = &reviewer.suppressed_error { + let warning = match suppressed_error { + FindReviewerError::ReviewerOffRotation { username } => Some(format!( + r"`{username}` is not on the review rotation at the moment. +They may take a while to respond. +" + )), + FindReviewerError::ReviewerAtMaxCapacity { username } => Some(format!( + "`{username}` is currently at their maximum review capacity. +They may take a while to respond." + )), + _ => None, + }; + if let Some(warning) = warning { + if let Err(err) = issue.post_comment(&ctx.github, &warning).await { + // This is a best-effort warning, do not do anything apart from logging if it fails + log::warn!("failed to post reviewer warning comment: {err}"); + } + } + } - state.data.names.insert(username.to_lowercase()); + // Record the reviewer in the database + state.data.names.insert(reviewer.name.to_lowercase()); state.save().await?; Ok(()) } @@ -330,7 +352,7 @@ async fn determine_assignee( event: &IssuesEvent, config: &AssignConfig, diff: &[FileDiff], -) -> anyhow::Result<(Option, bool)> { +) -> anyhow::Result<(Option, bool)> { let mut db_client = ctx.db.get().await; let teams = crate::team_data::teams(&ctx.github).await?; if let Some(name) = assign_command { @@ -693,7 +715,7 @@ fn get_team_name<'a>(teams: &Teams, issue: &Issue, name: &'a str) -> Option<&'a teams.teams.get(team_name).map(|_| team_name) } -#[derive(PartialEq, Debug)] +#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] enum FindReviewerError { /// User specified something like `r? foo/bar` where that team name could /// not be found. @@ -715,7 +737,7 @@ enum FindReviewerError { ReviewerPreviouslyAssigned { username: String }, /// Data required for assignment could not be loaded from the DB. DatabaseError(String), - /// The reviewer has too many PRs alreayd assigned. + /// The reviewer has too many PRs already assigned. ReviewerAtMaxCapacity { username: String }, } @@ -781,6 +803,24 @@ Please select a different reviewer.", } } +/// Reviewer that was found to be eligible as a result of `r? <...>`. +/// In some cases, a reviewer selection error might have been suppressed. +/// We store it here to allow sending a comment with a warning about the suppressed error. +#[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] +struct ReviewerSelection { + name: String, + suppressed_error: Option, +} + +impl ReviewerSelection { + fn from_name(name: String) -> Self { + Self { + name, + suppressed_error: None, + } + } +} + /// Finds a reviewer to assign to a PR. /// /// The `names` is a list of candidate reviewers `r?`, such as `compiler` or @@ -794,11 +834,11 @@ async fn find_reviewer_from_names( config: &AssignConfig, issue: &Issue, names: &[String], -) -> Result { +) -> Result { // Fast path for self-assign, which is always allowed. if let [name] = names { if is_self_assign(&name, &issue.user.login) { - return Ok(name.clone()); + return Ok(ReviewerSelection::from_name(name.clone())); } } @@ -827,26 +867,25 @@ async fn find_reviewer_from_names( // sure they are really worth the effort. log::info!( - "[#{}] Initial unfiltered list of candidates: {:?}", + "[#{}] Filtered list of candidates: {:?}", issue.number, candidates ); - // Return unfiltered list of candidates + // Select a random reviewer from the filtered list Ok(candidates .into_iter() .choose(&mut rand::thread_rng()) - .expect("candidate_reviewers_from_names should return at least one entry") - .to_string()) + .expect("candidate_reviewers_from_names should return at least one entry")) } -#[derive(Eq, PartialEq, Hash)] +#[derive(Eq, PartialEq, Hash, Debug)] struct ReviewerCandidate { name: String, origin: ReviewerCandidateOrigin, } -#[derive(Eq, PartialEq, Hash, Copy, Clone)] +#[derive(Eq, PartialEq, Hash, Copy, Clone, Debug)] enum ReviewerCandidateOrigin { /// This reviewer was directly requested for a review. Direct, @@ -962,7 +1001,7 @@ async fn candidate_reviewers_from_names<'a>( config: &'a AssignConfig, issue: &Issue, names: &'a [String], -) -> Result, FindReviewerError> { +) -> Result, FindReviewerError> { // Step 1: expand teams and groups into candidate names let expanded = expand_teams_and_groups(teams, issue, config, names)?; let expanded_count = expanded.len(); @@ -976,7 +1015,7 @@ async fn candidate_reviewers_from_names<'a>( // Set of candidate usernames to choose from. // We go through each expanded candidate and store either success or an error for them. - let mut candidates: Vec> = Vec::new(); + let mut candidates: Vec> = Vec::new(); let previous_reviewer_names = get_previous_reviewer_names(db, issue).await; // Step 2: pre-filter candidates based on checks that we can perform quickly @@ -1022,7 +1061,7 @@ async fn candidate_reviewers_from_names<'a>( if let Some(error_reason) = reason { candidates.push(Err(error_reason)); } else { - candidates.push(Ok(reviewer_candidate.name)); + candidates.push(Ok(reviewer_candidate)); } } assert_eq!(candidates.len(), expanded_count); @@ -1031,7 +1070,7 @@ async fn candidate_reviewers_from_names<'a>( // Step 3: gather potential usernames to form a DB query for review preferences let usernames: Vec = candidates .iter() - .filter_map(|res| res.as_deref().ok().map(|s| s.to_string())) + .filter_map(|res| res.as_ref().ok().map(|s| s.name.to_string())) .collect(); let usernames: Vec<&str> = usernames.iter().map(|s| s.as_str()).collect(); let review_prefs = get_review_prefs_batch(db, &usernames) @@ -1044,27 +1083,32 @@ async fn candidate_reviewers_from_names<'a>( // Step 4: check review preferences candidates = candidates .into_iter() - .map(|username| { + .map(|candidate| { // Only consider candidates that did not have an earlier error - let username = username?; + let candidate = candidate?; + let username = &candidate.name; // If no review prefs were found, we assume the default unlimited // review capacity and being on rotation. let Some(review_prefs) = review_prefs.get(username.as_str()) else { - return Ok(username); + return Ok(candidate); }; if let Some(capacity) = review_prefs.max_assigned_prs { let assigned_prs = workqueue.assigned_pr_count(review_prefs.user_id as UserId); // Is the reviewer at max capacity? if (assigned_prs as i32) >= capacity { - return Err(FindReviewerError::ReviewerAtMaxCapacity { username }); + return Err(FindReviewerError::ReviewerAtMaxCapacity { + username: username.clone(), + }); } } if review_prefs.rotation_mode == RotationMode::OffRotation { - return Err(FindReviewerError::ReviewerOffRotation { username }); + return Err(FindReviewerError::ReviewerOffRotation { + username: username.clone(), + }); } - return Ok(username); + return Ok(candidate); }) .collect(); } @@ -1072,7 +1116,7 @@ async fn candidate_reviewers_from_names<'a>( let valid_candidates: HashSet<&str> = candidates .iter() - .filter_map(|res| res.as_deref().ok()) + .filter_map(|res| res.as_ref().ok().map(|c| c.name.as_str())) .collect(); log::debug!( @@ -1083,13 +1127,24 @@ async fn candidate_reviewers_from_names<'a>( ); if valid_candidates.is_empty() { - // If we requested a single user for a review, we return a concrete error message - // describing why they couldn't be assigned. if is_single_user { - Err(candidates + // If we requested a single user for a review, we may suppress some errors. + // Check what error we got here. + let error = candidates .pop() .unwrap() - .expect_err("valid_candidates is empty, so this should be an error")) + .expect_err("valid_candidates is empty, so this should be an error"); + let username = match &error { + // If the reviewer is at capacity or off rotation, allow them to be requested, + // but store the suppressed error. + FindReviewerError::ReviewerOffRotation { username } + | FindReviewerError::ReviewerAtMaxCapacity { username } => username, + _ => return Err(error), + }; + Ok(HashSet::from([ReviewerSelection { + name: username.to_string(), + suppressed_error: Some(error), + }])) } else { // If it was a request for a team or a group, and no one is available, simply // return `NoReviewer`. @@ -1105,7 +1160,7 @@ async fn candidate_reviewers_from_names<'a>( } else { Ok(valid_candidates .into_iter() - .map(|s| s.to_string()) + .map(|s| ReviewerSelection::from_name(s.to_string())) .collect()) } } diff --git a/src/handlers/assign/tests/tests_candidates.rs b/src/handlers/assign/tests/tests_candidates.rs index 3a26fe1f7..a9e8f94f9 100644 --- a/src/handlers/assign/tests/tests_candidates.rs +++ b/src/handlers/assign/tests/tests_candidates.rs @@ -13,7 +13,7 @@ struct AssignCtx { teams: Teams, config: AssignConfig, issue: Issue, - reviewer_workqueue: ReviewerWorkqueue, + reviewer_workqueue: HashMap>, } impl AssignCtx { @@ -51,7 +51,7 @@ impl AssignCtx { fn assign_prs(mut self, user_id: UserId, count: u64) -> Self { let prs: HashSet = (0..count).collect(); - self.reviewer_workqueue.set_user_prs(user_id, prs); + self.reviewer_workqueue.insert(user_id, prs); self } @@ -88,13 +88,14 @@ impl AssignCtx { async fn check( mut self, names: &[&str], - expected: Result<&[&str], FindReviewerError>, - ) -> anyhow::Result { + expected: Result<&[ReviewerSelection], FindReviewerError>, + ) -> anyhow::Result { let names: Vec<_> = names.iter().map(|n| n.to_string()).collect(); + let workqueue = ReviewerWorkqueue::new(self.reviewer_workqueue.clone()); let reviewers = candidate_reviewers_from_names( self.test_ctx.db_client_mut(), - Arc::new(RwLock::new(self.reviewer_workqueue)), + Arc::new(RwLock::new(workqueue)), &self.teams, &self.config, &self.issue, @@ -103,9 +104,9 @@ impl AssignCtx { .await; match (reviewers, expected) { (Ok(candidates), Ok(expected)) => { - let mut candidates: Vec<_> = candidates.into_iter().collect(); + let mut candidates: Vec<&ReviewerSelection> = candidates.iter().collect(); candidates.sort(); - let expected: Vec<_> = expected.iter().map(|x| *x).collect(); + let expected: Vec<&ReviewerSelection> = expected.iter().collect(); assert_eq!(candidates, expected); } (Err(actual), Err(expected)) => { @@ -114,7 +115,19 @@ impl AssignCtx { (Ok(candidates), Err(_)) => panic!("expected Err, got Ok: {candidates:?}"), (Err(e), Ok(_)) => panic!("expected Ok, got Err: {e}"), }; - Ok(self.test_ctx) + Ok(self) + } +} + +impl From for TestContext { + fn from(value: AssignCtx) -> Self { + value.test_ctx + } +} + +impl From<&str> for ReviewerSelection { + fn from(value: &str) -> Self { + ReviewerSelection::from_name(value.to_string()) } } @@ -135,7 +148,7 @@ async fn no_assigned_prs() { review_prefs_test(ctx) .set_review_prefs(&user, Some(3), RotationMode::OnRotation) .await - .check(&["martin"], Ok(&["martin"])) + .check(&["martin"], Ok(&["martin".into()])) .await }) .await; @@ -147,7 +160,7 @@ async fn no_review_prefs() { ctx.add_user("martin", 1).await; review_prefs_test(ctx) .assign_prs(1, 3) - .check(&["martin"], Ok(&["martin"])) + .check(&["martin"], Ok(&["martin".into()])) .await }) .await; @@ -155,18 +168,25 @@ async fn no_review_prefs() { #[tokio::test] async fn at_max_capacity() { + let teams = toml::toml!(compiler = ["martin", "diana"]); run_db_test(|ctx| async move { let user = user("martin", 1); review_prefs_test(ctx) + .teams(&teams) .set_review_prefs(&user, Some(3), RotationMode::OnRotation) .await .assign_prs(user.id, 3) .check( &["martin"], - Err(FindReviewerError::ReviewerAtMaxCapacity { - username: "martin".to_string(), - }), + Ok(&[ReviewerSelection { + name: "martin".to_string(), + suppressed_error: Some(FindReviewerError::ReviewerAtMaxCapacity { + username: "martin".to_string(), + }), + }]), ) + .await? + .check(&["compiler"], Ok(&["diana".into()])) .await }) .await; @@ -180,7 +200,7 @@ async fn below_max_capacity() { .set_review_prefs(&user, Some(3), RotationMode::OnRotation) .await .assign_prs(user.id, 2) - .check(&["martin"], Ok(&["martin"])) + .check(&["martin"], Ok(&["martin".into()])) .await }) .await; @@ -188,18 +208,25 @@ async fn below_max_capacity() { #[tokio::test] async fn above_max_capacity() { + let teams = toml::toml!(compiler = ["martin", "diana"]); run_db_test(|ctx| async move { let user = user("martin", 1); review_prefs_test(ctx) + .teams(&teams) .set_review_prefs(&user, Some(3), RotationMode::OnRotation) .await .assign_prs(user.id, 10) .check( &["martin"], - Err(FindReviewerError::ReviewerAtMaxCapacity { - username: "martin".to_string(), - }), + Ok(&[ReviewerSelection { + name: "martin".to_string(), + suppressed_error: Some(FindReviewerError::ReviewerAtMaxCapacity { + username: "martin".to_string(), + }), + }]), ) + .await? + .check(&["compiler"], Ok(&["diana".into()])) .await }) .await; @@ -207,18 +234,25 @@ async fn above_max_capacity() { #[tokio::test] async fn max_capacity_zero() { + let teams = toml::toml!(compiler = ["martin", "diana"]); run_db_test(|ctx| async move { let user = user("martin", 1); review_prefs_test(ctx) + .teams(&teams) .set_review_prefs(&user, Some(0), RotationMode::OnRotation) .await .assign_prs(user.id, 0) .check( &["martin"], - Err(FindReviewerError::ReviewerAtMaxCapacity { - username: "martin".to_string(), - }), + Ok(&[ReviewerSelection { + name: "martin".to_string(), + suppressed_error: Some(FindReviewerError::ReviewerAtMaxCapacity { + username: "martin".to_string(), + }), + }]), ) + .await? + .check(&["compiler"], Ok(&["diana".into()])) .await }) .await; @@ -226,18 +260,25 @@ async fn max_capacity_zero() { #[tokio::test] async fn ignore_username_case() { + let teams = toml::toml!(compiler = ["martin", "diana"]); run_db_test(|ctx| async move { let user = user("MARtin", 1); review_prefs_test(ctx) + .teams(&teams) .set_review_prefs(&user, Some(3), RotationMode::OnRotation) .await .assign_prs(user.id, 3) .check( &["MARTIN"], - Err(FindReviewerError::ReviewerAtMaxCapacity { - username: "MARTIN".to_string(), - }), + Ok(&[ReviewerSelection { + name: "MARTIN".to_string(), + suppressed_error: Some(FindReviewerError::ReviewerAtMaxCapacity { + username: "MARTIN".to_string(), + }), + }]), ) + .await? + .check(&["compiler"], Ok(&["diana".into()])) .await }) .await; @@ -251,32 +292,14 @@ async fn unlimited_capacity() { .set_review_prefs(&user, None, RotationMode::OnRotation) .await .assign_prs(user.id, 10) - .check(&["martin"], Ok(&["martin"])) - .await - }) - .await; -} - -#[tokio::test] -async fn ignore_user_off_rotation_direct() { - run_db_test(|ctx| async move { - let user = user("martin", 1); - review_prefs_test(ctx) - .set_review_prefs(&user, None, RotationMode::OffRotation) - .await - .check( - &["martin"], - Err(FindReviewerError::ReviewerOffRotation { - username: "martin".to_string(), - }), - ) + .check(&["martin"], Ok(&["martin".into()])) .await }) .await; } #[tokio::test] -async fn ignore_user_off_rotation_through_team() { +async fn user_off_rotation() { run_db_test(|ctx| async move { let teams = toml::toml!(compiler = ["martin", "diana"]); let user = user("martin", 1); @@ -284,26 +307,17 @@ async fn ignore_user_off_rotation_through_team() { .teams(&teams) .set_review_prefs(&user, None, RotationMode::OffRotation) .await - .check(&["compiler"], Ok(&["diana"])) - .await - }) - .await; -} - -#[tokio::test] -async fn review_prefs_prefer_capacity_before_rotation() { - run_db_test(|ctx| async move { - let user = user("martin", 1); - review_prefs_test(ctx) - .set_review_prefs(&user, Some(1), RotationMode::OffRotation) - .await - .assign_prs(user.id, 2) .check( &["martin"], - Err(FindReviewerError::ReviewerAtMaxCapacity { - username: "martin".to_string(), - }), + Ok(&[ReviewerSelection { + name: "martin".to_string(), + suppressed_error: Some(FindReviewerError::ReviewerOffRotation { + username: "martin".to_string(), + }), + }]), ) + .await? + .check(&["compiler"], Ok(&["diana".into()])) .await }) .await; @@ -325,7 +339,7 @@ async fn multiple_reviewers() { .assign_prs(users[0].id, 4) .assign_prs(users[1].id, 2) .assign_prs(users[2].id, 2) - .check(&["team"], Ok(&["diana", "jana"])) + .check(&["team"], Ok(&["diana".into(), "jana".into()])) .await }) .await; @@ -364,7 +378,7 @@ async fn nested_groups() { ); run_db_test(|ctx| async move { basic_test(ctx, config, issue().call()) - .check(&["c"], Ok(&["nrc", "pnkfelix"])) + .check(&["c"], Ok(&["nrc".into(), "pnkfelix".into()])) .await }) .await; @@ -400,7 +414,10 @@ async fn candidate_filtered_author() { ); run_db_test(|ctx| async move { basic_test(ctx, config, issue().author(user("user2", 1)).call()) - .check(&["compiler"], Ok(&["user1", "user3", "user4"])) + .check( + &["compiler"], + Ok(&["user1".into(), "user3".into(), "user4".into()]), + ) .await }) .await; @@ -419,7 +436,7 @@ async fn candidate_filtered_assignee() { .call(); run_db_test(|ctx| async move { basic_test(ctx, config, issue) - .check(&["compiler"], Ok(&["user4"])) + .check(&["compiler"], Ok(&["user4".into()])) .await }) .await; @@ -441,7 +458,12 @@ async fn groups_teams_users() { .teams(&teams) .check( &["team1", "group1", "user3"], - Ok(&["t-user1", "t-user2", "user1", "user3"]), + Ok(&[ + "t-user1".into(), + "t-user2".into(), + "user1".into(), + "user3".into(), + ]), ) .await }) @@ -459,12 +481,12 @@ async fn group_team_user_precedence() { run_db_test(|ctx| async move { let ctx = basic_test(ctx, config.clone(), issue().call()) .teams(&teams) - .check(&["compiler"], Ok(&["user2"])) + .check(&["compiler"], Ok(&["user2".into()])) .await?; - basic_test(ctx, config, issue().call()) + basic_test(ctx.into(), config, issue().call()) .teams(&teams) - .check(&["rust-lang/compiler"], Ok(&["user2"])) + .check(&["rust-lang/compiler"], Ok(&["user2".into()])) .await }) .await; @@ -483,14 +505,11 @@ async fn what_do_slashes_mean() { run_db_test(|ctx| async move { // Random slash names should work from groups. - let ctx = basic_test(ctx, config.clone(), issue()) - .teams(&teams) - .check(&["foo/bar"], Ok(&["foo-user"])) - .await?; - basic_test(ctx, config, issue()) .teams(&teams) - .check(&["rust-lang-nursery/compiler"], Ok(&["user2"])) + .check(&["foo/bar"], Ok(&["foo-user".into()])) + .await? + .check(&["rust-lang-nursery/compiler"], Ok(&["user2".into()])) .await }) .await; @@ -518,25 +537,26 @@ async fn invalid_org_doesnt_match() { } #[tokio::test] -async fn vacation() { +async fn users_on_vacation() { let teams = toml::toml!(bootstrap = ["jyn514", "Mark-Simulacrum"]); let config = toml::toml!(users_on_vacation = ["jyn514"]); run_db_test(|ctx| async move { - // Test that `r? user` returns a specific error about the user being on vacation. - let ctx = basic_test(ctx, config.clone(), issue().call()) + basic_test(ctx, config, issue().call()) .teams(&teams) + // Allow direct assignment .check( &["jyn514"], - Err(FindReviewerError::ReviewerOffRotation { - username: "jyn514".to_string(), - }), + Ok(&[ReviewerSelection { + name: "jyn514".to_string(), + suppressed_error: Some(FindReviewerError::ReviewerOffRotation { + username: "jyn514".to_string(), + }), + }]), ) - .await?; - - basic_test(ctx, config.clone(), issue().call()) - .teams(&teams) - .check(&["bootstrap"], Ok(&["Mark-Simulacrum"])) + .await? + // But ignore the user when requesting a team + .check(&["bootstrap"], Ok(&["Mark-Simulacrum".into()])) .await }) .await; @@ -552,7 +572,7 @@ async fn previous_reviewers_ignore_in_team_success() { .teams(&teams) .set_previous_reviewers(HashSet::from([&user])) .await - .check(&["compiler"], Ok(&["jyn514"])) + .check(&["compiler"], Ok(&["jyn514".into()])) .await }) .await; @@ -591,7 +611,7 @@ async fn previous_reviewers_direct_assignee() { .teams(&teams) .set_previous_reviewers(HashSet::from([&user1, &user2])) .await - .check(&["jyn514"], Ok(&["jyn514"])) + .check(&["jyn514"], Ok(&["jyn514".into()])) .await }) .await diff --git a/src/handlers/pr_tracking.rs b/src/handlers/pr_tracking.rs index 8177534ec..0f9fc2d0d 100644 --- a/src/handlers/pr_tracking.rs +++ b/src/handlers/pr_tracking.rs @@ -43,11 +43,6 @@ impl ReviewerWorkqueue { .map(|prs| prs.len() as u64) .unwrap_or(0) } - - #[cfg(test)] - pub fn set_user_prs(&mut self, user_id: UserId, prs: HashSet) { - self.reviewers.insert(user_id, prs); - } } pub(super) enum ReviewPrefsInput { diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 895433fc3..6442902ad 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -121,14 +121,15 @@ impl TestContext { } } -pub(crate) async fn run_db_test(f: F) +pub(crate) async fn run_db_test(f: F) where F: FnOnce(TestContext) -> Fut, - Fut: Future>, + Fut: Future>, + Ctx: Into, { if let Ok(db_url) = std::env::var("TEST_DB_URL") { let ctx = TestContext::new(&db_url).await; - let ctx = f(ctx).await.expect("Test failed"); + let ctx: TestContext = f(ctx).await.expect("Test failed").into(); ctx.finish().await; } else { eprintln!("Skipping test because TEST_DB_URL was not passed");