Implement comment communication in tests

Author: Kobzol

src/bors/handlers/ping.rs

@@ -17,29 +17,13 @@ pub(super) async fn command_ping<Client: RepositoryClient>(
 
 #[cfg(test)]
 mod tests {
-    use tracing_test::traced_test;
-
-    use crate::tests::event::default_pr_number;
     use crate::tests::mocks::run_test;
-    use crate::tests::state::ClientBuilder;
 
     #[sqlx::test]
     async fn test_ping(pool: sqlx::PgPool) {
-        let state = ClientBuilder::default()
-            .pool(pool.clone())
-            .create_state()
-            .await;
-        state.comment("@bors ping").await;
-        state
-            .client()
-            .check_comments(default_pr_number(), &["Pong 🏓!"]);
-    }
-
-    #[traced_test]
-    #[sqlx::test]
-    async fn test_ping2(pool: sqlx::PgPool) {
         run_test(pool, |mut tester| async {
             tester.post_comment("@bors ping").await;
+            assert_eq!(tester.get_comment().await, "Pong 🏓!");
             Ok(tester)
         })
         .await;

src/tests/mocks/bors.rs

@@ -10,9 +10,10 @@ use tower::Service;
 
 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, World};
+use crate::tests::mocks::{ExternalHttpMock, Repo, World};
 use crate::{
     create_app, create_bors_process, BorsContext, CommandParser, ServerState, WebhookSecret,
 };
@@ -106,8 +107,22 @@ impl BorsTester {
         }
     }
 
+    /// Wait until the next bot comment is received on the default repo and the default PR.
+    pub async fn get_comment(&mut self) -> String {
+        self.http_mock
+            .gh_server
+            .get_comment(Repo::default().name, default_pr_number())
+            .await
+            .content
+    }
+
     pub async fn post_comment(&mut self, content: &str) {
-        self.webhook_comment(Comment::new(content)).await;
+        self.webhook_comment(Comment::new(
+            Repo::default().name,
+            default_pr_number(),
+            content,
+        ))
+        .await;
     }
 
     async fn webhook_comment(&mut self, comment: Comment) {
@@ -140,7 +155,11 @@ impl BorsTester {
     }
 
     pub async fn finish(self) {
+        // Make sure that the event channel senders are closed
         drop(self.app);
+        // Wait until all events are handled in the bors service
         self.bors.await.unwrap();
+        // Flush any local queues
+        self.http_mock.gh_server.assert_empty_queues().await;
     }
 }

src/tests/mocks/comment.rs

@@ -6,27 +6,30 @@ use serde::Serialize;
 use url::Url;
 
 use crate::github::GithubRepoName;
-use crate::tests::event::default_pr_number;
-use crate::tests::mocks::repository::{GitHubRepository, Repo};
+use crate::tests::mocks::repository::GitHubRepository;
 use crate::tests::mocks::user::{GitHubUser, User};
 
 #[derive(Clone, Debug)]
 pub struct Comment {
-    repo: GithubRepoName,
-    pr: u64,
-    author: User,
-    content: String,
+    pub repo: GithubRepoName,
+    pub pr: u64,
+    pub author: User,
+    pub content: String,
 }
 
 impl Comment {
-    pub fn new(content: &str) -> Self {
+    pub fn new(repo: GithubRepoName, pr: u64, content: &str) -> Self {
         Self {
-            repo: Repo::default().name,
-            pr: default_pr_number(),
+            repo,
+            pr,
             author: User::default(),
             content: content.to_string(),
         }
     }
+
+    pub fn with_author(self, author: User) -> Self {
+        Self { author, ..self }
+    }
 }
 
 // Copied from octocrab, since its version if #[non_exhaustive]

src/tests/mocks/github.rs

@@ -1,5 +1,6 @@
 use octocrab::Octocrab;
 use std::collections::HashMap;
+use std::time::Duration;
 use wiremock::MockServer;
 
 use crate::create_github_client;
@@ -9,6 +10,8 @@ use crate::tests::mocks::comment::Comment;
 use crate::tests::mocks::repository::{mock_repo, mock_repo_list};
 use crate::tests::mocks::World;
 
+const DEFAULT_TIMEOUT: Duration = Duration::from_secs(3);
+
 /// Represents the state of a simulated GH repo.
 pub struct GitHubRepoState {
     // We store comments from all PRs inside a single queue, because
@@ -18,6 +21,35 @@ pub struct GitHubRepoState {
     pending_comments: Vec<Comment>,
 }
 
+impl GitHubRepoState {
+    /// Wait until a comment from the given pull request was received.
+    /// If a comment from a different PR is received, it is inserted into
+    /// `pending_comments`, to be picked up later.
+    async fn get_comment(&mut self, pr: u64) -> Comment {
+        // First, try to resolve the comment from the pending comment list
+        if let Some(index) = self.pending_comments.iter().position(|c| c.pr == pr) {
+            return self.pending_comments.remove(index);
+        }
+        // If it is not there, wait until some comment is received
+        loop {
+            let comment = self
+                .comments_queue
+                .recv()
+                .await
+                .expect("Channel was closed while waiting for a comment");
+
+            if comment.pr == pr {
+                return comment;
+            }
+            tracing::warn!(
+                "Received comment for PR {}, while expected for PR {pr}",
+                comment.pr
+            );
+            self.pending_comments.push(comment);
+        }
+    }
+}
+
 pub struct GitHubMockServer {
     mock_server: MockServer,
     repos: HashMap<GithubRepoName, GitHubRepoState>,
@@ -38,7 +70,7 @@ impl GitHubMockServer {
                 name.clone(),
                 GitHubRepoState {
                     comments_queue: comments_rx,
-                    pending_comments: vec![],
+                    pending_comments: Default::default(),
                 },
             );
         }
@@ -56,6 +88,38 @@ impl GitHubMockServer {
         )
         .unwrap()
     }
+
+    pub async fn get_comment(&mut self, repo: GithubRepoName, pr: u64) -> Comment {
+        let repo = self.repos.get_mut(&repo).unwrap();
+        let fut = repo.get_comment(pr);
+        tokio::time::timeout(DEFAULT_TIMEOUT, fut)
+            .await
+            .expect("Timed out while waiting for a comment to be received")
+    }
+
+    /// Make sure that there are no leftover events left in the queues.
+    pub async fn assert_empty_queues(mut self) {
+        // This will remove all mocks and thus also any leftover
+        // channel senders, so that we can be sure below that the `recv`
+        // call will not block indefinitely.
+        self.mock_server.reset().await;
+        drop(self.mock_server);
+
+        for (name, repo) in self.repos.iter_mut() {
+            if !repo.pending_comments.is_empty() {
+                panic!(
+                    "Expected that {name} won't have any received comments, but it has {:?}",
+                    repo.pending_comments
+                );
+            }
+            // Make sure that the queue has been closed and nothing is in it.
+            if let Some(comment) = repo.comments_queue.recv().await {
+                panic!(
+                    "Expected that {name} won't have any received comments, but it has {comment:?}"
+                );
+            }
+        }
+    }
 }
 
 const GITHUB_MOCK_PRIVATE_KEY: &str = r###"-----BEGIN PRIVATE KEY-----

src/tests/mocks/pull_request.rs

@@ -1,34 +1,46 @@
 use serde::{Deserialize, Serialize};
+use tokio::sync::mpsc::Sender;
 use wiremock::{
     matchers::{method, path},
     Mock, MockServer, Request, ResponseTemplate,
 };
 
 use super::{
     comment::{Comment, GitHubComment},
-    Repo,
+    Repo, User,
 };
 
-pub async fn mock_pull_requests(repo: &Repo, mock_server: &MockServer) {
-    for pr_number in &repo.known_prs {
+pub async fn mock_pull_requests(
+    repo: &Repo,
+    comments_tx: Sender<Comment>,
+    mock_server: &MockServer,
+) {
+    let repo_name = repo.name.clone();
+    for &pr_number in &repo.known_prs {
         Mock::given(method("GET"))
-            .and(path(format!("/repos/{}/pulls/{}", repo.name, pr_number)))
+            .and(path(format!("/repos/{repo_name}/pulls/{pr_number}")))
             .respond_with(
-                ResponseTemplate::new(200).set_body_json(GitHubPullRequest::new(*pr_number)),
+                ResponseTemplate::new(200).set_body_json(GitHubPullRequest::new(pr_number)),
             )
             .mount(mock_server)
             .await;
 
+        let repo_name = repo_name.clone();
+        let comments_tx = comments_tx.clone();
         Mock::given(method("POST"))
             .and(path(format!(
-                "/repos/{}/issues/{}/comments",
-                repo.name, pr_number
+                "/repos/{repo_name}/issues/{pr_number}/comments",
             )))
             .respond_with(move |req: &Request| {
                 let comment_payload: CommentCreatePayload = req.body_json().unwrap();
-                let comment: Comment = comment_payload.into();
-                // let mut comments = comments.lock().unwrap();
-                // comments.entry(1).or_default().push(comment.clone());
+                let comment: Comment =
+                    Comment::new(repo_name.clone(), pr_number, &comment_payload.body)
+                        .with_author(User::new(1002, "bors"));
+
+                // 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
+                // `tx.send()`.
+                comments_tx.try_send(comment.clone()).unwrap();
                 ResponseTemplate::new(201).set_body_json(GitHubComment::from(comment))
             })
             .mount(mock_server)
@@ -101,9 +113,3 @@ fn default_pr_number() -> u64 {
 struct CommentCreatePayload {
     body: String,
 }
-
-impl From<CommentCreatePayload> for Comment {
-    fn from(payload: CommentCreatePayload) -> Self {
-        Comment::new(payload.body.as_str())
-    }
-}

src/tests/mocks/repository.rs

@@ -92,7 +92,7 @@ pub async fn mock_repo_list(world: &World, mock_server: &MockServer) {
 }
 
 pub async fn mock_repo(repo: &Repo, comments_tx: Sender<Comment>, mock_server: &MockServer) {
-    mock_pull_requests(repo, mock_server).await;
+    mock_pull_requests(repo, comments_tx, mock_server).await;
     mock_config(repo, mock_server).await;
 }