diff --git a/.sqlx/query-94ce8ab6ef6cb76aa62aaf81c9f4b1751318faf2009b411b6cd616f47a82abf0.json b/.sqlx/query-070fd5babee0d508900290c872bd9247855b9712d6633c1dc3683751941749b1.json similarity index 88% rename from .sqlx/query-94ce8ab6ef6cb76aa62aaf81c9f4b1751318faf2009b411b6cd616f47a82abf0.json rename to .sqlx/query-070fd5babee0d508900290c872bd9247855b9712d6633c1dc3683751941749b1.json index 98b40198..cda6106b 100644 --- a/.sqlx/query-94ce8ab6ef6cb76aa62aaf81c9f4b1751318faf2009b411b6cd616f47a82abf0.json +++ b/.sqlx/query-070fd5babee0d508900290c872bd9247855b9712d6633c1dc3683751941749b1.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n id,\n repository as \"repository: GithubRepoName\",\n pr_number as \"pr_number: i64\",\n tag as \"tag: CommentTag\",\n node_id,\n created_at as \"created_at: DateTime\"\n FROM comment\n WHERE repository = $1\n AND pr_number = $2\n AND tag = $3\n ", + "query": "\n SELECT\n id,\n repository as \"repository: GithubRepoName\",\n pr_number as \"pr_number: i64\",\n tag as \"tag: CommentTag\",\n node_id,\n created_at as \"created_at: DateTime\"\n FROM comment\n WHERE repository = $1\n AND pr_number = $2\n AND tag = $3\n ORDER BY created_at\n ", "describe": { "columns": [ { @@ -50,5 +50,5 @@ false ] }, - "hash": "94ce8ab6ef6cb76aa62aaf81c9f4b1751318faf2009b411b6cd616f47a82abf0" + "hash": "070fd5babee0d508900290c872bd9247855b9712d6633c1dc3683751941749b1" } diff --git a/src/bors/comment.rs b/src/bors/comment.rs index 5eeac044..35844703 100644 --- a/src/bors/comment.rs +++ b/src/bors/comment.rs @@ -198,6 +198,14 @@ pub fn try_build_started_comment( Comment::new(msg) } +pub fn append_workflow_links_to_comment(comment_content: &mut String, workflow_links: Vec) { + comment_content.push_str("\n**Workflows**:\n\n"); + + for link in workflow_links { + comment_content.push_str(&format!("- {link}\n")); + } +} + pub fn merge_conflict_comment(branch: &str) -> Comment { let message = format!( r#":lock: Merge conflict diff --git a/src/bors/handlers/help.rs b/src/bors/handlers/help.rs index 3b27534d..d2250a29 100644 --- a/src/bors/handlers/help.rs +++ b/src/bors/handlers/help.rs @@ -87,7 +87,7 @@ mod tests { async fn help_command(pool: sqlx::PgPool) { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment("@bors help").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" You can use the following commands: ## PR management diff --git a/src/bors/handlers/info.rs b/src/bors/handlers/info.rs index b4a4760d..ae1a420f 100644 --- a/src/bors/handlers/info.rs +++ b/src/bors/handlers/info.rs @@ -93,7 +93,7 @@ mod tests { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment("@bors info").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" ## Status of PR `1` - Not Approved @@ -113,7 +113,7 @@ mod tests { tester.post_comment("@bors info").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" ## Status of PR `1` - Approved by: `default-user` @@ -134,7 +134,7 @@ mod tests { tester.post_comment("@bors info").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" ## Status of PR `1` - Not Approved @@ -155,7 +155,7 @@ mod tests { tester.post_comment("@bors info").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" ## Status of PR `1` - Not Approved @@ -186,7 +186,7 @@ mod tests { tester.post_comment("@bors info").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" ## Status of PR `1` - Approved by: `default-user` diff --git a/src/bors/handlers/mod.rs b/src/bors/handlers/mod.rs index 3ac68178..7ac622a8 100644 --- a/src/bors/handlers/mod.rs +++ b/src/bors/handlers/mod.rs @@ -108,7 +108,7 @@ pub async fn handle_bors_repository_event( repo = payload.repository.to_string(), id = payload.run_id.into_inner() ); - handle_workflow_started(db, payload) + handle_workflow_started(repo, db, payload) .instrument(span.clone()) .await?; @@ -721,7 +721,7 @@ mod tests { async fn unknown_command(pool: sqlx::PgPool) { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment(Comment::from("@bors foo")).await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r#"Unknown command "foo". Run `@bors help` to see available commands."#); + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r#"Unknown command "foo". Run `@bors help` to see available commands."#); Ok(()) }) .await; diff --git a/src/bors/handlers/ping.rs b/src/bors/handlers/ping.rs index 431df8dc..84263c70 100644 --- a/src/bors/handlers/ping.rs +++ b/src/bors/handlers/ping.rs @@ -22,7 +22,7 @@ mod tests { async fn ping_command(pool: sqlx::PgPool) { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment("@bors ping").await?; - assert_eq!(tester.get_comment_text(()).await?, "Pong 🏓!"); + assert_eq!(tester.get_next_comment_text(()).await?, "Pong 🏓!"); Ok(()) }) .await; diff --git a/src/bors/handlers/pr_events.rs b/src/bors/handlers/pr_events.rs index fe7e7f07..80d1048e 100644 --- a/src/bors/handlers/pr_events.rs +++ b/src/bors/handlers/pr_events.rs @@ -318,7 +318,7 @@ mod tests { .await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" :warning: The base branch changed to `beta`, and the PR will need to be re-approved. @@ -368,7 +368,7 @@ mod tests { tester.push_to_pr(()).await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" :warning: A new commit `pr-1-commit-1` was pushed to the branch, the PR will need to be re-approved. @@ -681,7 +681,7 @@ mod tests { .workflow_start(WorkflowRunData::from(tester.auto_branch().await).with_run_id(123)) .await?; tester.push_to_pr(()).await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" :warning: A new commit `pr-1-commit-1` was pushed to the branch, the PR will need to be re-approved. @@ -709,7 +709,7 @@ mod tests { tester.workflow_start(tester.auto_branch().await).await?; tester.push_to_pr(()).await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" :warning: A new commit `pr-1-commit-1` was pushed to the branch, the PR will need to be re-approved. diff --git a/src/bors/handlers/refresh.rs b/src/bors/handlers/refresh.rs index d60d326c..59062cbd 100644 --- a/src/bors/handlers/refresh.rs +++ b/src/bors/handlers/refresh.rs @@ -284,7 +284,7 @@ timeout = 3600 tester.cancel_timed_out_builds().await; }) .await; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @":boom: Test timed out after `3600`s"); + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @":boom: Test timed out after `3600`s"); assert_eq!( tester .db() diff --git a/src/bors/handlers/retry.rs b/src/bors/handlers/retry.rs index 1f9e82a5..8aec2478 100644 --- a/src/bors/handlers/retry.rs +++ b/src/bors/handlers/retry.rs @@ -55,7 +55,7 @@ mod tests { .post_comment(Comment::from("@bors retry").with_author(User::unprivileged())) .await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @"@unprivileged-user: :key: Insufficient privileges: not in review users" ); Ok(()) @@ -68,7 +68,7 @@ mod tests { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment(Comment::from("@bors retry")).await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @":exclamation: You can only retry pull requests that are approved and have a previously failed auto build" ); Ok(()) @@ -89,7 +89,7 @@ mod tests { tester.wait_for_pr((), |pr| pr.auto_build.is_none()).await?; tester.process_merge_queue().await; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @":hourglass: Testing commit pr-1-sha with merge merge-1-pr-1..." ); Ok(()) diff --git a/src/bors/handlers/review.rs b/src/bors/handlers/review.rs index 26134aa1..497674e9 100644 --- a/src/bors/handlers/review.rs +++ b/src/bors/handlers/review.rs @@ -388,7 +388,7 @@ mod tests { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment("@bors r+").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" :pushpin: Commit pr-1-sha has been approved by `default-user` @@ -414,7 +414,7 @@ mod tests { .post_comment(format!(r#"@bors r={approve_user}"#).as_str()) .await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" :pushpin: Commit pr-1-sha has been approved by `user1` @@ -438,7 +438,7 @@ mod tests { .post_comment(Comment::from("@bors try").with_author(User::unprivileged())) .await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @"@unprivileged-user: :key: Insufficient privileges: not in try users" ); Ok(()) @@ -454,7 +454,7 @@ mod tests { .await?; tester.post_comment("@bors p=2").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @"@unprivileged-user: :key: Insufficient privileges: not in review users" ); Ok(()) @@ -467,7 +467,7 @@ mod tests { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment("@bors r+").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" :pushpin: Commit pr-1-sha has been approved by `default-user` @@ -480,7 +480,7 @@ mod tests { .expect_approved_by(&User::default_pr_author().name); tester.post_comment("@bors r-").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @"Commit pr-1-sha has been unapproved." ); @@ -498,7 +498,7 @@ mod tests { .post_comment(Comment::from("@bors r-").with_author(User::unprivileged())) .await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @"@unprivileged-user: :key: Insufficient privileges: not in review users" ); @@ -518,7 +518,7 @@ mod tests { tester.set_pr_status_closed(()).await?; tester.post_comment("@bors r-").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @":clipboard: Only unclosed PRs can be unapproved." ); @@ -609,7 +609,7 @@ mod tests { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment("@bors treeclosed=5").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @"Tree closed for PRs with priority less than 5" ); @@ -640,7 +640,7 @@ mod tests { .run_test(async |tester: &mut BorsTester| { tester.post_comment("@bors treeclosed=5").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @"@default-user: :key: Insufficient privileges: not in review users" ); Ok(()) @@ -659,7 +659,7 @@ mod tests { .run_test(async |tester: &mut BorsTester| { tester.post_comment("@bors r+").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @"@default-user: :key: Insufficient privileges: not in review users" ); Ok(()) @@ -676,7 +676,7 @@ mod tests { .post_comment(review_comment("@bors delegate+")) .await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r#" :v: @default-user, you can now approve this pull request! @@ -762,7 +762,7 @@ mod tests { .run_test(async |tester: &mut BorsTester| { tester.post_comment("@bors delegate+").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @"@default-user: :key: Insufficient privileges: not in review users" ); Ok(()) @@ -872,7 +872,7 @@ mod tests { .post_comment(Comment::from("@bors delegate+").with_author(User::try_user())) .await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @"@user-with-try-privileges: :key: Insufficient privileges: not in review users" ); Ok(()) @@ -1113,7 +1113,7 @@ mod tests { .post_comment(review_comment("@bors delegate=try")) .await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" :v: @default-user, you can now perform try builds on this pull request! @@ -1140,7 +1140,7 @@ mod tests { tester.expect_comments((), 1).await; tester.post_comment("@bors try").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" :hourglass: Trying commit pr-1-sha with merge merge-0-pr-1… To cancel the try build, run the command `@bors try cancel`. @@ -1167,7 +1167,7 @@ mod tests { tester.post_comment("@bors r+").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @"@default-user: :key: Insufficient privileges: not in review users" ); tester.get_pr_copy(()).await.expect_unapproved(); @@ -1207,7 +1207,7 @@ mod tests { .set_pr_status_draft(()) .await?; tester.post_comment("@bors r+").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @":clipboard: Only open, non-draft PRs can be approved."); + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @":clipboard: Only open, non-draft PRs can be approved."); tester.get_pr_copy(()).await.expect_unapproved(); Ok(()) }) @@ -1221,7 +1221,7 @@ mod tests { .set_pr_status_closed(()) .await?; tester.post_comment("@bors r+").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @":clipboard: Only open, non-draft PRs can be approved."); + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @":clipboard: Only open, non-draft PRs can be approved."); tester.get_pr_copy(()).await.expect_unapproved(); Ok(()) }) @@ -1235,7 +1235,7 @@ mod tests { .set_pr_status_merged(()) .await?; tester.post_comment("@bors r+").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @":clipboard: Only open, non-draft PRs can be approved."); + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @":clipboard: Only open, non-draft PRs can be approved."); tester.get_pr_copy(()).await.expect_unapproved(); Ok(()) }) @@ -1251,7 +1251,7 @@ mod tests { }) .await?; tester.post_comment("@bors r+").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" :clipboard: Looks like this PR is still in progress, ignoring approval. Hint: Remove **[do not merge]** from this PR's title when it is ready for review. @@ -1280,7 +1280,7 @@ labels_blocking_approval = ["proposed-final-comment-period"] }) .await?; tester.post_comment("@bors r+").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @":clipboard: This PR cannot be approved because it currently has the following label: `proposed-final-comment-period`."); + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @":clipboard: This PR cannot be approved because it currently has the following label: `proposed-final-comment-period`."); tester.get_pr_copy(()).await.expect_unapproved(); Ok(()) }) @@ -1307,7 +1307,7 @@ labels_blocking_approval = ["proposed-final-comment-period", "final-comment-peri }) .await?; tester.post_comment("@bors r+").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @":clipboard: This PR cannot be approved because it currently has the following labels: `proposed-final-comment-period`, `final-comment-period`."); + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @":clipboard: This PR cannot be approved because it currently has the following labels: `proposed-final-comment-period`, `final-comment-period`."); tester.get_pr_copy(()).await.expect_unapproved(); Ok(()) }) @@ -1322,7 +1322,7 @@ labels_blocking_approval = ["proposed-final-comment-period", "final-comment-peri tester.get_pr_copy(()).await.expect_auto_build(|_| true); tester.workflow_start(tester.auto_branch().await).await?; tester.post_comment("@bors r-").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" Commit pr-1-sha has been unapproved. Auto build cancelled due to unapproval. Cancelled workflows: @@ -1346,7 +1346,7 @@ labels_blocking_approval = ["proposed-final-comment-period", "final-comment-peri tester.get_pr_copy(()).await.expect_auto_build(|_| true); tester.workflow_start(tester.auto_branch().await).await?; tester.post_comment("@bors r-").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" Commit pr-1-sha has been unapproved. Auto build cancelled due to unapproval. It was not possible to cancel some workflows. diff --git a/src/bors/handlers/trybuild.rs b/src/bors/handlers/trybuild.rs index e94046b1..e0667090 100644 --- a/src/bors/handlers/trybuild.rs +++ b/src/bors/handlers/trybuild.rs @@ -303,7 +303,7 @@ mod tests { tester.expect_comments((), 1).await; tester.workflow_full_success(tester.try_branch().await).await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r#" :sunny: Try build successful ([Workflow1](https://github.com/rust-lang/borstest/actions/runs/1)) Build commit: merge-0-pr-1 (`merge-0-pr-1`, parent: `main-sha1`) @@ -323,7 +323,7 @@ mod tests { tester.expect_comments((), 1).await; tester.workflow_full_failure(tester.try_branch().await).await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @":broken_heart: Test for merge-0-pr-1 failed: [Workflow1](https://github.com/rust-lang/borstest/actions/runs/1)" ); Ok(()) @@ -354,7 +354,7 @@ mod tests { tester.modify_repo(&default_repo_name(), |repo| repo.update_workflow_run(workflow.clone(), WorkflowStatus::Pending)).await; tester.workflow_full_failure(workflow).await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" :broken_heart: Test for merge-0-pr-1 failed: [Workflow1](https://github.com/rust-lang/borstest/actions/runs/1). Failed jobs: @@ -374,7 +374,7 @@ mod tests { .post_comment(Comment::from("@bors try").with_author(User::unprivileged())) .await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @"@unprivileged-user: :key: Insufficient privileges: not in try users" ); Ok(()) @@ -389,7 +389,7 @@ mod tests { .post_comment(Comment::from("@bors try").with_author(User::try_user())) .await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" :hourglass: Trying commit pr-1-sha with merge merge-0-pr-1… @@ -406,7 +406,7 @@ mod tests { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment("@bors try").await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" :hourglass: Trying commit pr-1-sha with merge merge-0-pr-1… @@ -549,7 +549,7 @@ try-job: Bar .await?; tester.expect_comments((), 1).await; tester.post_comment("@bors try parent=last").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" :hourglass: Trying commit pr-1-sha with merge merge-1-pr-1… To cancel the try build, run the command `@bors try cancel`. @@ -575,7 +575,7 @@ try-job: Bar tester .post_comment("@bors try parent=last") .await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @":exclamation: There was no previous build. Please set an explicit parent or remove the `parent=last` argument to use the default parent."); + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @":exclamation: There was no previous build. Please set an explicit parent or remove the `parent=last` argument to use the default parent."); Ok(()) }) .await; @@ -588,7 +588,7 @@ try-job: Bar tester .post_comment("@bors try") .await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r###" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r###" :lock: Merge conflict This pull request and the base branch diverged in a way that cannot @@ -650,7 +650,7 @@ try-job: Bar tester.post_comment("@bors try").await?; tester.expect_comments((), 1).await; tester.post_comment("@bors try").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" :hourglass: Trying commit pr-1-sha with merge merge-1-pr-1… To cancel the try build, run the command `@bors try cancel`. @@ -672,12 +672,12 @@ try-job: Bar "#, ) .await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" :hourglass: Trying commit pr-1-sha with merge merge-0-pr-1… To cancel the try build, run the command `@bors try cancel`. "); - insta::assert_snapshot!(tester.get_comment_text(()).await?, @"Try build cancelled. Cancelled workflows:"); + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @"Try build cancelled. Cancelled workflows:"); Ok(()) }) .await; @@ -693,7 +693,7 @@ try-job: Bar .await?; tester.post_comment("@bors try").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" :hourglass: Trying commit pr-1-sha with merge merge-1-pr-1… (The previously running try build was automatically cancelled.) @@ -717,7 +717,7 @@ try-job: Bar tester.expect_comments((), 1).await; tester.post_comment("@bors try").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" :hourglass: Trying commit pr-1-sha with merge merge-1-pr-1… To cancel the try build, run the command `@bors try cancel`. @@ -729,7 +729,7 @@ try-job: Bar .with_check_suite_id(2), ) .await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r#" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r#" :sunny: Try build successful ([Workflow1](https://github.com/rust-lang/borstest/actions/runs/2)) Build commit: merge-1-pr-1 (`merge-1-pr-1`, parent: `main-sha1`) @@ -744,7 +744,7 @@ try-job: Bar async fn try_cancel_no_running_build(pool: sqlx::PgPool) { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment("@bors try cancel").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @":exclamation: There is currently no try build in progress."); + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @":exclamation: There is currently no try build in progress."); Ok(()) }) .await; @@ -768,7 +768,7 @@ try-job: Bar )) .await?; tester.post_comment("@bors try cancel").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" Try build cancelled. Cancelled workflows: - https://github.com/rust-lang/borstest/actions/runs/123 - https://github.com/rust-lang/borstest/actions/runs/124 @@ -789,7 +789,7 @@ try-job: Bar .workflow_event(WorkflowEvent::started(tester.try_branch().await)) .await?; tester.post_comment("@bors try cancel").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @"Try build was cancelled. It was not possible to cancel some workflows."); + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @"Try build was cancelled. It was not possible to cancel some workflows."); let build = tester.db().find_build( &default_repo_name(), tester.try_branch().await.get_name().to_string(), @@ -843,7 +843,7 @@ try-job: Bar tester.workflow_full_success(w2).await?; tester.workflow_start(w3).await?; tester.post_comment("@bors try cancel").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" Try build cancelled. Cancelled workflows: - https://github.com/rust-lang/borstest/actions/runs/3 "); @@ -881,7 +881,7 @@ try_failed = ["+foo", "+bar", "-baz"] )) .run_test(async |tester: &mut BorsTester| { tester.post_comment("@bors try").await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" :hourglass: Trying commit pr-1-sha with merge merge-0-pr-1… To cancel the try build, run the command `@bors try cancel`. @@ -1036,7 +1036,7 @@ try_failed = ["+foo", "+bar", "-baz"] async fn hide_try_build_started_comment_after_success(pool: sqlx::PgPool) { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment("@bors try").await?; - let comment = tester.get_comment(()).await?; + let comment = tester.get_next_comment(()).await?; tester .workflow_full_success(tester.try_branch().await) .await?; @@ -1054,7 +1054,7 @@ try_failed = ["+foo", "+bar", "-baz"] async fn hide_try_build_started_comment_after_failure(pool: sqlx::PgPool) { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment("@bors try").await?; - let comment = tester.get_comment(()).await?; + let comment = tester.get_next_comment(()).await?; tester .workflow_full_failure(tester.try_branch().await) .await?; @@ -1072,7 +1072,7 @@ try_failed = ["+foo", "+bar", "-baz"] async fn hide_try_build_started_comment_after_restart(pool: sqlx::PgPool) { run_test(pool, async |tester: &mut BorsTester| { tester.post_comment("@bors try").await?; - let comment = tester.get_comment(()).await?; + let comment = tester.get_next_comment(()).await?; // Hide the previous "Try build started" comment when we restart the build tester.post_comment("@bors try").await?; @@ -1085,4 +1085,32 @@ try_failed = ["+foo", "+bar", "-baz"] }) .await; } + + #[sqlx::test] + async fn update_try_build_started_comment_after_workflow_starts(pool: sqlx::PgPool) { + run_test(pool, async |tester: &mut BorsTester| { + tester.post_comment("@bors try").await?; + let comment = tester.get_next_comment(()).await?; + + tester.workflow_start(tester.try_branch().await).await?; + + // Check that the comment text has been updated with a link to the started workflow + let updated_comment = tester + .get_comment_by_node_id(&comment.node_id.unwrap()) + .await + .unwrap(); + insta::assert_snapshot!(updated_comment.content, @r" + :hourglass: Trying commit pr-1-sha with merge merge-0-pr-1… + + To cancel the try build, run the command `@bors try cancel`. + + **Workflows**: + + - https://github.com/rust-lang/borstest/actions/runs/1 + "); + + Ok(()) + }) + .await; + } } diff --git a/src/bors/handlers/workflow.rs b/src/bors/handlers/workflow.rs index 11229f6f..332443d1 100644 --- a/src/bors/handlers/workflow.rs +++ b/src/bors/handlers/workflow.rs @@ -1,5 +1,8 @@ +use super::trybuild::TRY_BRANCH_NAME; use crate::PgDbClient; -use crate::bors::comment::{build_failed_comment, try_build_succeeded_comment}; +use crate::bors::comment::{ + CommentTag, append_workflow_links_to_comment, build_failed_comment, try_build_succeeded_comment, +}; use crate::bors::event::{WorkflowRunCompleted, WorkflowRunStarted}; use crate::bors::handlers::labels::handle_label_trigger; use crate::bors::handlers::{BuildType, is_bors_observed_branch}; @@ -17,6 +20,7 @@ use std::sync::Arc; use std::time::Duration; pub(super) async fn handle_workflow_started( + repo: Arc, db: Arc, payload: WorkflowRunStarted, ) -> anyhow::Result<()> { @@ -53,14 +57,53 @@ pub(super) async fn handle_workflow_started( tracing::info!("Storing workflow started into DB"); db.create_workflow( &build, - payload.name, - payload.url, + payload.name.clone(), + payload.url.clone(), payload.run_id.into(), - payload.workflow_type, + payload.workflow_type.clone(), WorkflowStatus::Pending, ) .await?; + if build.branch == TRY_BRANCH_NAME { + add_workflow_links_to_try_build_start_comment(repo, db, &build, payload).await?; + } + + Ok(()) +} + +async fn add_workflow_links_to_try_build_start_comment( + repo: Arc, + db: Arc, + build: &BuildModel, + payload: WorkflowRunStarted, +) -> anyhow::Result<()> { + let Some(pr) = db.find_pr_by_build(build).await? else { + tracing::warn!("PR for build not found"); + return Ok(()); + }; + let comments = db + .get_tagged_bot_comments(&payload.repository, pr.number, CommentTag::TryBuildStarted) + .await?; + + let Some(try_build_comment) = comments.last() else { + tracing::warn!("No try build comment found for PR"); + return Ok(()); + }; + + let workflows = db.get_workflow_urls_for_build(build).await?; + + let mut comment_content = repo + .client + .get_comment_content(&try_build_comment.node_id) + .await?; + + append_workflow_links_to_comment(&mut comment_content, workflows); + + repo.client + .update_comment_content(&try_build_comment.node_id, &comment_content) + .await?; + Ok(()) } @@ -108,7 +151,6 @@ pub(super) async fn handle_workflow_completed( ) .await } - /// Attempt to complete a pending build after a workflow run has been completed. /// We assume that the status of the completed workflow run has already been updated in the /// database. @@ -553,7 +595,7 @@ mod tests { tester.workflow_full_success(w2).await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r#" :sunny: Try build successful - [Workflow1](https://github.com/rust-lang/borstest/actions/runs/1) :white_check_mark: @@ -583,7 +625,7 @@ mod tests { tester.workflow_event(WorkflowEvent::success(w1)).await?; tester.workflow_event(WorkflowEvent::success(w2)).await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r#" :sunny: Try build successful - [Workflow1](https://github.com/rust-lang/borstest/actions/runs/1) :white_check_mark: @@ -612,7 +654,7 @@ mod tests { tester.workflow_event(WorkflowEvent::failure(w2)).await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @":broken_heart: Test for merge-0-pr-1 failed: [Workflow1](https://github.com/rust-lang/borstest/actions/runs/1), [Workflow1](https://github.com/rust-lang/borstest/actions/runs/2)" ); Ok(()) @@ -639,7 +681,7 @@ min_ci_time = 10 .with_duration(Duration::from_secs(1)), ) .await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r" :broken_heart: Test for merge-0-pr-1 failed: [Workflow1](https://github.com/rust-lang/borstest/actions/runs/1) A workflow was considered to be a failure because it took only `1s`. The minimum duration for CI workflows is configured to be `10s`. "); @@ -666,7 +708,7 @@ min_ci_time = 20 .with_duration(Duration::from_secs(100)), ) .await?; - insta::assert_snapshot!(tester.get_comment_text(()).await?, @r#" + insta::assert_snapshot!(tester.get_next_comment_text(()).await?, @r#" :sunny: Try build successful ([Workflow1](https://github.com/rust-lang/borstest/actions/runs/1)) Build commit: merge-0-pr-1 (`merge-0-pr-1`, parent: `main-sha1`) diff --git a/src/bors/merge_queue.rs b/src/bors/merge_queue.rs index b61c2ec7..d298081e 100644 --- a/src/bors/merge_queue.rs +++ b/src/bors/merge_queue.rs @@ -520,7 +520,7 @@ merge_queue_enabled = false tester.approve(()).await?; tester.process_merge_queue().await; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @":hourglass: Testing commit pr-1-sha with merge merge-0-pr-1..." ); Ok(()) @@ -537,7 +537,7 @@ merge_queue_enabled = false tester.process_merge_queue().await; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r" :sunny: Test successful - [Workflow1](https://github.com/rust-lang/borstest/actions/runs/1) Approved by: `default-user` @@ -557,7 +557,7 @@ merge_queue_enabled = false tester.workflow_full_failure(tester.auto_branch().await).await?; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @":broken_heart: Test for merge-0-pr-1 failed: [Workflow1](https://github.com/rust-lang/borstest/actions/runs/1)" ); Ok(()) @@ -699,7 +699,7 @@ merge_queue_enabled = false tester.workflow_full_success(tester.auto_branch().await).await?; tester.process_merge_queue().await; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @":eyes: Test was successful, but fast-forwarding failed: this PR has conflicts with the `main` branch" ); Ok(()) @@ -720,7 +720,7 @@ merge_queue_enabled = false tester.workflow_full_success(tester.auto_branch().await).await?; tester.process_merge_queue().await; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @":eyes: Test was successful, but fast-forwarding failed: the tested commit was behind the `main` branch" ); Ok(()) @@ -744,7 +744,7 @@ merge_queue_enabled = false .await?; tester.process_merge_queue().await; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @":eyes: Test was successful, but fast-forwarding failed: IO error" ); Ok(()) @@ -816,7 +816,7 @@ merge_queue_enabled = false tester.approve(()).await?; tester.process_merge_queue().await; insta::assert_snapshot!( - tester.get_comment_text(()).await?, + tester.get_next_comment_text(()).await?, @r#" :lock: Merge conflict diff --git a/src/database/mod.rs b/src/database/mod.rs index ee99acec..4f6fd553 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -394,7 +394,7 @@ impl PullRequestModel { /// Describes whether a workflow is a Github Actions workflow or if it's a job from some external /// CI. -#[derive(Debug, PartialEq, sqlx::Type)] +#[derive(Debug, Clone, PartialEq, sqlx::Type)] #[sqlx(type_name = "TEXT")] #[sqlx(rename_all = "lowercase")] pub enum WorkflowType { diff --git a/src/database/operations.rs b/src/database/operations.rs index a1b60014..b8826531 100644 --- a/src/database/operations.rs +++ b/src/database/operations.rs @@ -1043,6 +1043,7 @@ pub(crate) async fn get_tagged_bot_comments( WHERE repository = $1 AND pr_number = $2 AND tag = $3 + ORDER BY created_at "#, repo as &GithubRepoName, pr_number.0 as i32, diff --git a/src/github/api/client.rs b/src/github/api/client.rs index d8f8f483..b7d372ac 100644 --- a/src/github/api/client.rs +++ b/src/github/api/client.rs @@ -457,12 +457,31 @@ impl GithubRepositoryClient { variables: V, } + #[derive(serde::Deserialize, Debug)] + struct Error { + #[allow(unused)] + message: String, + } + + #[derive(serde::Deserialize)] + struct RawResponse { + errors: Option>, + #[serde(flatten)] + result: T, + } + let response = self .client - .graphql::(&Payload { query, variables }) + .graphql::>(&Payload { query, variables }) .await .context("GraphQL request failed")?; - Ok(response) + + let errors = response.errors.unwrap_or_default(); + if !errors.is_empty() { + Err(anyhow::anyhow!("Query ended with error(s): {errors:?}")) + } else { + Ok(response.result) + } } /// Hides a comment on an Issue, Commit, Pull Request, or Gist. @@ -498,6 +517,89 @@ impl GithubRepositoryClient { .await?; Ok(()) } + + pub async fn get_comment_content(&self, node_id: &str) -> anyhow::Result { + const QUERY: &str = r#" + query($node_id: ID!) { + node(id: $node_id) { + ... on IssueComment { + body + } + } + } + "#; + + #[derive(Serialize)] + struct Variables<'a> { + node_id: &'a str, + } + + #[derive(Deserialize)] + struct Output { + data: OutputInner, + } + + #[derive(Deserialize)] + struct OutputInner { + node: Option, + } + + #[derive(Deserialize)] + struct IssueCommentNode { + body: String, + } + + tracing::debug!(node_id, "Fetching comment content"); + + let output = perform_retryable("get_comment_content", RetryMethod::default(), || async { + self.graphql::(QUERY, Variables { node_id }) + .await + .context("Failed to fetch comment content") + }) + .await?; + + match output.data.node { + Some(comment) => Ok(comment.body), + None => anyhow::bail!("No comment found for node_id: {node_id}"), + } + } + pub async fn update_comment_content( + &self, + node_id: &str, + new_body: &str, + ) -> anyhow::Result<()> { + const QUERY: &str = r#" + mutation($id: ID!, $body: String!) { + updateIssueComment(input: {id: $id, body: $body}) { + issueComment { + id + } + } + } + "#; + + #[derive(Serialize)] + struct Variables<'a> { + id: &'a str, + body: &'a str, + } + + tracing::debug!(node_id, "Updating comment content"); + + perform_retryable("update_comment_content", RetryMethod::default(), || async { + self.graphql::<(), Variables>( + QUERY, + Variables { + id: node_id, + body: new_body, + }, + ) + .await + .context("Failed to update comment") + }) + .await?; + Ok(()) + } } /// The reasons a piece of content can be reported or hidden. diff --git a/src/tests/mocks/comment.rs b/src/tests/mocks/comment.rs index da4f2800..5d760464 100644 --- a/src/tests/mocks/comment.rs +++ b/src/tests/mocks/comment.rs @@ -1,3 +1,8 @@ +use crate::github::api::client::HideCommentReason; +use crate::tests::mocks::User; +use crate::tests::mocks::pull_request::PrIdentifier; +use crate::tests::mocks::repository::GitHubRepository; +use crate::tests::mocks::user::GitHubUser; use chrono::Utc; use octocrab::models::events::payload::{IssueCommentEventAction, IssueCommentEventChanges}; use octocrab::models::issues::IssueStateReason; @@ -5,18 +10,14 @@ use octocrab::models::{Author, CommentId, IssueId, IssueState, Label}; use serde::Serialize; use url::Url; -use crate::tests::mocks::User; -use crate::tests::mocks::pull_request::PrIdentifier; -use crate::tests::mocks::repository::GitHubRepository; -use crate::tests::mocks::user::GitHubUser; - -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub struct Comment { pub pr_ident: PrIdentifier, pub author: User, pub content: String, pub id: Option, pub node_id: Option, + pub hide_reason: Option, } impl Comment { @@ -27,6 +28,7 @@ impl Comment { content: content.to_string(), id: None, node_id: None, + hide_reason: None, } } diff --git a/src/tests/mocks/github.rs b/src/tests/mocks/github.rs index fd00c02b..7ee9ecd7 100644 --- a/src/tests/mocks/github.rs +++ b/src/tests/mocks/github.rs @@ -8,9 +8,9 @@ use crate::create_github_client; use crate::github::api::client::HideCommentReason; use crate::tests::GitHubState; use crate::tests::mocks::app::{AppHandler, default_app_id}; +use crate::tests::mocks::dynamic_mock_req; use crate::tests::mocks::pull_request::CommentMsg; use crate::tests::mocks::repository::{mock_repo, mock_repo_list}; -use crate::tests::mocks::{HiddenComment, dynamic_mock_req}; pub struct GitHubMockServer { mock_server: MockServer, @@ -108,40 +108,97 @@ async fn mock_graphql(github: Arc>, mock_server: let query: Document<&str> = graphql_parser::parse_query(&body.query).expect("Could not parse GraphQL query"); let definition = query.definitions.into_iter().next().unwrap(); - let mutation = match definition { - Definition::Operation(OperationDefinition::Mutation(mutation)) => mutation, + let selection_set = match definition { + Definition::Operation(OperationDefinition::Mutation(mutation)) => { + mutation.selection_set + } + Definition::Operation(OperationDefinition::Query(query)) => query.selection_set, _ => panic!("Unexpected GraphQL query: {}", body.query), }; - let selection = mutation.selection_set.items.into_iter().next().unwrap(); + let selection = selection_set.items.into_iter().next().unwrap(); let operation = match selection { Selection::Field(field) => field, _ => panic!("Unexpected GraphQL selection"), }; - if operation.name == "minimizeComment" { - #[derive(serde::Deserialize)] - struct Variables { - node_id: String, - reason: HideCommentReason, + + match operation.name { + "minimizeComment" => { + #[derive(serde::Deserialize)] + struct Variables { + node_id: String, + reason: HideCommentReason, + } + + let github = github.clone(); + let data: Variables = serde_json::from_value(body.variables).unwrap(); + + // We have to use e.g. `blocking_lock` to lock from a sync function. + // It has to happen in a separate thread though. + std::thread::spawn(move || { + github + .blocking_lock() + .modify_comment(&data.node_id, |c| c.hide_reason = Some(data.reason)); + }) + .join() + .unwrap(); + ResponseTemplate::new(200).set_body_json(HashMap::::new()) } + "updateIssueComment" => { + #[derive(serde::Deserialize)] + struct Variables { + id: String, + body: String, + } + + let data: Variables = serde_json::from_value(body.variables).unwrap(); + let response = serde_json::json!({ + "issueComment": { + "id": data.id, + } + }); + + let github = github.clone(); + std::thread::spawn(move || { + github + .blocking_lock() + .modify_comment(&data.id, |c| c.content = data.body); + }) + .join() + .unwrap(); - let github = github.clone(); - let data: Variables = serde_json::from_value(body.variables).unwrap(); + ResponseTemplate::new(200).set_body_json(response) + } + // Get comment content + "node" => { + #[derive(serde::Deserialize)] + struct Variables { + node_id: String, + } - // We have to use e.g. `blocking_lock` to lock from a sync function. - // It has to happen in a separate thread though. - std::thread::spawn(move || { - github.blocking_lock().add_hidden_comment(HiddenComment { - node_id: data.node_id, - reason: data.reason, + let data: Variables = serde_json::from_value(body.variables).unwrap(); + + let github = github.clone(); + let comment_text = std::thread::spawn(move || { + github + .blocking_lock() + .get_comment_by_node_id(&data.node_id) + .unwrap() + .content + .clone() + }) + .join() + .unwrap(); + let response = serde_json::json!({ + "data": { + "node": { + "body": comment_text + } + } }); - }) - .join() - .unwrap(); - } else { - panic!("Unexpected operation {}", operation.name); + ResponseTemplate::new(200).set_body_json(response) + } + _ => panic!("Unexpected GraphQL operation {}", operation.name), } - - ResponseTemplate::new(200).set_body_json(HashMap::::new()) }, "POST", "^/graphql$".to_string(), diff --git a/src/tests/mocks/mod.rs b/src/tests/mocks/mod.rs index 26159cdc..903e506f 100644 --- a/src/tests/mocks/mod.rs +++ b/src/tests/mocks/mod.rs @@ -26,16 +26,10 @@ pub mod repository; pub mod user; pub mod workflow; -#[derive(Debug)] -pub struct HiddenComment { - pub node_id: String, - pub reason: HideCommentReason, -} - /// Represents the state of GitHub. pub struct GitHubState { pub(super) repos: HashMap>>, - hidden_comments: Vec, + comments: HashMap, } impl GitHubState { @@ -70,8 +64,8 @@ impl GitHubState { self } - pub fn add_hidden_comment(&mut self, comment: HiddenComment) { - self.hidden_comments.push(comment); + pub fn modify_comment(&mut self, node_id: &str, func: F) { + func(self.comments.get_mut(node_id).unwrap()); } pub fn check_sha_history(&self, repo: GithubRepoName, branch: &str, expected_shas: &[&str]) { @@ -103,7 +97,7 @@ impl GitHubState { /// events to arrive from the bors service. /// As such, it has to be written carefully to avoid holding GH/repo locks that are also /// acquired by dynamic HTTP mock handlers. - pub async fn get_comment>( + pub async fn get_next_comment>( state: Arc>, id: Id, ) -> anyhow::Result { @@ -166,16 +160,27 @@ impl GitHubState { { let mut gh_state = state.lock().await; - let repo = gh_state - .repos - .get_mut(&id.repo) - .unwrap_or_else(|| panic!("Repository `{}` not found", id.repo)); - let mut repo = repo.lock(); - let pr = repo - .pull_requests - .get_mut(&id.number) - .expect("Pull request not found"); - pr.add_comment_to_history(comment.clone()); + { + let repo = gh_state + .repos + .get_mut(&id.repo) + .unwrap_or_else(|| panic!("Repository `{}` not found", id.repo)); + let mut repo = repo.lock(); + let pr = repo + .pull_requests + .get_mut(&id.number) + .expect("Pull request not found"); + pr.add_comment_to_history(comment.clone()); + } + + assert_eq!( + gh_state.comments.insert( + comment.node_id.clone().expect("Comment without node ID"), + comment.clone() + ), + None, + "Duplicated comment {comment:?}" + ); } eprintln!( @@ -185,13 +190,19 @@ impl GitHubState { Ok(comment) } - pub fn check_hidden_comment(&self, comment: &Comment, reason: HideCommentReason) { - assert!( - self.hidden_comments - .iter() - .any(|c| &c.node_id == comment.node_id.as_ref().unwrap() && c.reason == reason), - "Comment {comment:?} was not hidden with reason {reason:?}.\nHidden comments: {:?}", - self.hidden_comments + pub fn get_comment_by_node_id(&self, node_id: &str) -> Option<&Comment> { + self.comments.get(node_id) + } + + pub fn check_comment_was_hidden(&self, node_id: &str, reason: HideCommentReason) { + let comment = self + .comments + .get(node_id) + .expect("Comment with node_id {node_id} was not modified through GraphQL"); + assert_eq!( + comment.hide_reason, + Some(reason), + "Comment {comment:?} was not hidden with reason {reason:?}." ); } } @@ -201,7 +212,7 @@ impl Default for GitHubState { let repo = Repo::default(); Self { repos: HashMap::from([(repo.name.clone(), Arc::new(Mutex::new(repo)))]), - hidden_comments: Default::default(), + comments: Default::default(), } } } diff --git a/src/tests/mocks/pull_request.rs b/src/tests/mocks/pull_request.rs index 1e735485..c75ac731 100644 --- a/src/tests/mocks/pull_request.rs +++ b/src/tests/mocks/pull_request.rs @@ -34,7 +34,7 @@ pub fn default_pr_number() -> u64 { /// - `()`, which uses the default repo and default PR number. /// - A PR number, which uses the default repository. /// - A tuple (, ). -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub struct PrIdentifier { pub repo: GithubRepoName, pub number: u64, diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 23ad882c..07df1b09 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -364,18 +364,21 @@ impl BorsTester { /// Wait until the next bot comment is received on the specified repo and PR, and return its /// text. - pub async fn get_comment_text>( + pub async fn get_next_comment_text>( &mut self, id: Id, ) -> anyhow::Result { - Ok(GitHubState::get_comment(self.github.clone(), id) + Ok(GitHubState::get_next_comment(self.github.clone(), id) .await? .content) } /// Wait until the next bot comment is received on the specified repo and PR, and return it. - pub async fn get_comment>(&mut self, id: Id) -> anyhow::Result { - GitHubState::get_comment(self.github.clone(), id).await + pub async fn get_next_comment>( + &mut self, + id: Id, + ) -> anyhow::Result { + GitHubState::get_next_comment(self.github.clone(), id).await } //-- Generation of GitHub events --// @@ -741,7 +744,7 @@ impl BorsTester { pub async fn start_auto_build>(&mut self, id: Id) -> anyhow::Result<()> { let id = id.into(); self.process_merge_queue().await; - let comment = self.get_comment_text(id).await?; + let comment = self.get_next_comment_text(id).await?; assert!(comment.contains("Testing commit")); Ok(()) } @@ -754,7 +757,7 @@ impl BorsTester { ) -> anyhow::Result<()> { self.workflow_full_success(self.auto_branch().await).await?; self.process_merge_queue().await; - let comment = self.get_comment_text(id).await?; + let comment = self.get_next_comment_text(id).await?; assert!(comment.contains("Test successful")); Ok(()) } @@ -775,7 +778,7 @@ impl BorsTester { let id = id.into(); for i in 0..count { let id = id.clone(); - self.get_comment_text(id) + self.get_next_comment_text(id) .await .unwrap_or_else(|_| panic!("Failed to get comment #{i}")); } @@ -786,7 +789,7 @@ impl BorsTester { self.github .lock() .await - .check_hidden_comment(comment, reason); + .check_comment_was_hidden(comment.node_id.as_deref().unwrap(), reason); } pub async fn expect_check_run( @@ -852,7 +855,7 @@ impl BorsTester { /// /// This method is useful if you execute a command that produces no comment as an output /// and you need to wait until it has been processed by bors. - /// Prefer using [BorsTester::expect_comments] or [BorsTester::get_comment_text] to synchronize + /// Prefer using [BorsTester::expect_comments] or [BorsTester::get_next_comment_text] to synchronize /// if you are waiting for a comment to be posted to a PR. pub async fn wait_for(&self, condition: F) -> anyhow::Result<()> where @@ -894,6 +897,15 @@ impl BorsTester { result } + /// Get a GitHub comment that might have been modified by API calls from bors. + pub async fn get_comment_by_node_id(&self, node_id: &str) -> Option { + self.github + .lock() + .await + .get_comment_by_node_id(node_id) + .cloned() + } + //-- Internal helper functions --/ async fn webhook_comment(&mut self, comment: Comment) -> anyhow::Result<()> { self.send_webhook(