Automatically merge PRs with auto-merge #15

wilzbach · 2016-12-18T17:15:22Z

No description provided.

… std.regex

wilzbach

.

wilzbach · 2016-12-18T17:18:15Z

source/app.d

+        enum splitRE = regex(`[^\d]+`); // ctRegex throws a weird error in unittest compilation
        auto closed = !m.captures[1].empty;
-        return m.captures[5].stripRight.splitter(ctRegex!`[^\d]+`)
+        return m.captures[5].stripRight.splitter(splitRE)


With ctRegex and the unittest below, I was getting the following error:
However I couldn't reproduce this in an isolated case :/

m.captures[5] = ["16319", ""];

core.exception.AssertError@/usr/include/dlang/dmd/std/regex/package.d(563): wrong match: 9..4 ---------------- ??:? _d_assert_msg [0xa47df5] /usr/include/dlang/dmd/std/regex/package.d:564 std.regex.Captures!(immutable(char)[], ulong).Captures.opIndex!().opIndexinout(pure nothrow @trusted inout(immutable(char)[]) function(ulong)) [0x878691] source/app.d:155 @safe app.IssueRef[] app.matchIssueRefs(immutable(char)[]).matchToRefs!(std.regex.Captures!(immutable(char)[], ulong).Captures).matchToRefs(std.regex.Captures!(immutable(char)[], ulong).Captures) [0x8772ec] /usr/include/dlang/dmd/std/algorithm/iteration.d:593 @property @safe app.IssueRef[] std.algorithm.iteration.__T9MapResultS383app14matchIssueRefsFAyaZ11matchToRefsTS3std5regex108__T10RegexMatchTAyaS853std5regex8internal12backtracking29__T19BacktrackingMatcherVbi1Z19BacktrackingMatcherZ10RegexMatchZ.MapResult.front() [0x87e89d] /usr/include/dlang/dmd/std/algorithm/iteration.d:2376 ref @safe std.algorithm.iteration.joiner!(std.algorithm.iteration.__T9MapResultS383app14matchIssueRefsFAyaZ11matchToRefsTS3std5regex108__T10RegexMatchTAyaS853std5regex8internal12backtracking29__T19BacktrackingMatcherVbi1Z19BacktrackingMatcherZ10RegexMatchZ.MapResult).joiner(std.algorithm.iteration.__T9MapResultS383app14matchIssueRefsFAyaZ11matchToRefsTS3std5regex108__T10RegexMatchTAyaS853std5regex8internal12backtracking29__T19BacktrackingMatcherVbi1Z19BacktrackingMatcherZ10RegexMatchZ.MapResult).Result std.algorithm.iteration.joiner!(std.algorithm.iteration.__T9MapResultS383app14matchIssueRefsFAyaZ11matchToRefsTS3std5regex108__T10RegexMatchTAyaS853std5regex8internal12backtracking29__T19BacktrackingMatcherVbi1Z19BacktrackingMatcherZ10RegexMatchZ.MapResult).joiner(std.algorithm.iteration.__T9MapResultS383app14matchIssueRefsFAyaZ11matchToRefsTS3std5regex108__T10RegexMatchTAyaS853std5regex8internal12backtracking29__T19BacktrackingMatcherVbi1Z19BacktrackingMatcherZ10RegexMatchZ.MapResult).Result.__ctor(std.algorithm.iteration.__T9MapResultS383app14matchIssueRefsFAyaZ11matchToRefsTS3std5regex108__T10RegexMatchTAyaS853std5regex8internal12backtracking29__T19BacktrackingMatcherVbi1Z19BacktrackingMatcherZ10RegexMatchZ.MapResult) [0x87f3ea] /usr/include/dlang/dmd/std/algorithm/iteration.d:2419 @safe std.algorithm.iteration.joiner!(std.algorithm.iteration.__T9MapResultS383app14matchIssueRefsFAyaZ11matchToRefsTS3std5regex108__T10RegexMatchTAyaS853std5regex8internal12backtracking29__T19BacktrackingMatcherVbi1Z19BacktrackingMatcherZ10RegexMatchZ.MapResult).joiner(std.algorithm.iteration.__T9MapResultS383app14matchIssueRefsFAyaZ11matchToRefsTS3std5regex108__T10RegexMatchTAyaS853std5regex8internal12backtracking29__T19BacktrackingMatcherVbi1Z19BacktrackingMatcherZ10RegexMatchZ.MapResult).Result std.algorithm.iteration.joiner!(std.algorithm.iteration.__T9MapResultS383app14matchIssueRefsFAyaZ11matchToRefsTS3std5regex108__T10RegexMatchTAyaS853std5regex8internal12backtracking29__T19BacktrackingMatcherVbi1Z19BacktrackingMatcherZ10RegexMatchZ.MapResult).joiner(std.algorithm.iteration.__T9MapResultS383app14matchIssueRefsFAyaZ11matchToRefsTS3std5regex108__T10RegexMatchTAyaS853std5regex8internal12backtracking29__T19BacktrackingMatcherVbi1Z19BacktrackingMatcherZ10RegexMatchZ.MapResult) [0x87f2ed] source/app.d:164 @safe std.algorithm.iteration.joiner!(std.algorithm.iteration.__T9MapResultS383app14matchIssueRefsFAyaZ11matchToRefsTS3std5regex108__T10RegexMatchTAyaS853std5regex8internal12backtracking29__T19BacktrackingMatcherVbi1Z19BacktrackingMatcherZ10RegexMatchZ.MapResult).joiner(std.algorithm.iteration.__T9MapResultS383app14matchIssueRefsFAyaZ11matchToRefsTS3std5regex108__T10RegexMatchTAyaS853std5regex8internal12backtracking29__T19BacktrackingMatcherVbi1Z19BacktrackingMatcherZ10RegexMatchZ.MapResult).Result app.matchIssueRefs(immutable(char)[]) [0x81e169] source/app.d:169 void app.__unittestL167_8() [0x81e1c7] ??:? void app.__modtest() [0x8ba6c0] ??:? int core.runtime.runModuleUnitTests().__foreachbody2(object.ModuleInfo*) [0xaa6788] ??:? int object.ModuleInfo.opApply(scope int delegate(object.ModuleInfo*)).__lambda2(immutable(object.ModuleInfo*)) [0xa469eb] ??:? int rt.minfo.moduleinfos_apply(scope int delegate(immutable(object.ModuleInfo*))).__foreachbody2(ref rt.sections_elf_shared.DSO) [0xa534c6] ??:? int rt.sections_elf_shared.DSO.opApply(scope int delegate(ref rt.sections_elf_shared.DSO)) [0xa53a0d] ??:? int rt.minfo.moduleinfos_apply(scope int delegate(immutable(object.ModuleInfo*))) [0xa53457] ??:? int object.ModuleInfo.opApply(scope int delegate(object.ModuleInfo*)) [0xa469c7] ??:? runModuleUnitTests [0xaa667a] ??:? void rt.dmain2._d_run_main(int, char**, extern (C) int function(char[][])*).runAll() [0xa4e5aa] ??:? void rt.dmain2._d_run_main(int, char**, extern (C) int function(char[][])*).tryExec(scope void delegate()) [0xa4e54c] ??:? _d_run_main [0xa4e4bd] ??:? main [0x81b677] ??:? __libc_start_main [0x15bd5290]

You still know for which input?

Yes for the existing unittest below, hence the capture variable is set as:

m.captures[5] = ["16319", ""];

wilzbach · 2016-12-18T17:21:32Z

source/app.d

+string travisAPIURL = "https://api.travis-ci.org";
+string trelloAPIURL = "https://api.trello.com";
+string bugzillaURL = "https://issues.dlang.org";
+bool runAsync = true;


This makes the testing a lot easier :)

wilzbach · 2016-12-18T17:21:38Z

source/app.d

+string trelloAPIURL = "https://api.trello.com";
+string bugzillaURL = "https://issues.dlang.org";
+bool runAsync = true;
+bool runTrello = true;


For now I disabled the Trello capabilties in the testing framework

wilzbach · 2016-12-18T17:27:06Z

source/app.d

+            // https://developer.github.com/changes/2016-09-26-pull-request-merge-api-update/
+            req.headers["Accept"] = "application/vnd.github.polaris-preview+json";
+            req.writeJsonBody(reqInput);
+        }, prUrl);


We try the merge here .. it will probably fail & in the error case we just print a log message

We need to enable squash merges on all repos for that, right?

We need to enable squash merges on all repos for that, right?

Yep, but we don't need to do this for now or we could just enable it for one repo.

wilzbach · 2016-12-18T17:53:35Z

source/app.d

 //==============================================================================

-Json verifyRequest(string signature, string data)
+auto getSignature(string data)


split in two parts so that a "fake" signature can be stubbed

wilzbach · 2016-12-18T17:56:12Z

source/app.d

-            searchForAutoMergePrs(repoSlug);
+            if (Clock.currTime - lastFullPRCheck >= timeBetweenFullPRChecks)
+            {
+                searchForAutoMergePrs(repoSlug);


Simple throttling for querying over all open PRs. Currently it's throttle to 15.minutes

MartinNowak · 2016-12-18T18:13:29Z

.travis.yml

+d:
+ - dmd-beta
+ - dmd
+ - ldc


What about gdc?

OTOH, same reasoning as with the OS, we merely need to test the version we deploy with.

What about gdc?

I stopped testing/using it a long time ago, because one just runs into too many fixed bugs or missing features.

MartinNowak · 2016-12-18T18:16:02Z

payloads/bugzilla_api/buglist.cgi?bug_id=8573&ctype=csv&columnlist=short_desc.json

@@ -0,0 +1,2 @@
+bug_id,"short_desc"


That's a csv file, not json.

-> Changed in #16

MartinNowak · 2016-12-18T18:17:49Z

payloads/github_api/repos_dlang_dmd_pulls?state=open.json

@@ -0,0 +1,9128 @@
+[


Maybe it's possible to gzip the files? Vibe.d serves pre-compressed files by default.

Yeah probably, but it's compressed in the git history anyways and locally it doesn't matter?
The entire payloads folder has less than 1M atm.

MartinNowak · 2016-12-18T18:19:10Z

.travis.yml

+sudo: false
+os:
+ - linux
+ - osx


Really OSX? We don't want to test vibe.d, and aren't using any platform specific stuff.

Fair enough -> In #16 I removed macOS

MartinNowak · 2016-12-18T18:21:26Z

source/app.d

+    travisAuth = "token " ~ environment["TRAVIS_TOKEN"];
+
+    // workaround for stupid openssl.conf on Heroku
+    if (environment.get("DYNO") !is null)


Thanks for fixing that one.

MartinNowak · 2016-12-18T18:36:21Z

source/app.d

+        static if (T.length)
+            req.writeJsonBody(arg);
+    }, url);
+}


The weird vibe.d request API is actually a good test case for the scope/DIP1000 topic. It's only using callbacks to not escape scoped classes.

MartinNowak · 2016-12-18T18:37:09Z

source/app.d

+
+auto analyseLabels(Json[] labels)
+{
+    auto labelNames = labels.map!`a["name"].get!string`.array;


Please avoid string lambdas.

MartinNowak · 2016-12-18T18:42:42Z

source/app.d

+            // https://developer.github.com/changes/2016-09-26-pull-request-merge-api-update/
+            req.headers["Accept"] = "application/vnd.github.polaris-preview+json";
+            req.writeJsonBody(reqInput);
+        }, prUrl);


We need to enable squash merges on all repos for that, right?

MartinNowak · 2016-12-18T18:48:39Z

source/app.d

+    auto prs = ghGetRequest("%s/repos/%s/pulls?state=open".format(githubAPIURL, repoSlug)).readJson[];
+    foreach (pr; prs)
+    {
+        handleGithubLabel(repoSlug, pr["number"].get!uint, pr["commits_url"].get!string, true);


Not restricting label?
Sounds a bit better to first get all label:auto-merge, then all label:auto-merge-squash, subtract the latter from the former (in case of both), and then loop over the 2 arrays.

Would mean, you need to break up the handleGithubLabel function into an information retrieving part and an action part.

Not restricting label?
Sounds a bit better to first get all label:auto-merge, then all label:auto-merge-squash, subtract the latter from the former (in case of both), and then loop over the 2 arrays.

At GitHub there are two APIs for this: issue and pulls.
With the issue API we can directly filter on the labels -> #17

MartinNowak · 2016-12-18T18:53:05Z

test/utils.d

+    fakeSettings.bindAddresses = ["0.0.0.0"];
+    fakeSettings.options = HTTPServerOption.defaults & HTTPServerOption.parseJsonBody;
+    auto router = new URLRouter;
+    router.any("*", &payloadServer);


How about letting the router do the GH, Trello, Travis dispatching?

MartinNowak · 2016-12-18T19:02:29Z

Maybe one bigger thing?
Can we just have the test server, serve files matching a particular folder structure, without hard-coding the responses?
So payloads/github/repos/dlang/dmd/pulls?state=open would be a text file for that exact request?
We could unconditionally replace API URLs for github in payloads/github.

MartinNowak · 2016-12-18T19:15:59Z

test/comments.d

+// no existing dlang bot comment -> create comment
+unittest
+{
+    auto expectedURLs = ["/github/repos/dlang/phobos/pulls/4921/commits",


Looks a bit as if it could be done somewhat similar to the router.

setAPIExpectations!( "/github/repos/dlang/phobos/pulls/4921/commits", "/github/repos/dlang/phobos/issues/4921/labels", //... "/github/repos/dlang/phobos/issues/4921/comments", (scope req, scope res) { // assert(req == ); }); postStatus("...");

wilzbach · 2016-12-18T23:06:02Z

Closed in favor of #17

wilzbach added 5 commits December 18, 2016 09:07

Upgrade to Vibe.d 0.7.30 and add separate test configuration

477e16e

Dump Payloads

fdcd75c

Add small testing/stub framework for Vibe.d

0f54c04

Add auto-merge capability to dlang-bot (+many tests)

20b9363

Use runtime regex instead of ctRegex to avoid weird unittest error in…

81ed301

… std.regex

wilzbach force-pushed the merge-prs branch 2 times, most recently from e63d8e8 to cacedfa Compare December 18, 2016 17:54

wilzbach commented Dec 18, 2016

View reviewed changes

Add mergeAllPRs() throttling

a5acf58

wilzbach force-pushed the merge-prs branch from cacedfa to a5acf58 Compare December 18, 2016 17:59

wilzbach added 3 commits December 18, 2016 19:02

Add simple Travis config

bf5c290

Exclude dmd-nightly from Travis builds

cae539c

remove commit sha debug print

9733ee3

MartinNowak reviewed Dec 18, 2016

View reviewed changes

wilzbach mentioned this pull request Dec 18, 2016

Add simple test suite #16

Merged

wilzbach closed this Dec 18, 2016

Uh oh!

Automatically merge PRs with auto-merge #15

Automatically merge PRs with auto-merge #15

Uh oh!

Conversation

wilzbach commented Dec 18, 2016

Uh oh!

wilzbach left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinNowak commented Dec 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Dec 18, 2016

Uh oh!

Uh oh!