Milo: Increase test coverage for appengine/buildbot

milo/appengine/buildbot/html.go

Index: milo/appengine/buildbot/html.go
diff --git a/milo/appengine/buildbot/html.go b/milo/appengine/buildbot/html.go
index 3b1d2b84f08b7ae8d30bc9fedef0563269a0725d..557bca8294836f559192aa0ea82a162d133918d3 100644
--- a/milo/appengine/buildbot/html.go
+++ b/milo/appengine/buildbot/html.go
@@ -38,7 +38,7 @@ func BuildHandler(c *router.Context) {
 		return
 	}
 
-	result, err := build(c.Context, master, builder, num)
+	result, err := Build(c.Context, master, builder, num)
 	if err != nil {
 		var code int
 		switch err {
@@ -84,6 +84,14 @@ func BuilderHandler(c *router.Context) {
 
 	result, err := builderImpl(c.Context, master, builder, limit, cursor)
 	if err != nil {

[nodir, 2017/05/26 18:33:05]

we have 4 cases here essentially. I think using switch would make this code a
bit simpler and makes it impossible to forget a return statement

_, notFound := err.(errBuilderNotFound)
switch {
case notFound: ...
case err == errNotAuth: ...
case err != nil: ...
default:...
}
} // end of function

[Ryan Tseng, 2017/05/26 22:49:22]

I was reluctlant to do this because I wouldn't have been able to inline _,
notFound := err.(errBuilderNotFound).  But now that I tried it it doesn't look
too bad, changed

+		if _, ok := err.(errBuilderNotFound); ok {
+			common.ErrorPage(c, http.StatusNotFound, err.Error())
+			return
+		}
+		if err == errNotAuth {
+			common.ErrorPage(c, http.StatusUnauthorized, err.Error())
+			return
+		}
 		common.ErrorPage(c, http.StatusInternalServerError, err.Error())
 		return
 	}