diff --git a/data/payloads/github.com_dlang_phobos_pull_4921.diff b/data/payloads/github.com_dlang_phobos_pull_4921.diff new file mode 100644 index 0000000..e69de29 diff --git a/source/dlangbot/app.d b/source/dlangbot/app.d index d104bd4..c8521a4 100644 --- a/source/dlangbot/app.d +++ b/source/dlangbot/app.d @@ -7,7 +7,7 @@ import dlangbot.trello; import dlangbot.utils; public import dlangbot.bugzilla : bugzillaLogin, bugzillaPassword, bugzillaURL; -public import dlangbot.github_api : githubAPIURL, githubAuth, githubHookSecret; +public import dlangbot.github_api : githubURL, githubAPIURL, githubAuth, githubHookSecret; public import dlangbot.trello : trelloAPIURL, trelloAuth, trelloSecret; public import dlangbot.twitter : oAuth, tweet, twitterURL, twitterEnabled; public import dlangbot.buildkite : buildkiteAPIURL, buildkiteAuth, buildkiteHookSecret, dlangbotAgentAuth; diff --git a/source/dlangbot/github.d b/source/dlangbot/github.d index 5d25ca0..237a47c 100644 --- a/source/dlangbot/github.d +++ b/source/dlangbot/github.d @@ -52,6 +52,13 @@ string markdownEscape(string desc) return app.data; } +string manualChangelogURL(string repoSlug) +{ + return "https://github.com/%s/blob/master/%s".format( + repoSlug, repoSlug == "dlang/dlang.org" ? "language-changelog" : "changelog", + ); +} + string formatComment(in ref PullRequest pr, in IssueRef[] refs, in Issue[] descs, in UserMessage[] msgs) { import std.array : appender; @@ -101,8 +108,8 @@ If you have addressed all reviews or aren't sure how to proceed, don't hesitate app.formattedWrite( `Your PR doesn't reference any Bugzilla issue. -If your PR contains non-trivial changes, please [reference a Bugzilla issue](https://github.com/dlang-bots/dlang-bot#automated-references) or create a [manual changelog](https://github.com/%s/blob/master/%s). -`, pr.repoSlug, pr.repoSlug == "dlang/dlang.org" ? "language-changelog" : "changelog"); +If your PR contains non-trivial changes, please [reference a Bugzilla issue](https://github.com/dlang-bots/dlang-bot#automated-references) or create a [manual changelog](%s). +`, manualChangelogURL(pr.repoSlug)); } if (msgs.length) diff --git a/source/dlangbot/github_api.d b/source/dlangbot/github_api.d index b85a461..9185ad1 100644 --- a/source/dlangbot/github_api.d +++ b/source/dlangbot/github_api.d @@ -1,5 +1,6 @@ module dlangbot.github_api; +shared string githubURL = "https://github.com"; shared string githubAPIURL = "https://api.github.com"; shared string githubAuth, githubHookSecret; @@ -241,6 +242,7 @@ struct PullRequest @name("created_at") SysTime createdAt; @name("updated_at") SysTime updatedAt; @name("closed_at") Nullable!SysTime closedAt; + @name("diff_url") string diffURL; bool locked; // TODO: update payloads //@name("maintainer_can_modify") bool maintainerCanModify; diff --git a/source/dlangbot/warnings.d b/source/dlangbot/warnings.d index fd313ea..7c1a09a 100644 --- a/source/dlangbot/warnings.d +++ b/source/dlangbot/warnings.d @@ -1,7 +1,7 @@ module dlangbot.warnings; import dlangbot.bugzilla : Issue, IssueRef; -import dlangbot.github : PullRequest; +import dlangbot.github : PullRequest, manualChangelogURL; import std.algorithm; @@ -14,9 +14,26 @@ struct UserMessage } -// check diff length -void checkDiff(in ref PullRequest pr, ref UserMessage[] msgs) +void checkEnhancementChangelog(in ref PullRequest pr, ref UserMessage[] msgs, + in Issue[] bugzillaIssues, in IssueRef[] refs) { + import dlangbot.utils : request, expectOK; + import vibe.stream.operations : readAllUTF8; + import std.format : format; + + if (bugzillaIssues.any!(i => i.status.among("NEW", "ASSIGNED", "REOPENED") && + i.severity == "enhancement" && + refs.canFind!(r => r.id == i.id && r.fixed))) + { + auto diff = request(pr.diffURL).expectOK.bodyReader.readAllUTF8; + if (!diff.canFind("\n+++ b/changelog/")) + { + msgs ~= UserMessage(UserMessage.Type.Warning, + "Pull requests implementing enhancements should include a [full changelog entry](%s)." + .format(manualChangelogURL(pr.repoSlug)) + ); + } + } } @@ -55,8 +72,8 @@ git rebase --onto upstream/stable upstream/master UserMessage[] checkForWarnings(in PullRequest pr, in Issue[] bugzillaIssues, in IssueRef[] refs) { UserMessage[] msgs; - pr.checkDiff(msgs); pr.checkBugzilla(msgs, bugzillaIssues, refs); + pr.checkEnhancementChangelog(msgs, bugzillaIssues, refs); return msgs; } diff --git a/test/bugzilla.d b/test/bugzilla.d index 8a94664..0c4cc33 100644 --- a/test/bugzilla.d +++ b/test/bugzilla.d @@ -225,7 +225,7 @@ unittest assert("keywords" in req.json["params"][0]); auto comment = req.json["params"][0]["comment"]["body"].get!string; - enum expected = q"EOF + auto expected = q"EOF @MartinNowak created dlang/dmd pull request #6359 "fix Issue 16794 - dmd not working on Ubuntu 16.10" fixing this issue: - fix Issue 16794 - dmd not working on Ubuntu 16.10 @@ -235,7 +235,7 @@ unittest - also see https://github.com/dlang/installer/pull/207 https://github.com/dlang/dmd/pull/6359 -EOF".chomp; +EOF".chomp.replace("https://github.com/dlang/installer", githubURL ~ "/dlang/installer"); // compensate for API reference rewriting assert(comment == expected, comment); auto j = Json(["error" : Json(null), "result" : Json.emptyObject]); diff --git a/test/comments.d b/test/comments.d index d90818f..bebacaa 100644 --- a/test/comments.d +++ b/test/comments.d @@ -131,6 +131,7 @@ unittest 8573,"A simpler Phobos function that returns the index of the mix or max item","NEW","---","enhancement","P2",`); }, "/github/repos/dlang/phobos/issues/4921/labels", + "/github.com/dlang/phobos/pull/4921.diff", "/github/repos/dlang/phobos/issues/4921/labels", (ref Json j) { j = Json.emptyArray; }, @@ -169,6 +170,7 @@ unittest 8574,"Some Bug","NEW","---","normal","P2",`); }, "/github/repos/dlang/phobos/issues/4921/labels", + "/github.com/dlang/phobos/pull/4921.diff", "/github/repos/dlang/phobos/issues/4921/labels", (ref Json j) { j = Json.emptyArray; }, @@ -200,6 +202,7 @@ unittest "/github/repos/dlang/phobos/issues/4921/comments", "/bugzilla/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority,keywords", "/github/repos/dlang/phobos/issues/4921/labels", + "/github.com/dlang/phobos/pull/4921.diff", "/github/repos/dlang/phobos/issues/4921/labels", (ref Json j) { j[0]["name"] = "Enhancement"; }, @@ -558,3 +561,89 @@ unittest assert("a\\b".markdownEscape == "a\\\\b"); assert("a+b".markdownEscape == r"a\+b"); } + +@("enhancement-with-changelog") +unittest +{ + setAPIExpectations( + "/github/repos/dlang/phobos/pulls/4921/commits", (ref Json j) { + j[0]["commit"]["message"] = "Fix Issue 8573"; + }, + "/github/repos/dlang/phobos/issues/4921/comments", + "/bugzilla/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority,keywords", + (scope HTTPServerRequest req, scope HTTPServerResponse res){ + res.writeBody( +`bug_id,"short_desc","bug_status","resolution","bug_severity","priority","keywords" +8573,"A simpler Phobos function that returns the index of the mix or max item","NEW","---","enhancement","P2",`); + }, + "/github/repos/dlang/phobos/issues/4921/labels", + "/github.com/dlang/phobos/pull/4921.diff", (scope HTTPServerRequest req, scope HTTPServerResponse res){ + res.writeBody(q"EOF +diff --git a/changelog/bla-bla.dd b/changelog/bla-bla.dd +new file mode 100644 +index 00000000000..0147f501f08 +--- /dev/null ++++ b/changelog/bla-bla.dd +@@ -0,0 +1,1 @@ ++bla bla +EOF"); + }, + "/github/repos/dlang/phobos/issues/4921/labels", (ref Json j) { + j = Json.emptyArray; + }, + "/github/orgs/dlang/public_members?per_page=100", + "/github/repos/dlang/phobos/issues/comments/262784442", + (scope HTTPServerRequest req, scope HTTPServerResponse res){ + assert(req.method == HTTPMethod.PATCH); + assert(!req.json["body"].get!string.canFind("Pull requests implementing enhancements should include")); + res.writeVoidBody; + }, + "/github/repos/dlang/phobos/issues/4921/labels", + (scope HTTPServerRequest req, scope HTTPServerResponse res){ + assert(req.json[].equal(["Enhancement"])); + }, + "/trello/1/search?query=name:%22Issue%208573%22&"~trelloAuth, + "/bugzilla/jsonrpc.cgi", // Bug.comments + "/bugzilla/jsonrpc.cgi", // Bug.update + ); + + postGitHubHook("dlang_phobos_synchronize_4921.json"); +} + +@("enhancement-with-changelog") +unittest +{ + setAPIExpectations( + "/github/repos/dlang/phobos/pulls/4921/commits", (ref Json j) { + j[0]["commit"]["message"] = "Fix Issue 8573"; + }, + "/github/repos/dlang/phobos/issues/4921/comments", + "/bugzilla/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority,keywords", + (scope HTTPServerRequest req, scope HTTPServerResponse res){ + res.writeBody( +`bug_id,"short_desc","bug_status","resolution","bug_severity","priority","keywords" +8573,"A simpler Phobos function that returns the index of the mix or max item","NEW","---","enhancement","P2",`); + }, + "/github/repos/dlang/phobos/issues/4921/labels", + "/github.com/dlang/phobos/pull/4921.diff", + "/github/repos/dlang/phobos/issues/4921/labels", (ref Json j) { + j = Json.emptyArray; + }, + "/github/orgs/dlang/public_members?per_page=100", + "/github/repos/dlang/phobos/issues/comments/262784442", + (scope HTTPServerRequest req, scope HTTPServerResponse res){ + assert(req.method == HTTPMethod.PATCH); + assert(req.json["body"].get!string.canFind("Pull requests implementing enhancements should include")); + res.writeVoidBody; + }, + "/github/repos/dlang/phobos/issues/4921/labels", + (scope HTTPServerRequest req, scope HTTPServerResponse res){ + assert(req.json[].equal(["Enhancement"])); + }, + "/trello/1/search?query=name:%22Issue%208573%22&"~trelloAuth, + "/bugzilla/jsonrpc.cgi", // Bug.comments + "/bugzilla/jsonrpc.cgi", // Bug.update + ); + + postGitHubHook("dlang_phobos_synchronize_4921.json"); +} diff --git a/test/utils.d b/test/utils.d index 4535e33..b72e155 100644 --- a/test/utils.d +++ b/test/utils.d @@ -83,6 +83,7 @@ void startFakeAPIServer() auto fakeAPIServerURL = "http://" ~ fakeSettings.bindAddresses[0] ~ ":" ~ fakeSettings.port.to!string; + githubURL = fakeAPIServerURL ~ "/github.com"; githubAPIURL = fakeAPIServerURL ~ "/github"; trelloAPIURL = fakeAPIServerURL ~ "/trello"; buildkiteAPIURL = fakeAPIServerURL ~ "/buildkite"; @@ -156,9 +157,10 @@ auto payloadServer(scope HTTPServerRequest req, scope HTTPServerResponse res) { logInfo("reading payload: %s", filePath); auto payload = filePath.readText; - if (req.requestURL.startsWith("/github", "/trello", "/buildkite", "/hcloud")) + if (req.requestURL.startsWith("/github/", "/trello/", "/buildkite/", "/hcloud/")) { auto payloadJson = payload.parseJsonString; + replaceAPIReferences("https://github.com", githubURL, payloadJson); replaceAPIReferences("https://api.github.com", githubAPIURL, payloadJson); replaceAPIReferences("https://api.trello.com", trelloAPIURL, payloadJson); replaceAPIReferences("https://api.buildkite.com/v2", buildkiteAPIURL, payloadJson); @@ -175,6 +177,11 @@ auto payloadServer(scope HTTPServerRequest req, scope HTTPServerResponse res) } } +void replaceAPIReferences(string official, string local, ref string str) +{ + str = str.replace(official, local); +} + void replaceAPIReferences(string official, string local, ref Json json) { void recursiveReplace(ref Json j) @@ -188,7 +195,10 @@ void replaceAPIReferences(string official, string local, ref Json json) case Json.Type.string: string v = j.get!string; if (v.countUntil(official) >= 0) - j = v.replace(official, local); + { + replaceAPIReferences(official, local, v); + j = v; + } break; default: break; @@ -294,19 +304,23 @@ void postGitHubHook(string payload, string eventType = "pull_request", auto req = requestHTTP(ghTestHookURL, (scope req) { req.method = HTTPMethod.POST; - auto payload = payload.readText.parseJsonString; + auto payload = payload.readText; // localize accessed URLs + replaceAPIReferences("https://github.com", githubURL, payload); replaceAPIReferences("https://api.github.com", githubAPIURL, payload); req.headers["X-GitHub-Event"] = eventType; if (postprocess !is null) - postprocess(payload, req); + { + auto payloadJson = payload.parseJsonString; + postprocess(payloadJson, req); + payload = payloadJson.toString; + } - auto respStr = payload.toString; - req.headers["X-Hub-Signature"] = getSignature(respStr); - req.writeBody(cast(ubyte[]) respStr); + req.headers["X-Hub-Signature"] = getSignature(payload); + req.writeBody(cast(ubyte[]) payload); }); assert(req.statusCode == 200, "Request failed with status %d. Response body:\n\n%s"