Skip to content

Revert "Improve mergeableState recognition" #162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wilzbach wants to merge 1 commit into from
Closed

Revert "Improve mergeableState recognition" #162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 12 additions & 24 deletions source/dlangbot/cron.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,15 @@ auto detectInactiveStablePR(PRTuple t)

auto detectPRWithMergeConflicts(PRTuple t)
{
import std.typecons : Nullable;

if (t.pr.mergeable.isNull)
{
logInfo("[cron-daily/%s/%d/detectMerge]: mergeable is null.", t.pr.repoSlug, t.pr.number);
logInfo("[cron-daily/%s/%d]: detectMerge - mergeable is null.", t.pr.repoSlug, t.pr.number);
// repeat request to receive computed mergeable information
foreach (i; 0 .. 4)
{
import vibe.core.core : sleep;
t.config.waitAfterMergeNullState.sleep;
logInfo("[cron-daily/%s/%d/detectMerge]: repeating request", t.pr.repoSlug, t.pr.number);
logInfo("[cron-daily/%s/%d]: detectMerge - repeating request", t.pr.repoSlug, t.pr.number);
t.pr = t.pr.refresh;
if (!t.pr.mergeable.isNull)
goto mergable;
Expand All @@ -93,40 +91,30 @@ auto detectPRWithMergeConflicts(PRTuple t)
}
mergable:

Nullable!bool isMergeable;
bool isMergeable;
if (!t.pr.mergeableState.isNull)
{
logInfo("[cron-daily/%s/%d/detectMerge]: mergeableState = %s", t.pr.repoSlug, t.pr.number, t.pr.mergeableState.get);
logInfo("[cron-daily/%s/%d]: mergableState = %s", t.pr.repoSlug, t.pr.number, t.pr.mergeableState.get);
with (PullRequest.MergeableState)
final switch(t.pr.mergeableState)
{
case clean:
// branch is up to date with master and has no conflicts
isMergeable = true;
break;
case unstable:
// branch isn't up to date with master, but has no conflicts
case unknown, checking:
// should only be set if mergeable is null
return LabelResponse(LabelAction.none, "");
case clean, unstable:
isMergeable = true;
break;
case dirty:
// GitHub detected conflicts
isMergeable = false;
break;
case unknown, checking:
// should only be set if mergeable is null
// a reviewer has blocked the merge (not observed yet)
case blocked:
// the repo requires reviews and the PR hasn't been approved yet
// the repo requires status checks and they have failed
break;
return LabelResponse(LabelAction.none, "");
}
}

if (isMergeable.isNull)
else
{
if (t.pr.mergeable.isNull)
return LabelResponse(LabelAction.none, "");

logInfo("[cron-daily/%s/%d/detectMerge]: mergeable = %s", t.pr.repoSlug, t.pr.number, t.pr.mergeable.get);
logInfo("[cron-daily/%s/%d]: mergable = %s", t.pr.repoSlug, t.pr.number, t.pr.mergeable.get);
isMergeable = t.pr.mergeable.get;
}

Expand Down
48 changes: 11 additions & 37 deletions test/cronjob.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,6 @@ import std.stdio;

string[] repositories = ["dlang/phobos"];

void dontTestStalled(ref Json j)
{
import std.datetime : Clock, days;
j[$ - 1]["created_at"] = (Clock.currTime - 2.days).toISOExtString;
j[$ - 1]["updated_at"] = (Clock.currTime - 2.days).toISOExtString;
}

// test the first items of the cron job
unittest
{
Expand Down Expand Up @@ -69,13 +62,13 @@ unittest
// test that stalled isn't falsely removed (e.g. by recent labelling)
unittest
{
import std.datetime : Clock, days;
setAPIExpectations(
"/github/repos/dlang/phobos/issues?state=open&sort=updated&direction=asc", (ref Json j) {
// only test one pull request
j = Json([j[0]]);
},
"/github/repos/dlang/phobos/pulls/2526", (ref Json j) {
import std.datetime : Clock, days;
// simulate a recent label update
j["updated_at"] = (Clock.currTime - 2.days).toISOExtString;
},
Expand All @@ -95,6 +88,7 @@ unittest
// test that no label updates are sent if no activity was found
unittest
{
import std.datetime : Clock, days;
setAPIExpectations(
"/github/repos/dlang/phobos/issues?state=open&sort=updated&direction=asc", (ref Json j) {
// only test one pull request
Expand All @@ -104,7 +98,10 @@ unittest
j["mergeable"] = false;
},
"/github/repos/dlang/phobos/status/a04acd6a2813fb344d3e47369cf7fd64523ece44",
"/github/repos/dlang/phobos/issues/2526/comments", &dontTestStalled,
"/github/repos/dlang/phobos/issues/2526/comments", (ref Json j) {
j[$ - 1]["created_at"] = (Clock.currTime - 2.days).toISOExtString;
j[$ - 1]["updated_at"] = (Clock.currTime - 2.days).toISOExtString;
},
"/github/repos/dlang/phobos/pulls/2526/comments",
);

Expand All @@ -114,6 +111,7 @@ unittest
// test that the merge state gets refreshed
unittest
{
import std.datetime : Clock, days;
setAPIExpectations(
"/github/repos/dlang/phobos/issues?state=open&sort=updated&direction=asc", (ref Json j) {
// only test one pull request
Expand All @@ -127,39 +125,15 @@ unittest
j["mergeable_state"] = "dirty";
},
"/github/repos/dlang/phobos/status/a04acd6a2813fb344d3e47369cf7fd64523ece44",
"/github/repos/dlang/phobos/issues/2526/comments", &dontTestStalled,
"/github/repos/dlang/phobos/pulls/2526/comments",
"/github/repos/dlang/phobos/issues/2526/labels",
(scope HTTPServerRequest req, scope HTTPServerResponse res){
assert(req.method == HTTPMethod.PUT);
assert(req.json[].map!(e => e.get!string).equal(["blocked", "needs rebase"]));
},
);

testCronDaily(repositories);
}

// for "blocked" PRs, the `mergeable` attribute should be preferred
// if mergeable is true, "needs rebase" should be removed
unittest
{
setAPIExpectations(
"/github/repos/dlang/phobos/issues?state=open&sort=updated&direction=asc", (ref Json j) {
// only test one pull request
j = Json([j[0]]);
j[0]["labels"][0]["name"] = "needs rebase";
},
"/github/repos/dlang/phobos/pulls/2526", (ref Json j) {
j["mergeable"] = true;
j["mergeable_state"] = "blocked";
"/github/repos/dlang/phobos/issues/2526/comments", (ref Json j) {
j[$ - 1]["created_at"] = (Clock.currTime - 2.days).toISOExtString;
j[$ - 1]["updated_at"] = (Clock.currTime - 2.days).toISOExtString;
},
"/github/repos/dlang/phobos/status/a04acd6a2813fb344d3e47369cf7fd64523ece44",
"/github/repos/dlang/phobos/issues/2526/comments", &dontTestStalled,
"/github/repos/dlang/phobos/pulls/2526/comments",
"/github/repos/dlang/phobos/issues/2526/labels",
(scope HTTPServerRequest req, scope HTTPServerResponse res){
assert(req.method == HTTPMethod.PUT);
assert(req.json[].length == 0);
assert(req.json[].map!(e => e.get!string).equal(["blocked", "needs rebase"]));
},
);

Expand Down