Skip to content

Commit 9420439

Browse files
committed
Add tests for range-diff comment handler
1 parent 6ebc414 commit 9420439

File tree

1 file changed

+122
-18
lines changed

1 file changed

+122
-18
lines changed

src/handlers/check_commits/force_push_range_diff.rs

Lines changed: 122 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use anyhow::Context as _;
22

33
use crate::config::RangeDiffConfig;
44
use crate::db::issue_data::IssueData;
5+
use crate::github::CommitBase;
56
use crate::github::GithubCompare;
67
use crate::github::IssueRepository;
78
use crate::github::IssuesEvent;
@@ -35,25 +36,45 @@ pub(super) async fn handle_event(
3536
};
3637

3738
let base = event.issue.base.as_ref().context("no base ref")?;
38-
let org = event.repository.owner();
39-
let repo = event.repository.name();
39+
let issue_repo = IssueRepository {
40+
organization: event.repository.owner().to_string(),
41+
repository: event.repository.name().to_string(),
42+
};
4043

4144
let compare_before = ctx
4245
.github
43-
.compare(
44-
&IssueRepository {
45-
organization: org.to_string(),
46-
repository: repo.to_string(),
47-
},
48-
&base.sha,
49-
&before,
50-
)
46+
.compare(&issue_repo, &base.sha, &before)
5147
.await
5248
.context("failed to get the before compare")?;
5349

50+
if let Some(message) = changed_base_commit(
51+
host,
52+
&issue_repo,
53+
base,
54+
&compare_before,
55+
compare_after,
56+
before,
57+
after,
58+
) {
59+
// Rebase detected, post a comment linking to our range-diff.
60+
post_new_comment(ctx, event, message).await?;
61+
}
62+
63+
Ok(())
64+
}
65+
66+
fn changed_base_commit(
67+
host: &str,
68+
issue_repo: &IssueRepository,
69+
base: &CommitBase,
70+
compare_before: &GithubCompare,
71+
compare_after: &GithubCompare,
72+
oldhead: &str,
73+
newhead: &str,
74+
) -> Option<String> {
5475
// Does the merge_base_commits differs? No, not a force-push with rebase.
5576
if compare_before.merge_base_commit.sha == compare_after.merge_base_commit.sha {
56-
return Ok(());
77+
return None;
5778
}
5879

5980
let protocol = if host.starts_with("localhost:") {
@@ -63,19 +84,16 @@ pub(super) async fn handle_event(
6384
};
6485

6586
let branch = &base.git_ref;
66-
let (oldbase, oldhead) = (&compare_before.merge_base_commit.sha, before);
67-
let (newbase, newhead) = (&compare_after.merge_base_commit.sha, after);
87+
let oldbase = &compare_before.merge_base_commit.sha;
88+
let newbase = &compare_after.merge_base_commit.sha;
6889

6990
let message = format!(
70-
r#"This PR was rebased onto a different {branch} commit. Here's a [range-diff]({protocol}://{host}/gh-range-diff/{org}/{repo}/{oldbase}..{oldhead}/{newbase}..{newhead}) highlighting what actually changed.
91+
r#"This PR was rebased onto a different {branch} commit. Here's a [range-diff]({protocol}://{host}/gh-range-diff/{issue_repo}/{oldbase}..{oldhead}/{newbase}..{newhead}) highlighting what actually changed.
7192
7293
*Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.*"#
7394
);
7495

75-
// Rebase detected, post a comment linking to our range-diff.
76-
post_new_comment(ctx, event, message).await?;
77-
78-
Ok(())
96+
Some(message)
7997
}
8098

8199
async fn post_new_comment(
@@ -114,3 +132,89 @@ async fn post_new_comment(
114132

115133
Ok(())
116134
}
135+
136+
#[cfg(test)]
137+
mod tests {
138+
use crate::handlers::check_commits::dummy_commit_from_body;
139+
140+
use super::changed_base_commit;
141+
142+
#[test]
143+
fn unchanged_base_commit() {
144+
assert_eq!(
145+
changed_base_commit(
146+
"mytriagebot.com",
147+
&crate::github::IssueRepository {
148+
organization: "rust-lang".to_string(),
149+
repository: "rust".to_string()
150+
},
151+
&crate::github::CommitBase {
152+
sha: "sha-base-commit".to_string(),
153+
git_ref: "master".to_string(),
154+
repo: None
155+
},
156+
&crate::github::GithubCompare {
157+
base_commit: dummy_commit_from_body("base-commit-sha", "base-commit-body"),
158+
merge_base_commit: dummy_commit_from_body(
159+
"same-merge-commit",
160+
"merge-commit-body"
161+
),
162+
files: vec![]
163+
},
164+
&crate::github::GithubCompare {
165+
base_commit: dummy_commit_from_body("base-commit-sha", "base-commit-body"),
166+
merge_base_commit: dummy_commit_from_body(
167+
"same-merge-commit",
168+
"merge-commit-body"
169+
),
170+
files: vec![]
171+
},
172+
"oldhead",
173+
"newhead"
174+
),
175+
None
176+
);
177+
}
178+
179+
#[test]
180+
fn changed_base_commit_() {
181+
assert_eq!(
182+
changed_base_commit(
183+
"mytriagebot.com",
184+
&crate::github::IssueRepository {
185+
organization: "rust-lang".to_string(),
186+
repository: "rust".to_string()
187+
},
188+
&crate::github::CommitBase {
189+
sha: "sha-base-commit".to_string(),
190+
git_ref: "master".to_string(),
191+
repo: None
192+
},
193+
&crate::github::GithubCompare {
194+
base_commit: dummy_commit_from_body("base-commit-sha", "base-commit-body"),
195+
merge_base_commit: dummy_commit_from_body(
196+
"before-merge-commit",
197+
"merge-commit-body"
198+
),
199+
files: vec![]
200+
},
201+
&crate::github::GithubCompare {
202+
base_commit: dummy_commit_from_body("base-commit-sha", "base-commit-body"),
203+
merge_base_commit: dummy_commit_from_body(
204+
"after-merge-commit",
205+
"merge-commit-body"
206+
),
207+
files: vec![]
208+
},
209+
"oldhead",
210+
"newhead"
211+
),
212+
Some(
213+
r#"This PR was rebased onto a different master commit. Here's a [range-diff](https://mytriagebot.com/gh-range-diff/rust-lang/rust/before-merge-commit..oldhead/after-merge-commit..newhead) highlighting what actually changed.
214+
215+
*Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.*"#
216+
.to_string()
217+
)
218+
);
219+
}
220+
}

0 commit comments

Comments
 (0)