Skip to content

Commit fb97301

Browse files
KobzolKobzol
authored
Merge pull request #261 from rust-lang/simplify-wait-for
Add helper methods for waiting for a PR condition
2 parents 811e09a + b5afe63 commit fb97301

File tree

4 files changed

+65
-128
lines changed

4 files changed

+65
-128
lines changed

‎src/bors/handlers/info.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,7 @@ mod tests {
105105
run_test(pool, |mut tester| async {
106106
tester.post_comment("@bors p=5").await?;
107107
tester
108-
.wait_for(|| async {
109-
let Some(pr) = tester.default_pr_db().await? else {
110-
return Ok(false);
111-
};
112-
Ok(pr.priority == Some(5))
113-
})
108+
.wait_for_default_pr(|pr| pr.priority == Some(5))
114109
.await?;
115110

116111
tester.post_comment("@bors info").await?;

‎src/bors/handlers/pr_events.rs

Lines changed: 24 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -310,12 +310,9 @@ mod tests {
310310
run_test(pool, |mut tester| async {
311311
let pr = tester.open_pr(default_repo_name(), false).await?;
312312
tester
313-
.wait_for(|| async {
314-
let Some(pr) = tester.pr_db(default_repo_name(), pr.number.0).await? else {
315-
return Ok(false);
316-
};
317-
Ok(pr.base_branch == *default_branch_name()
318-
&& pr.pr_status == PullRequestStatus::Open)
313+
.wait_for_pr(default_repo_name(), pr.number.0, |pr| {
314+
pr.base_branch == *default_branch_name()
315+
&& pr.pr_status == PullRequestStatus::Open
319316
})
320317
.await?;
321318
Ok(tester)
@@ -333,11 +330,8 @@ mod tests {
333330
})
334331
.await?;
335332
tester
336-
.wait_for(|| async {
337-
let Some(pr) = tester.default_pr_db().await? else {
338-
return Ok(false);
339-
};
340-
Ok(pr.base_branch == "foo")
333+
.wait_for_pr(default_repo_name(), default_pr_number(), |pr| {
334+
pr.base_branch == "foo"
341335
})
342336
.await?;
343337
Ok(tester)
@@ -354,12 +348,7 @@ mod tests {
354348
})
355349
.await?;
356350
tester
357-
.wait_for(|| async {
358-
let Some(pr) = tester.default_pr_db().await? else {
359-
return Ok(false);
360-
};
361-
Ok(pr.mergeable_state == MergeableState::HasConflicts)
362-
})
351+
.wait_for_default_pr(|pr| pr.mergeable_state == MergeableState::HasConflicts)
363352
.await?;
364353
Ok(tester)
365354
})
@@ -371,29 +360,20 @@ mod tests {
371360
run_test(pool, |mut tester| async {
372361
let pr = tester.open_pr(default_repo_name(), false).await?;
373362
tester
374-
.wait_for(|| async {
375-
let Some(pr) = tester.pr_db(default_repo_name(), pr.number.0).await? else {
376-
return Ok(false);
377-
};
378-
Ok(pr.pr_status == PullRequestStatus::Open)
363+
.wait_for_pr(default_repo_name(), pr.number.0, |pr| {
364+
pr.pr_status == PullRequestStatus::Open
379365
})
380366
.await?;
381367
tester.close_pr(default_repo_name(), pr.number.0).await?;
382368
tester
383-
.wait_for(|| async {
384-
let Some(pr) = tester.pr_db(default_repo_name(), pr.number.0).await? else {
385-
return Ok(false);
386-
};
387-
Ok(pr.pr_status == PullRequestStatus::Closed)
369+
.wait_for_pr(default_repo_name(), pr.number.0, |pr| {
370+
pr.pr_status == PullRequestStatus::Closed
388371
})
389372
.await?;
390373
tester.reopen_pr(default_repo_name(), pr.number.0).await?;
391374
tester
392-
.wait_for(|| async {
393-
let Some(pr) = tester.pr_db(default_repo_name(), pr.number.0).await? else {
394-
return Ok(false);
395-
};
396-
Ok(pr.pr_status == PullRequestStatus::Open)
375+
.wait_for_pr(default_repo_name(), pr.number.0, |pr| {
376+
pr.pr_status == PullRequestStatus::Open
397377
})
398378
.await?;
399379
Ok(tester)
@@ -406,22 +386,16 @@ mod tests {
406386
run_test(pool, |mut tester| async {
407387
let pr = tester.open_pr(default_repo_name(), true).await?;
408388
tester
409-
.wait_for(|| async {
410-
let Some(pr) = tester.pr_db(default_repo_name(), pr.number.0).await? else {
411-
return Ok(false);
412-
};
413-
Ok(pr.pr_status == PullRequestStatus::Draft)
389+
.wait_for_pr(default_repo_name(), pr.number.0, |pr| {
390+
pr.pr_status == PullRequestStatus::Draft
414391
})
415392
.await?;
416393
tester
417394
.ready_for_review(default_repo_name(), pr.number.0)
418395
.await?;
419396
tester
420-
.wait_for(|| async {
421-
let Some(pr) = tester.pr_db(default_repo_name(), pr.number.0).await? else {
422-
return Ok(false);
423-
};
424-
Ok(pr.pr_status == PullRequestStatus::Open)
397+
.wait_for_pr(default_repo_name(), pr.number.0, |pr| {
398+
pr.pr_status == PullRequestStatus::Open
425399
})
426400
.await?;
427401
Ok(tester)
@@ -434,22 +408,16 @@ mod tests {
434408
run_test(pool, |mut tester| async {
435409
let pr = tester.open_pr(default_repo_name(), false).await?;
436410
tester
437-
.wait_for(|| async {
438-
let Some(pr) = tester.pr_db(default_repo_name(), pr.number.0).await? else {
439-
return Ok(false);
440-
};
441-
Ok(pr.pr_status == PullRequestStatus::Open)
411+
.wait_for_pr(default_repo_name(), pr.number.0, |pr| {
412+
pr.pr_status == PullRequestStatus::Open
442413
})
443414
.await?;
444415
tester
445416
.convert_to_draft(default_repo_name(), pr.number.0)
446417
.await?;
447418
tester
448-
.wait_for(|| async {
449-
let Some(pr) = tester.pr_db(default_repo_name(), pr.number.0).await? else {
450-
return Ok(false);
451-
};
452-
Ok(pr.pr_status == PullRequestStatus::Draft)
419+
.wait_for_pr(default_repo_name(), pr.number.0, |pr| {
420+
pr.pr_status == PullRequestStatus::Draft
453421
})
454422
.await?;
455423
Ok(tester)
@@ -462,20 +430,14 @@ mod tests {
462430
run_test(pool, |mut tester| async {
463431
let pr = tester.open_pr(default_repo_name(), false).await?;
464432
tester
465-
.wait_for(|| async {
466-
let Some(pr) = tester.pr_db(default_repo_name(), pr.number.0).await? else {
467-
return Ok(false);
468-
};
469-
Ok(pr.pr_status == PullRequestStatus::Open)
433+
.wait_for_pr(default_repo_name(), pr.number.0, |pr| {
434+
pr.pr_status == PullRequestStatus::Open
470435
})
471436
.await?;
472437
tester.merge_pr(default_repo_name(), pr.number.0).await?;
473438
tester
474-
.wait_for(|| async {
475-
let Some(pr) = tester.pr_db(default_repo_name(), pr.number.0).await? else {
476-
return Ok(false);
477-
};
478-
Ok(pr.pr_status == PullRequestStatus::Merged)
439+
.wait_for_pr(default_repo_name(), pr.number.0, |pr| {
440+
pr.pr_status == PullRequestStatus::Merged
479441
})
480442
.await?;
481443
Ok(tester)

‎src/bors/handlers/review.rs

Lines changed: 10 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -456,12 +456,7 @@ mod tests {
456456
run_test(pool, |mut tester| async {
457457
tester.post_comment("@bors p=5").await?;
458458
tester
459-
.wait_for(|| async {
460-
let Some(pr) = tester.default_pr_db().await? else {
461-
return Ok(false);
462-
};
463-
Ok(pr.priority == Some(5))
464-
})
459+
.wait_for_default_pr(|pr| pr.priority == Some(5))
465460
.await?;
466461
Ok(tester)
467462
})
@@ -473,12 +468,7 @@ mod tests {
473468
run_test(pool, |mut tester| async {
474469
tester.post_comment("@bors p=5").await?;
475470
tester
476-
.wait_for(|| async {
477-
let Some(pr) = tester.default_pr_db().await? else {
478-
return Ok(false);
479-
};
480-
Ok(pr.priority == Some(5))
481-
})
471+
.wait_for_default_pr(|pr| pr.priority == Some(5))
482472
.await?;
483473

484474
tester.post_comment("@bors r+").await?;
@@ -496,12 +486,7 @@ mod tests {
496486
run_test(pool, |mut tester| async {
497487
tester.post_comment("@bors p=5").await?;
498488
tester
499-
.wait_for(|| async {
500-
let Some(pr) = tester.default_pr_db().await? else {
501-
return Ok(false);
502-
};
503-
Ok(pr.priority == Some(5))
504-
})
489+
.wait_for_default_pr(|pr| pr.priority == Some(5))
505490
.await?;
506491

507492
tester.post_comment("@bors r+ p=10").await?;
@@ -660,12 +645,7 @@ mod tests {
660645

661646
tester.post_comment("@bors p=7").await?;
662647
tester
663-
.wait_for(|| async {
664-
let Some(pr) = tester.default_pr_db().await? else {
665-
return Ok(false);
666-
};
667-
Ok(pr.priority == Some(7))
668-
})
648+
.wait_for_default_pr(|pr| pr.priority == Some(7))
669649
.await?;
670650

671651
Ok(tester)
@@ -708,12 +688,7 @@ mod tests {
708688
.post_comment(review_comment("@bors delegate-"))
709689
.await?;
710690
tester
711-
.wait_for(|| async {
712-
let Some(pr) = tester.default_pr_db().await? else {
713-
return Ok(false);
714-
};
715-
Ok(pr.delegated_permission.is_none())
716-
})
691+
.wait_for_default_pr(|pr| pr.delegated_permission.is_none())
717692
.await?;
718693

719694
Ok(tester)
@@ -733,12 +708,7 @@ mod tests {
733708

734709
tester.post_comment("@bors delegate-").await?;
735710
tester
736-
.wait_for(|| async {
737-
let Some(pr) = tester.default_pr_db().await? else {
738-
return Ok(false);
739-
};
740-
Ok(pr.delegated_permission.is_none())
741-
})
711+
.wait_for_default_pr(|pr| pr.delegated_permission.is_none())
742712
.await?;
743713

744714
Ok(tester)
@@ -929,12 +899,7 @@ mod tests {
929899
run_test(pool, |mut tester| async {
930900
tester.post_comment("@bors rollup").await?;
931901
tester
932-
.wait_for(|| async {
933-
let Some(pr) = tester.default_pr_db().await? else {
934-
return Ok(false);
935-
};
936-
Ok(pr.rollup == Some(RollupMode::Always))
937-
})
902+
.wait_for_default_pr(|pr| pr.rollup == Some(RollupMode::Always))
938903
.await?;
939904
Ok(tester)
940905
})
@@ -946,12 +911,7 @@ mod tests {
946911
run_test(pool, |mut tester| async {
947912
tester.post_comment("@bors rollup=maybe").await?;
948913
tester
949-
.wait_for(|| async {
950-
let Some(pr) = tester.default_pr_db().await? else {
951-
return Ok(false);
952-
};
953-
Ok(pr.rollup == Some(RollupMode::Maybe))
954-
})
914+
.wait_for_default_pr(|pr| pr.rollup == Some(RollupMode::Maybe))
955915
.await?;
956916
Ok(tester)
957917
})
@@ -963,12 +923,7 @@ mod tests {
963923
run_test(pool, |mut tester| async {
964924
tester.post_comment("@bors rollup").await?;
965925
tester
966-
.wait_for(|| async {
967-
let Some(pr) = tester.default_pr_db().await? else {
968-
return Ok(false);
969-
};
970-
Ok(pr.rollup == Some(RollupMode::Always))
971-
})
926+
.wait_for_default_pr(|pr| pr.rollup == Some(RollupMode::Always))
972927
.await?;
973928

974929
tester.post_comment("@bors r+").await?;
@@ -990,12 +945,7 @@ mod tests {
990945
run_test(pool, |mut tester| async {
991946
tester.post_comment("@bors rollup=never").await?;
992947
tester
993-
.wait_for(|| async {
994-
let Some(pr) = tester.default_pr_db().await? else {
995-
return Ok(false);
996-
};
997-
Ok(pr.rollup == Some(RollupMode::Never))
998-
})
948+
.wait_for_default_pr(|pr| pr.rollup == Some(RollupMode::Never))
999949
.await?;
1000950

1001951
tester.post_comment("@bors r+ rollup").await?;

‎src/tests/mocks/bors.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,36 @@ impl BorsTester {
169169
self.pr_db(default_repo_name(), default_pr_number()).await
170170
}
171171

172+
/// Wait until a pull request is in the database and satisfies a given condition.
173+
///
174+
/// This is a convenience wrapper around `wait_for` that simplifies checking for PR conditions.
175+
pub async fn wait_for_pr<F>(
176+
&self,
177+
repo: GithubRepoName,
178+
pr_number: u64,
179+
condition: F,
180+
) -> anyhow::Result<()>
181+
where
182+
F: Fn(&PullRequestModel) -> bool,
183+
{
184+
self.wait_for(|| async {
185+
let Some(pr) = self.pr_db(repo.clone(), pr_number).await? else {
186+
return Ok(false);
187+
};
188+
Ok(condition(&pr))
189+
})
190+
.await
191+
}
192+
193+
/// Wait until the default pull request is in the database and satisfies a given condition.
194+
pub async fn wait_for_default_pr<F>(&self, condition: F) -> anyhow::Result<()>
195+
where
196+
F: Fn(&PullRequestModel) -> bool,
197+
{
198+
self.wait_for_pr(default_repo_name(), default_pr_number(), condition)
199+
.await
200+
}
201+
172202
pub fn create_branch(&mut self, name: &str) -> MappedMutexGuard<RawMutex, Branch> {
173203
// We cannot clone the Arc, otherwise it won't work
174204
let repo = self.github.repos.get(&default_repo_name()).unwrap();

0 commit comments

Comments
 (0)