Skip to content

Commit c5c43de

Browse files
authored
Merge pull request #187 from vohoanglong0107/refactor-centralize-comments
refactor: centralize build and workflow comments
2 parents ea91f11 + e079238 commit c5c43de

File tree

6 files changed

+181
-106
lines changed

6 files changed

+181
-106
lines changed

src/bors/comment.rs

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
use serde::Serialize;
22

3-
use crate::github::CommitSha;
3+
use crate::{
4+
database::{WorkflowModel, WorkflowStatus},
5+
github::CommitSha,
6+
};
47

58
/// A comment that can be posted to a pull request.
69
pub struct Comment {
@@ -34,13 +37,14 @@ impl Comment {
3437
}
3538
}
3639

37-
pub fn try_build_succeeded_comment(workflow_list: String, commit_sha: CommitSha) -> Comment {
40+
pub fn try_build_succeeded_comment(workflows: &[WorkflowModel], commit_sha: CommitSha) -> Comment {
41+
let workflows_status = list_workflows_status(workflows);
3842
Comment {
3943
text: format!(
4044
r#":sunny: Try build successful
4145
{}
4246
Build commit: {} (`{}`)"#,
43-
workflow_list, commit_sha, commit_sha
47+
workflows_status, commit_sha, commit_sha
4448
),
4549
metadata: Some(CommentMetadata::TryBuildCompleted {
4650
merge_sha: commit_sha.to_string(),
@@ -55,3 +59,51 @@ pub fn try_build_in_progress_comment() -> Comment {
5559
pub fn cant_find_last_parent_comment() -> Comment {
5660
Comment::new(":exclamation: There was no previous build. Please set an explicit parent or remove the `parent=last` argument to use the default parent.".to_string())
5761
}
62+
63+
pub fn no_try_build_in_progress_comment() -> Comment {
64+
Comment::new(":exclamation: There is currently no try build in progress.".to_string())
65+
}
66+
67+
pub fn unclean_try_build_cancelled_comment() -> Comment {
68+
Comment::new(
69+
"Try build was cancelled. It was not possible to cancel some workflows.".to_string(),
70+
)
71+
}
72+
73+
pub fn try_build_cancelled_comment(workflow_urls: impl Iterator<Item = String>) -> Comment {
74+
let mut try_build_cancelled_comment = r#"Try build cancelled.
75+
Cancelled workflows:"#
76+
.to_string();
77+
for url in workflow_urls {
78+
try_build_cancelled_comment += format!("\n- {}", url).as_str();
79+
}
80+
Comment::new(try_build_cancelled_comment)
81+
}
82+
83+
pub fn workflow_failed_comment(workflows: &[WorkflowModel]) -> Comment {
84+
let workflows_status = list_workflows_status(workflows);
85+
Comment::new(format!(
86+
r#":broken_heart: Test failed
87+
{}"#,
88+
workflows_status
89+
))
90+
}
91+
92+
fn list_workflows_status(workflows: &[WorkflowModel]) -> String {
93+
workflows
94+
.iter()
95+
.map(|w| {
96+
format!(
97+
"- [{}]({}) {}",
98+
w.name,
99+
w.url,
100+
if w.status == WorkflowStatus::Success {
101+
":white_check_mark:"
102+
} else {
103+
":x:"
104+
}
105+
)
106+
})
107+
.collect::<Vec<_>>()
108+
.join("\n")
109+
}

src/bors/handlers/refresh.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ async fn cancel_timed_out_builds(repo: &RepositoryState, db: &PgDbClient) -> any
4040
db.update_build_status(&build, BuildStatus::Cancelled)
4141
.await?;
4242
if let Some(pr) = db.find_pr_by_build(&build).await? {
43-
if let Err(error) = cancel_build_workflows(repo, db, &build).await {
43+
if let Err(error) = cancel_build_workflows(&repo.client, db, &build).await {
4444
tracing::error!(
4545
"Could not cancel workflows for SHA {}: {error:?}",
4646
build.commit_sha

src/bors/handlers/trybuild.rs

Lines changed: 97 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,16 @@ use anyhow::{anyhow, Context};
44

55
use crate::bors::command::Parent;
66
use crate::bors::comment::cant_find_last_parent_comment;
7+
use crate::bors::comment::no_try_build_in_progress_comment;
8+
use crate::bors::comment::try_build_cancelled_comment;
79
use crate::bors::comment::try_build_in_progress_comment;
10+
use crate::bors::comment::unclean_try_build_cancelled_comment;
811
use crate::bors::handlers::labels::handle_label_trigger;
912
use crate::bors::Comment;
1013
use crate::bors::RepositoryState;
1114
use crate::database::RunId;
12-
use crate::database::{BuildModel, BuildStatus, PullRequestModel, WorkflowStatus, WorkflowType};
15+
use crate::database::{BuildModel, BuildStatus, PullRequestModel};
16+
use crate::github::api::client::GithubRepositoryClient;
1317
use crate::github::GithubRepoName;
1418
use crate::github::{
1519
CommitSha, GithubUser, LabelTrigger, MergeError, PullRequest, PullRequestNumber,
@@ -44,6 +48,8 @@ pub(super) async fn command_try_build(
4448
return Ok(());
4549
}
4650

51+
// Create pr model based on CI repo, so we can retrieve the pr later when
52+
// the CI repo emits events
4753
let pr_model = db
4854
.get_or_create_pull_request(repo.client.repository(), pr.number)
4955
.await
@@ -62,66 +68,94 @@ pub(super) async fn command_try_build(
6268
}
6369
};
6470

71+
match attempt_merge(
72+
&repo.client,
73+
&pr.head.sha,
74+
&base_sha,
75+
&auto_merge_commit_message(pr, repo.client.repository(), "<try>", jobs),
76+
)
77+
.await?
78+
{
79+
MergeResult::Success(merge_sha) => {
80+
// If the merge was succesful, run CI with merged commit
81+
run_try_build(&repo.client, &db, pr_model, merge_sha.clone(), base_sha).await?;
82+
83+
handle_label_trigger(repo, pr.number, LabelTrigger::TryBuildStarted).await?;
84+
85+
repo.client
86+
.post_comment(pr.number, trying_build_comment(&pr.head.sha, &merge_sha))
87+
.await
88+
}
89+
MergeResult::Conflict => {
90+
repo.client
91+
.post_comment(pr.number, merge_conflict_comment(&pr.head.name))
92+
.await
93+
}
94+
}
95+
}
96+
97+
async fn attempt_merge(
98+
client: &GithubRepositoryClient,
99+
head_sha: &CommitSha,
100+
base_sha: &CommitSha,
101+
merge_message: &str,
102+
) -> anyhow::Result<MergeResult> {
65103
tracing::debug!("Attempting to merge with base SHA {base_sha}");
66104

67105
// First set the try branch to our base commit (either the selected parent or the main branch).
68-
repo.client
69-
.set_branch_to_sha(TRY_MERGE_BRANCH_NAME, &base_sha)
106+
client
107+
.set_branch_to_sha(TRY_MERGE_BRANCH_NAME, base_sha)
70108
.await
71109
.map_err(|error| anyhow!("Cannot set try merge branch to {}: {error:?}", base_sha.0))?;
72110

73111
// Then merge the PR commit into the try branch
74-
match repo
75-
.client
76-
.merge_branches(
77-
TRY_MERGE_BRANCH_NAME,
78-
&pr.head.sha,
79-
&auto_merge_commit_message(pr, repo.client.repository(), "<try>", jobs),
80-
)
112+
match client
113+
.merge_branches(TRY_MERGE_BRANCH_NAME, head_sha, merge_message)
81114
.await
82115
{
83116
Ok(merge_sha) => {
84117
tracing::debug!("Merge successful, SHA: {merge_sha}");
85-
// If the merge was succesful, then set the actual try branch that will run CI to the
86-
// merged commit.
87-
repo.client
88-
.set_branch_to_sha(TRY_BRANCH_NAME, &merge_sha)
89-
.await
90-
.map_err(|error| anyhow!("Cannot set try branch to main branch: {error:?}"))?;
91-
92-
db.attach_try_build(
93-
pr_model,
94-
TRY_BRANCH_NAME.to_string(),
95-
merge_sha.clone(),
96-
base_sha.clone(),
97-
)
98-
.await?;
99-
tracing::info!("Try build started");
100-
101-
handle_label_trigger(repo, pr.number, LabelTrigger::TryBuildStarted).await?;
102118

103-
let comment = Comment::new(format!(
104-
":hourglass: Trying commit {} with merge {}…",
105-
pr.head.sha.clone(),
106-
merge_sha
107-
));
108-
repo.client.post_comment(pr.number, comment).await?;
109-
Ok(())
119+
Ok(MergeResult::Success(merge_sha))
110120
}
111121
Err(MergeError::Conflict) => {
112122
tracing::warn!("Merge conflict");
113-
repo.client
114-
.post_comment(
115-
pr.number,
116-
Comment::new(merge_conflict_message(&pr.head.name)),
117-
)
118-
.await?;
119-
Ok(())
123+
124+
Ok(MergeResult::Conflict)
120125
}
121126
Err(error) => Err(error.into()),
122127
}
123128
}
124129

130+
async fn run_try_build(
131+
client: &GithubRepositoryClient,
132+
db: &PgDbClient,
133+
pr_model: PullRequestModel,
134+
commit_sha: CommitSha,
135+
parent_sha: CommitSha,
136+
) -> anyhow::Result<()> {
137+
client
138+
.set_branch_to_sha(TRY_BRANCH_NAME, &commit_sha)
139+
.await
140+
.map_err(|error| anyhow!("Cannot set try branch to main branch: {error:?}"))?;
141+
142+
db.attach_try_build(
143+
pr_model,
144+
TRY_BRANCH_NAME.to_string(),
145+
commit_sha,
146+
parent_sha,
147+
)
148+
.await?;
149+
150+
tracing::info!("Try build started");
151+
Ok(())
152+
}
153+
154+
enum MergeResult {
155+
Success(CommitSha),
156+
Conflict,
157+
}
158+
125159
fn get_base_sha(
126160
pr_model: &PullRequestModel,
127161
parent: Option<Parent>,
@@ -168,17 +202,12 @@ pub(super) async fn command_try_cancel(
168202
let Some(build) = get_pending_build(pr) else {
169203
tracing::warn!("No build found");
170204
repo.client
171-
.post_comment(
172-
pr_number,
173-
Comment::new(
174-
":exclamation: There is currently no try build in progress.".to_string(),
175-
),
176-
)
205+
.post_comment(pr_number, no_try_build_in_progress_comment())
177206
.await?;
178207
return Ok(());
179208
};
180209

181-
match cancel_build_workflows(repo, db.as_ref(), &build).await {
210+
match cancel_build_workflows(&repo.client, db.as_ref(), &build).await {
182211
Err(error) => {
183212
tracing::error!(
184213
"Could not cancel workflows for SHA {}: {error:?}",
@@ -187,30 +216,21 @@ pub(super) async fn command_try_cancel(
187216
db.update_build_status(&build, BuildStatus::Cancelled)
188217
.await?;
189218
repo.client
190-
.post_comment(
191-
pr_number,
192-
Comment::new(
193-
"Try build was cancelled. It was not possible to cancel some workflows."
194-
.to_string(),
195-
),
196-
)
219+
.post_comment(pr_number, unclean_try_build_cancelled_comment())
197220
.await?
198221
}
199222
Ok(workflow_ids) => {
200223
db.update_build_status(&build, BuildStatus::Cancelled)
201224
.await?;
202225
tracing::info!("Try build cancelled");
203226

204-
let mut try_build_cancelled_comment = r#"Try build cancelled.
205-
Cancelled workflows:"#
206-
.to_string();
207-
for id in workflow_ids {
208-
let url = repo.client.get_workflow_url(id);
209-
try_build_cancelled_comment += format!("\n- {}", url).as_str();
210-
}
211-
212227
repo.client
213-
.post_comment(pr_number, Comment::new(try_build_cancelled_comment))
228+
.post_comment(
229+
pr_number,
230+
try_build_cancelled_comment(
231+
repo.client.get_workflow_urls(workflow_ids.into_iter()),
232+
),
233+
)
214234
.await?
215235
}
216236
};
@@ -219,20 +239,14 @@ Cancelled workflows:"#
219239
}
220240

221241
pub async fn cancel_build_workflows(
222-
repo: &RepositoryState,
242+
client: &GithubRepositoryClient,
223243
db: &PgDbClient,
224244
build: &BuildModel,
225245
) -> anyhow::Result<Vec<RunId>> {
226-
let pending_workflows = db
227-
.get_workflows_for_build(build)
228-
.await?
229-
.into_iter()
230-
.filter(|w| w.status == WorkflowStatus::Pending && w.workflow_type == WorkflowType::Github)
231-
.map(|w| w.run_id)
232-
.collect::<Vec<_>>();
246+
let pending_workflows = db.get_pending_workflows_for_build(build).await?;
233247

234248
tracing::info!("Cancelling workflows {:?}", pending_workflows);
235-
repo.client.cancel_workflows(&pending_workflows).await?;
249+
client.cancel_workflows(&pending_workflows).await?;
236250
Ok(pending_workflows)
237251
}
238252

@@ -267,8 +281,14 @@ fn auto_merge_commit_message(
267281
message
268282
}
269283

270-
fn merge_conflict_message(branch: &str) -> String {
271-
format!(
284+
fn trying_build_comment(head_sha: &CommitSha, merge_sha: &CommitSha) -> Comment {
285+
Comment::new(format!(
286+
":hourglass: Trying commit {head_sha} with merge {merge_sha}…"
287+
))
288+
}
289+
290+
fn merge_conflict_comment(branch: &str) -> Comment {
291+
let message = format!(
272292
r#":lock: Merge conflict
273293
274294
This pull request and the master branch diverged in a way that cannot
@@ -298,7 +318,8 @@ handled during merge and rebase. This is normal, and you should still perform st
298318
299319
</details>
300320
"#
301-
)
321+
);
322+
Comment::new(message)
302323
}
303324

304325
async fn check_try_permissions(

0 commit comments

Comments
 (0)