Merge pull request #107 from rust-lang/comments-comm

Implement comment checking in tests

Author: Kobzol

src/bors/handlers/ping.rs

@@ -17,32 +17,15 @@ 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::BorsBuilder;
-    use crate::tests::state::ClientBuilder;
+    use crate::tests::mocks::run_test;
 
     #[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) {
-        BorsBuilder::new(pool)
-            .run_test(|mut tester| async {
-                tester.comment("@bors ping").await;
-                Ok(tester)
-            })
-            .await;
+        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,
 };
@@ -50,6 +51,17 @@ impl BorsBuilder {
     }
 }
 
+/// Simple end-to-end test entrypoint for tests that don't need to prepare any custom state.
+pub async fn run_test<
+    F: FnOnce(BorsTester) -> Fut,
+    Fut: Future<Output = anyhow::Result<BorsTester>>,
+>(
+    pool: PgPool,
+    f: F,
+) {
+    BorsBuilder::new(pool).run_test(f).await
+}
+
 /// Represents a running bors web application and also the background
 /// bors process. This structure should be used in tests to test interaction
 /// with the bot.
@@ -58,6 +70,7 @@ impl BorsBuilder {
 /// send channels for the bors process, which should stop its async task.
 pub struct BorsTester {
     app: Router,
+    http_mock: ExternalHttpMock,
     bors: JoinHandle<()>,
 }
 
@@ -87,14 +100,32 @@ impl BorsTester {
         );
         let app = create_app(state);
         let bors = tokio::spawn(bors_process);
-        Self { app, bors }
+        Self {
+            app,
+            http_mock: mock,
+            bors,
+        }
     }
 
-    pub async fn comment(&mut self, content: &str) {
-        self.webhook_comment(Comment::new(content)).await;
+    /// 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(
+            Repo::default().name,
+            default_pr_number(),
+            content,
+        ))
+        .await;
     }
 
-    pub async fn webhook_comment(&mut self, comment: Comment) {
+    async fn webhook_comment(&mut self, comment: Comment) {
         self.send_webhook(
             "issue_comment",
             GitHubIssueCommentEventPayload::from(comment),
@@ -124,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,21 +1,83 @@
 use octocrab::Octocrab;
+use std::collections::HashMap;
+use std::time::Duration;
 use wiremock::MockServer;
 
 use crate::create_github_client;
+use crate::github::GithubRepoName;
 use crate::tests::mocks::app::{default_app_id, AppHandler};
-use crate::tests::mocks::repository::RepositoriesHandler;
+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
+    // we don't necessarily know beforehand which PRs will receive comments.
+    comments_queue: tokio::sync::mpsc::Receiver<Comment>,
+    // We need a local queue to avoid skipping comments received out of order.
+    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>,
 }
 
 impl GitHubMockServer {
     pub async fn start(world: &World) -> Self {
         let mock_server = MockServer::start().await;
-        RepositoriesHandler.mount(world, &mock_server).await;
+        mock_repo_list(world, &mock_server).await;
+
+        // Repositories are mocked separately to make it easier to
+        // pass comm. channels to them.
+        let mut repos = HashMap::default();
+        for (name, repo) in &world.repos {
+            let (comments_tx, comments_rx) = tokio::sync::mpsc::channel(1024);
+            mock_repo(repo, comments_tx, &mock_server).await;
+            repos.insert(
+                name.clone(),
+                GitHubRepoState {
+                    comments_queue: comments_rx,
+                    pending_comments: Default::default(),
+                },
+            );
+        }
+
         AppHandler::default().mount(&mock_server).await;
-        Self { mock_server }
+
+        Self { mock_server, repos }
     }
 
     pub fn client(&self) -> Octocrab {
@@ -26,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/mod.rs

@@ -8,7 +8,7 @@ use crate::tests::mocks::github::GitHubMockServer;
 use crate::tests::mocks::permissions::TeamApiMockServer;
 use crate::TeamApiClient;
 
-pub use bors::BorsBuilder;
+pub use bors::run_test;
 pub use repository::Repo;
 pub use user::User;