Merge pull request #169 from MartinNowak/fix167

fix #167 - fix misleading error output during tests

Author: wilzbach

.gitignore

@@ -1,6 +1,7 @@
 dlang-bot
 __test__unittest__
 dlang-bot-test-unittest
+ut.d # unit-threaded
 
 # Created by https://www.gitignore.io/api/d
 

.travis.yml

@@ -9,6 +9,10 @@ d:
  - ldc-beta
  - ldc
 
+script:
+  # Tests are neither random- nor parallelizable atm., so use a --single serial thread.
+  - dub test --compiler=$DC -- --single --trace
+
 addons:
   apt:
     packages:
@@ -19,8 +23,9 @@ matrix:
     - d: dmd-2.077.1 # the deployment compiler
       env: COVERAGE=true
       script:
-        # https://issues.dlang.org/show_bug.cgi?id=13742
-        - dub test --compiler=$DC --build=unittest-cov --build-mode=singleFile
+        # Use --build-mode=singleFile to workaround https://issues.dlang.org/show_bug.cgi?id=13742
+        # Tests are neither random- nor parallelizable atm., so use a --single serial thread.
+        - dub test --compiler=$DC --build=unittest-cov --build-mode=singleFile -- --single --trace
       after_success:
         - bash <(curl -s https://codecov.io/bash)
   allow_failures:

dub.sdl

@@ -4,13 +4,15 @@ copyright "Copyright © 2015, Martin Nowak"
 authors "Martin Nowak"
 dependency "vibe-d" version="~>0.8.0"
 subConfiguration "vibe-d:core" "vibe-core"
-configuration "default" {
-	versions "VibeCustomMain"
-	targetType "executable"
+targetType "executable"
+
+configuration "executable" {
 }
+
 configuration "unittest" {
-	versions "VibeCustomMain"
-	sourcePaths "source" "test"
-	importPaths "source" "test"
-	stringImportPaths "views"
+    dependency "unit-threaded" version="~>0.7.11"
+    mainSourceFile "ut.d"
+    preBuildCommands "dub run unit-threaded -c gen_ut_main -- -f ut.d"
+    sourcePaths "source" "test"
+    importPaths "source" "test"
 }

dub.selections.json

@@ -11,6 +11,7 @@
 		"openssl": "1.1.6+1.0.1g",
 		"stdx-allocator": "2.77.0",
 		"taggedalgebraic": "0.10.8",
+		"unit-threaded": "0.7.36",
 		"vibe-core": "1.4.0-alpha.1",
 		"vibe-d": "0.8.3-alpha.1"
 	}

source/dlangbot/appveyor.d

@@ -13,19 +13,19 @@ import vibe.core.log;
 void cancelBuild(string repoSlug, size_t buildId)
 {
     import std.format : format;
-    import vibe.http.client : requestHTTP;
     import vibe.http.common : HTTPMethod;
     import vibe.stream.operations : readAllUTF8;
+    import dlangbot.utils : request;
 
     auto url = "%s/builds/%s/%s/cancel".format(appveyorAPIURL, repoSlug, buildId);
-    requestHTTP(url, (scope req) {
+    request(url, (scope req) {
         req.headers["Authorization"] = appveyorAuth;
         req.method = HTTPMethod.DELETE;
     }, (scope res) {
         if (res.statusCode / 100 == 2)
             logInfo("[appveyor/%s]: Canceled Build %s\n", repoSlug, buildId);
         else
-            logWarn("[appveyor/%s]: POST %s failed;  %s %s.\n%s", repoSlug, url, res.statusPhrase,
+            logError("[appveyor/%s]: POST %s failed;  %s %s.\n%s", repoSlug, url, res.statusPhrase,
                 res.statusCode, res.bodyReader.readAllUTF8);
     });
 }
@@ -38,7 +38,7 @@ void dedupAppVeyorBuilds(string action, string repoSlug, uint pullRequestNumber)
     import std.format : format;
     import std.range : drop;
     import vibe.data.json : Json;
-    import vibe.http.client : requestHTTP;
+    import dlangbot.utils : request;
 
     if (action != "synchronize" && action != "merged")
         return;
@@ -51,7 +51,7 @@ void dedupAppVeyorBuilds(string action, string repoSlug, uint pullRequestNumber)
     // GET /api/projects/{accountName}/{projectSlug}/history?recordsNumber={records-per-page}[&startBuildId={buildId}&branch={branch}]
 
     auto url = "%s/projects/%s/history?recordsNumber=100".format(appveyorAPIURL, repoSlug);
-    auto activeBuildsForPR = requestHTTP(url, (scope req) {
+    auto activeBuildsForPR = request(url, (scope req) {
             req.headers["Authorization"] = appveyorAuth;
         })
         .readJson["builds"][]

source/dlangbot/bugzilla.d

@@ -62,14 +62,14 @@ struct Issue
 Issue[] getDescriptions(R)(R issueRefs)
 {
     import std.csv;
-    import vibe.http.client : requestHTTP;
     import vibe.stream.operations : readAllUTF8;
+    import dlangbot.utils : request;
 
     if (issueRefs.empty)
         return null;
     return "%s/buglist.cgi?bug_id=%(%d,%)&ctype=csv&columnlist=short_desc,bug_status,resolution,bug_severity,priority"
         .format(bugzillaURL, issueRefs.map!(r => r.id))
-        .requestHTTP
+        .request
         .bodyReader.readAllUTF8
         .csvReader!Issue(null)
         .array

source/dlangbot/github_api.d

@@ -11,20 +11,22 @@ import std.typecons : Nullable;
 
 import vibe.core.log;
 import vibe.data.json;
-import vibe.http.client : HTTPClientRequest, requestHTTP;
+import vibe.http.client : HTTPClientRequest;
 public import vibe.http.common : HTTPMethod;
 import vibe.stream.operations : readAllUTF8;
 
+import dlangbot.utils : request;
+
 auto ghGetRequest(string url)
 {
-    return requestHTTP(url, (scope req) {
+    return request(url, (scope req) {
         req.headers["Authorization"] = githubAuth;
     });
 }
 
 auto ghGetRequest(scope void delegate(scope HTTPClientRequest req) userReq, string url)
 {
-    return requestHTTP(url, (scope req) {
+    return request(url, (scope req) {
         req.headers["Authorization"] = githubAuth;
         userReq(req);
     });
@@ -33,7 +35,7 @@ auto ghGetRequest(scope void delegate(scope HTTPClientRequest req) userReq, stri
 auto ghSendRequest(scope void delegate(scope HTTPClientRequest req) userReq, string url)
 {
     HTTPMethod method;
-    requestHTTP(url, (scope req) {
+    request(url, (scope req) {
         req.headers["Authorization"] = githubAuth;
         userReq(req);
         method = req.method;
@@ -43,9 +45,6 @@ auto ghSendRequest(scope void delegate(scope HTTPClientRequest req) userReq, str
             logInfo("%s %s, %s\n", method, url, res.statusPhrase);
             res.bodyReader.readAllUTF8;
         }
-        else
-            logWarn("%s %s failed;  %s %s.\n%s", method, url,
-                res.statusPhrase, res.statusCode, res.bodyReader.readAllUTF8);
     });
 }
 

source/dlangbot/trello.d

@@ -4,12 +4,12 @@ string trelloAPIURL = "https://api.trello.com";
 string trelloSecret, trelloAuth;
 
 import dlangbot.bugzilla : Issue, IssueRef;
+import dlangbot.utils : request;
 import std.algorithm, std.range;
 import std.format : format;
 
 import vibe.core.log;
 import vibe.data.json;
-import vibe.http.client : requestHTTP;
 import vibe.http.common : HTTPMethod;
 import vibe.stream.operations : readAllUTF8;
 
@@ -20,7 +20,7 @@ import vibe.stream.operations : readAllUTF8;
 void trelloSendRequest(T...)(HTTPMethod method, string url, T arg)
     if (T.length <= 1)
 {
-    requestHTTP(url, (scope req) {
+    request(url, (scope req) {
         req.method = method;
         static if (T.length)
             req.writeJsonBody(arg);
@@ -29,7 +29,7 @@ void trelloSendRequest(T...)(HTTPMethod method, string url, T arg)
             logInfo("%s %s: %s\n", method, url.replace(trelloAuth, "key=[hidden]&token=[hidden]")
                     , res.statusPhrase);
         else
-            logWarn("%s %s: %s %s.\n%s", method, url.replace(trelloAuth, "key=[hidden]&token=[hidden]"),
+            logError("%s %s: %s %s.\n%s", method, url.replace(trelloAuth, "key=[hidden]&token=[hidden]"),
                 res.statusPhrase, res.statusCode, res.bodyReader.readAllUTF8);
     });
 }
@@ -79,7 +79,7 @@ auto findTrelloCards(int issueID)
 {
 
     return trelloAPI("/1/search?query=name:\"Issue %d\"", issueID)
-        .requestHTTP
+        .request
         .readJson["cards"][]
         .map!(c => TrelloCard(c["id"].get!string, issueID));
 }
@@ -89,7 +89,7 @@ struct Comment { string url, body_; }
 Comment getTrelloBotComment(string cardID)
 {
     auto res = trelloAPI("/1/cards/%s/actions?filter=commentCard", cardID)
-        .requestHTTP
+        .request
         .readJson[]
         .find!(c => c["memberCreator"]["username"] == "dlangbot");
     if (res.length)
@@ -102,10 +102,10 @@ Comment getTrelloBotComment(string cardID)
 void moveCardToList(string cardID, string listName)
 {
     auto card = trelloAPI("/1/cards/%s", cardID)
-        .requestHTTP
+        .request
         .readJson;
     auto lists = trelloAPI("/1/board/%s/lists", card["idBoard"].get!string)
-        .requestHTTP
+        .request
         .readJson[];
 
     immutable curListName = lists.find!(c => c["id"].get!string == card["idList"].get!string)

source/dlangbot/utils.d

@@ -2,8 +2,74 @@ module dlangbot.utils;
 
 import dlangbot.app : runAsync;
 
+import vibe.http.client : HTTPClientRequest, HTTPClientResponse;
+import vibe.http.common : HTTPStatus;
+
 import std.datetime : Duration;
 
+version (unittest) int _expectedStatusCode;
+
+bool expectedStatusCode(int statusCode) nothrow @safe @nogc
+{
+    version (unittest)
+    {
+        if (_expectedStatusCode == statusCode)
+        {
+            _expectedStatusCode = 0;
+            return true;
+        }
+    }
+    return false;
+}
+
+HTTPClientResponse request(
+    string url,
+    scope void delegate(scope HTTPClientRequest) requester = cast(void delegate(scope HTTPClientRequest req))null
+) @safe
+{
+    import vibe.core.log : logError;
+    import vibe.http.client : requestHTTP;
+    import vibe.http.common : HTTPMethod;
+
+    HTTPMethod method;
+    auto res = requestHTTP(
+        url,
+        (scope req) {
+            if (requester !is null)
+                requester(req);
+            method = req.method;
+        });
+    if (res.statusCode / 100 != 2 && !expectedStatusCode(res.statusCode))
+        logError("%s %s failed;  %s %s.", method, url, res.statusPhrase, res.statusCode);
+    return res;
+}
+
+void request(
+    string url,
+    scope void delegate(scope HTTPClientRequest) requester,
+    scope void delegate(scope HTTPClientResponse) responder
+) @safe
+{
+    import vibe.core.log : logError;
+    import vibe.http.client : requestHTTP;
+    import vibe.http.common : HTTPMethod;
+
+    HTTPMethod method;
+    requestHTTP(
+        url,
+        (scope req) {
+            requester(req);
+            method = req.method;
+        },
+        (scope res) {
+            if (res.statusCode / 100 != 2 && !expectedStatusCode(res.statusCode))
+                logError("%s %s failed;  %s %s.", method, url, res.statusPhrase, res.statusCode);
+            responder(res);
+        }
+    );
+
+}
+
 auto runTaskHelper(Fun, Args...)(Fun fun, auto ref Args args)
 {
     import std.functional : toDelegate;

test/appveyor.d

@@ -20,9 +20,9 @@ unittest
             j["builds"] = j["builds"][0..2];
         },
         "/appveyor/builds/greenify/dmd/13074433/cancel",
+        HTTPStatus.noContent,
         (scope HTTPServerRequest req, scope HTTPServerResponse res) {
             assert(req.method == HTTPMethod.DELETE);
-            res.statusCode = 204;
             res.writeVoidBody;
         },
     );