Merge pull request #108 from rust-lang/tests-migration

Kobzol · web-flow · commit 164043e7280c · 2024-06-02T10:04:34.000Z
Another round of test refactoring
diff --git a/src/bors/handlers/help.rs b/src/bors/handlers/help.rs
@@ -25,15 +25,15 @@ pub(super) async fn command_help<Client: RepositoryClient>(
 #[cfg(test)]
 mod tests {
     use crate::bors::handlers::help::HELP_MESSAGE;
-    use crate::tests::event::default_pr_number;
-    use crate::tests::state::ClientBuilder;
+    use crate::tests::mocks::run_test;
 
     #[sqlx::test]
-    async fn test_help(pool: sqlx::PgPool) {
-        let state = ClientBuilder::default().pool(pool).create_state().await;
-        state.comment("@bors help").await;
-        state
-            .client()
-            .check_comments(default_pr_number(), &[HELP_MESSAGE]);
+    async fn help_command(pool: sqlx::PgPool) {
+        run_test(pool, |mut tester| async {
+            tester.post_comment("@bors help").await;
+            assert_eq!(tester.get_comment().await, HELP_MESSAGE);
+            Ok(tester)
+        })
+        .await;
     }
 }
diff --git a/src/bors/handlers/mod.rs b/src/bors/handlers/mod.rs
@@ -1,5 +1,6 @@
-use anyhow::Context;
 use std::sync::Arc;
+
+use anyhow::Context;
 use tracing::Instrument;
 
 use crate::bors::command::{BorsCommand, CommandParseError};
@@ -273,25 +274,17 @@ fn is_bors_observed_branch(branch: &str) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use crate::tests::event::{comment, default_pr_number};
-    use crate::tests::state::{test_bot_user, ClientBuilder};
+    use crate::tests::mocks::{run_test, Comment, User};
 
     #[sqlx::test]
-    async fn test_ignore_bot_comment(pool: sqlx::PgPool) {
-        let state = ClientBuilder::default().pool(pool).create_state().await;
-        state
-            .comment(comment("@bors ping").author(test_bot_user()).create())
-            .await;
-        state.client().check_comments(default_pr_number(), &[]);
+    async fn ignore_bot_comment(pool: sqlx::PgPool) {
+        run_test(pool, |mut tester| async {
+            tester
+                .post_comment(Comment::from("@bors ping").with_author(User::bors_bot()))
+                .await;
+            // Returning here will make sure that no comments were received
+            Ok(tester)
+        })
+        .await;
     }
-
-    // #[sqlx::test]
-    // async fn test_do_not_comment_when_pr_fetch_fails(pool: sqlx::PgPool) {
-    //     let state = ClientBuilder::default().pool(pool).create_state().await;
-    //     state
-    //         .client()
-    //         .set_get_pr_fn(|_| Err(anyhow::anyhow!("Foo")));
-    //     state.comment(comment("foo").create()).await;
-    //     state.client().check_comments(default_pr_number(), &[]);
-    // }
 }
diff --git a/src/bors/handlers/ping.rs b/src/bors/handlers/ping.rs
@@ -20,7 +20,7 @@ mod tests {
     use crate::tests::mocks::run_test;
 
     #[sqlx::test]
-    async fn test_ping(pool: sqlx::PgPool) {
+    async fn ping_command(pool: sqlx::PgPool) {
         run_test(pool, |mut tester| async {
             tester.post_comment("@bors ping").await;
             assert_eq!(tester.get_comment().await, "Pong 🏓!");
diff --git a/src/bors/mod.rs b/src/bors/mod.rs
@@ -21,7 +21,6 @@ pub use context::BorsContext;
 pub use handlers::{handle_bors_global_event, handle_bors_repository_event};
 
 /// Provides functionality for working with a remote repository.
-#[async_trait]
 pub trait RepositoryClient: Send + Sync {
     fn repository(&self) -> &GithubRepoName;
 
diff --git a/src/github/api/client.rs b/src/github/api/client.rs
@@ -45,7 +45,6 @@ impl GithubRepositoryClient {
     }
 }
 
-#[async_trait]
 impl RepositoryClient for GithubRepositoryClient {
     fn repository(&self) -> &GithubRepoName {
         self.name()
diff --git a/src/github/webhook.rs b/src/github/webhook.rs
@@ -358,16 +358,15 @@ fn verify_gh_signature(
 #[cfg(test)]
 mod tests {
     use axum::extract::FromRequest;
-    use axum::http::{HeaderValue, Method};
-    use hmac::Mac;
-    use hyper::{Request, StatusCode};
+    use hyper::StatusCode;
     use tokio::sync::mpsc;
 
     use crate::bors::event::{BorsEvent, BorsGlobalEvent};
     use crate::github::server::{ServerState, ServerStateRef};
+    use crate::github::webhook::GitHubWebhook;
     use crate::github::webhook::WebhookSecret;
-    use crate::github::webhook::{GitHubWebhook, HmacSha256};
     use crate::tests::io::load_test_file;
+    use crate::tests::webhook::{create_webhook_request, TEST_WEBHOOK_SECRET};
 
     #[tokio::test]
     async fn test_installation_suspend() {
@@ -650,36 +649,14 @@ mod tests {
 
     async fn check_webhook(file: &str, event: &str) -> Result<GitHubWebhook, StatusCode> {
         let body = load_test_file(file);
-        let body_length = body.len();
-
-        let secret = "ABCDEF".to_string();
-        let mut mac =
-            HmacSha256::new_from_slice(secret.as_bytes()).expect("Cannot create HMAC key");
-        mac.update(body.as_bytes());
-        let hash = mac.finalize().into_bytes();
-        let hash = hex::encode(hash);
-        let signature = format!("sha256={hash}");
-
-        let mut request = Request::new(axum::body::Body::from(body));
-        *request.method_mut() = Method::POST;
-        let headers = request.headers_mut();
-        headers.insert("content-type", HeaderValue::from_static("application-json"));
-        headers.insert(
-            "content-length",
-            HeaderValue::from_str(&body_length.to_string()).unwrap(),
-        );
-        headers.insert("x-github-event", HeaderValue::from_str(event).unwrap());
-        headers.insert(
-            "x-hub-signature-256",
-            HeaderValue::from_str(&signature).unwrap(),
-        );
+        let request = create_webhook_request(event, &body);
 
         let (repository_tx, _) = mpsc::channel(1024);
         let (global_tx, _) = mpsc::channel(1024);
         let server_ref = ServerStateRef::new(ServerState::new(
             repository_tx,
             global_tx,
-            WebhookSecret::new(secret),
+            WebhookSecret::new(TEST_WEBHOOK_SECRET.to_string()),
         ));
         GitHubWebhook::from_request(request, &server_ref).await
     }
diff --git a/src/lib.rs b/src/lib.rs
@@ -1,3 +1,5 @@
+#![allow(async_fn_in_trait)]
+
 //! This is the library of the bors bot.
 mod bors;
 mod config;
diff --git a/src/tests/event.rs b/src/tests/event.rs
@@ -65,10 +65,6 @@ impl From<CommentBuilder> for PullRequestComment {
     }
 }
 
-pub fn comment(text: &str) -> CommentBuilder {
-    CommentBuilder::default().text(text.to_string())
-}
-
 pub fn suite_success() -> CheckSuite {
     CheckSuite {
         status: CheckSuiteStatus::Success,
diff --git a/src/tests/mocks/app.rs b/src/tests/mocks/app.rs
@@ -1,3 +1,4 @@
+use crate::tests::mocks::User;
 use serde::Serialize;
 use url::Url;
 use wiremock::{
@@ -53,7 +54,7 @@ impl Default for App {
             owner: GitHubUser::default(),
             name: "bors".to_string(),
             // same as bors user html_url
-            html_url: "https://test-bors.bot.com".parse().unwrap(),
+            html_url: GitHubUser::from(User::bors_bot()).html_url,
             external_url: "https://test-bors.bot.com".parse().unwrap(),
             permissions: Permissions {},
             events: vec!["*".to_string()],
diff --git a/src/tests/mocks/bors.rs b/src/tests/mocks/bors.rs
@@ -12,8 +12,8 @@ use crate::github::api::load_repositories;
 use crate::tests::database::MockedDBClient;
 use crate::tests::event::default_pr_number;
 use crate::tests::mocks::comment::{Comment, GitHubIssueCommentEventPayload};
-use crate::tests::mocks::webhook::{create_webhook_request, TEST_WEBHOOK_SECRET};
 use crate::tests::mocks::{ExternalHttpMock, Repo, World};
+use crate::tests::webhook::{create_webhook_request, TEST_WEBHOOK_SECRET};
 use crate::{
     create_app, create_bors_process, BorsContext, CommandParser, ServerState, WebhookSecret,
 };
@@ -116,13 +116,8 @@ impl BorsTester {
             .content
     }
 
-    pub async fn post_comment(&mut self, content: &str) {
-        self.webhook_comment(Comment::new(
-            Repo::default().name,
-            default_pr_number(),
-            content,
-        ))
-        .await;
+    pub async fn post_comment<C: Into<Comment>>(&mut self, comment: C) {
+        self.webhook_comment(comment.into()).await;
     }
 
     async fn webhook_comment(&mut self, comment: Comment) {
@@ -134,7 +129,7 @@ impl BorsTester {
     }
 
     async fn send_webhook<S: Serialize>(&mut self, event: &str, content: S) {
-        let webhook = create_webhook_request(event, content);
+        let webhook = create_webhook_request(event, &serde_json::to_string(&content).unwrap());
         let response = self
             .app
             .call(webhook)
diff --git a/src/tests/mocks/comment.rs b/src/tests/mocks/comment.rs
@@ -6,8 +6,10 @@ use serde::Serialize;
 use url::Url;
 
 use crate::github::GithubRepoName;
+use crate::tests::event::default_pr_number;
 use crate::tests::mocks::repository::GitHubRepository;
 use crate::tests::mocks::user::{GitHubUser, User};
+use crate::tests::mocks::Repo;
 
 #[derive(Clone, Debug)]
 pub struct Comment {
@@ -32,6 +34,12 @@ impl Comment {
     }
 }
 
+impl<'a> From<&'a str> for Comment {
+    fn from(value: &'a str) -> Self {
+        Comment::new(Repo::default().name, default_pr_number(), value)
+    }
+}
+
 // Copied from octocrab, since its version if #[non_exhaustive]
 #[derive(Serialize)]
 pub struct GitHubIssueCommentEventPayload {
diff --git a/src/tests/mocks/mod.rs b/src/tests/mocks/mod.rs
@@ -9,6 +9,7 @@ use crate::tests::mocks::permissions::TeamApiMockServer;
 use crate::TeamApiClient;
 
 pub use bors::run_test;
+pub use comment::Comment;
 pub use repository::Repo;
 pub use user::User;
 
@@ -20,7 +21,6 @@ mod permissions;
 mod pull_request;
 mod repository;
 mod user;
-mod webhook;
 
 pub struct World {
     repos: HashMap<GithubRepoName, Repo>,
diff --git a/src/tests/mocks/pull_request.rs b/src/tests/mocks/pull_request.rs
@@ -10,6 +10,10 @@ use super::{
     Repo, User,
 };
 
+pub fn default_pr_number() -> u64 {
+    1
+}
+
 pub async fn mock_pull_requests(
     repo: &Repo,
     comments_tx: Sender<Comment>,
@@ -35,7 +39,7 @@ pub async fn mock_pull_requests(
                 let comment_payload: CommentCreatePayload = req.body_json().unwrap();
                 let comment: Comment =
                     Comment::new(repo_name.clone(), pr_number, &comment_payload.body)
-                        .with_author(User::new(1002, "bors"));
+                        .with_author(User::bors_bot());
 
                 // We cannot use `tx.blocking_send()`, because this function is actually called
                 // from within an async task, but it is not async, so we also cannot use
@@ -105,10 +109,6 @@ struct Base {
     sha: String,
 }
 
-fn default_pr_number() -> u64 {
-    1
-}
-
 #[derive(Deserialize)]
 struct CommentCreatePayload {
     body: String,
diff --git a/src/tests/mocks/repository.rs b/src/tests/mocks/repository.rs
@@ -64,7 +64,7 @@ timeout = 3600
     }
 }
 
-fn default_repo_name() -> GithubRepoName {
+pub fn default_repo_name() -> GithubRepoName {
     GithubRepoName::new("rust-lang", "borstest")
 }
 
diff --git a/src/tests/mocks/user.rs b/src/tests/mocks/user.rs
@@ -8,6 +8,14 @@ pub struct User {
 }
 
 impl User {
+    pub fn default_user() -> Self {
+        Self::new(101, "default-user")
+    }
+
+    pub fn bors_bot() -> Self {
+        Self::new(102, "bors-bot")
+    }
+
     pub fn new(id: u64, name: &str) -> Self {
         Self {
             github_id: id,
@@ -18,7 +26,7 @@ impl User {
 
 impl Default for User {
     fn default() -> Self {
-        Self::new(101, "default-user")
+        Self::default_user()
     }
 }
 
@@ -30,7 +38,7 @@ pub struct GitHubUser {
     avatar_url: Url,
     gravatar_id: String,
     url: Url,
-    html_url: Url,
+    pub html_url: Url,
     followers_url: Url,
     following_url: Url,
     gists_url: Url,
diff --git a/src/tests/mod.rs b/src/tests/mod.rs
@@ -4,3 +4,4 @@ pub(crate) mod github;
 pub(crate) mod io;
 pub(crate) mod mocks;
 pub(crate) mod state;
+pub(crate) mod webhook;
diff --git a/src/tests/state.rs b/src/tests/state.rs
@@ -412,7 +412,6 @@ impl TestRepositoryClient {
     }
 }
 
-#[async_trait]
 impl RepositoryClient for Arc<TestRepositoryClient> {
     fn repository(&self) -> &GithubRepoName {
         &self.name
diff --git a/src/tests/webhook.rs b/src/tests/webhook.rs
@@ -1,13 +1,11 @@
 use axum::body::Body;
 use hmac::{Hmac, Mac};
 use http::Request;
-use serde::Serialize;
 use sha2::Sha256;
 
 pub const TEST_WEBHOOK_SECRET: &str = "ABCDEF";
 
-pub fn create_webhook_request<S: Serialize>(event: &str, content: S) -> Request<Body> {
-    let body = serde_json::to_string(&content).unwrap();
+pub fn create_webhook_request(event: &str, body: &str) -> Request<Body> {
     let mut mac = Hmac::<Sha256>::new_from_slice(TEST_WEBHOOK_SECRET.as_bytes()).unwrap();
     mac.update(body.as_bytes());
     let signature = hex::encode(mac.finalize().into_bytes());
@@ -18,6 +16,6 @@ pub fn create_webhook_request<S: Serialize>(event: &str, content: S) -> Request<
         .header("x-github-event", event)
         .header("x-hub-signature-256", signature)
         .header("Content-Type", "application/json")
-        .body(Body::from(body))
+        .body(Body::from(body.to_string()))
         .unwrap()
 }

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,6 @@ impl GithubRepositoryClient {`
`45`	`45`	`}`
`46`	`46`	`}`
`47`	`47`
`48`		`-#[async_trait]`
`49`	`48`	`impl RepositoryClient for GithubRepositoryClient {`
`50`	`49`	`fn repository(&self) -> &GithubRepoName {`
`51`	`50`	`self.name()`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+#![allow(async_fn_in_trait)]`
	`2`	`+`
`1`	`3`	`//! This is the library of the bors bot.`
`2`	`4`	`mod bors;`
`3`	`5`	`mod config;`
Original file line number	Diff line number	Diff line change
`@@ -65,10 +65,6 @@ impl From<CommentBuilder> for PullRequestComment {`
`65`	`65`	`}`
`66`	`66`	`}`
`67`	`67`
`68`		`-pub fn comment(text: &str) -> CommentBuilder {`
`69`		`- CommentBuilder::default().text(text.to_string())`
`70`		`-}`
`71`		`-`
`72`	`68`	`pub fn suite_success() -> CheckSuite {`
`73`	`69`	`CheckSuite {`
`74`	`70`	`status: CheckSuiteStatus::Success,`