Milo: Increase test coverage for appengine/buildbot

milo/appengine/buildbot/html_test.go

Index: milo/appengine/buildbot/html_test.go
diff --git a/milo/appengine/buildbot/html_test.go b/milo/appengine/buildbot/html_test.go
new file mode 100644
index 0000000000000000000000000000000000000000..98b9a56496830057db0dbc9e25dda800c167c227
--- /dev/null
+++ b/milo/appengine/buildbot/html_test.go
@@ -0,0 +1,100 @@
+package buildbot
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"golang.org/x/net/context"
+
+	"github.com/julienschmidt/httprouter"
+	"github.com/luci/gae/impl/memory"
+	"github.com/luci/luci-go/common/clock/testclock"
+	"github.com/luci/luci-go/milo/appengine/common"
+	"github.com/luci/luci-go/server/auth"
+	"github.com/luci/luci-go/server/auth/authtest"
+	"github.com/luci/luci-go/server/auth/identity"
+	"github.com/luci/luci-go/server/router"
+	"github.com/luci/luci-go/server/templates"
+	. "github.com/smartystreets/goconvey/convey"
+)
+
+func request(c context.Context, params map[string]string) *router.Context {
+	p := httprouter.Params{}
+	for k, v := range params {
+		p = append(p, httprouter.Param{Key: k, Value: v})
+	}
+	r := &http.Request{URL: &url.URL{Path: "/foobar"}}
+	c = common.WithRequest(c, r)
+	w := httptest.NewRecorder()
+	return &router.Context{
+		Context: c,
+		Params:  p,
+		Writer:  w,
+		Request: r,
+	}
+}
+
+func TestHtml(t *testing.T) {
+	c := memory.UseWithAppID(context.Background(), "dev~luci-milo")
+	c, _ = testclock.UseTime(c, testclock.TestTimeUTC)
+	c = templates.Use(c, common.GetTemplateBundle())
+	c = auth.WithState(c, &authtest.FakeState{Identity: identity.AnonymousIdentity})
+
+	Convey(`HTML handler tests`, t, func() {
+		Convey(`Build pages`, func() {
+			Convey(`Empty request`, func() {
+				rc := request(c, map[string]string{})

[nodir, 2017/05/25 15:23:04]

passing nil map would be simpler and work too

[Ryan Tseng, 2017/05/26 18:10:52]

Done.

+				BuildHandler(rc)
+				response := rc.Writer.(*httptest.ResponseRecorder)
+				So(response.Code, ShouldEqual, http.StatusBadRequest)
+			})
+			Convey(`Missing builder`, func() {
+				rc := request(c, map[string]string{"master": "foo"})
+				BuildHandler(rc)
+				response := rc.Writer.(*httptest.ResponseRecorder)
+				So(response.Code, ShouldEqual, http.StatusBadRequest)
+			})
+			Convey(`Missing build`, func() {
+				rc := request(c, map[string]string{"master": "foo", "builder": "bar"})
+				BuildHandler(rc)
+				response := rc.Writer.(*httptest.ResponseRecorder)
+				So(response.Code, ShouldEqual, http.StatusBadRequest)
+			})
+			Convey(`Invalid build number`, func() {
+				rc := request(c, map[string]string{"master": "foo", "builder": "bar", "build": "baz"})
+				BuildHandler(rc)
+				response := rc.Writer.(*httptest.ResponseRecorder)
+				So(response.Code, ShouldEqual, http.StatusBadRequest)
+			})
+			Convey(`Normal build`, func() {
+				rc := request(c, map[string]string{"master": "debug", "builder": "newline", "build": "1234"})
+				BuildHandler(rc)
+				response := rc.Writer.(*httptest.ResponseRecorder)
+				So(response.Code, ShouldEqual, http.StatusOK)
+			})
+		})
+
+		Convey(`Builder pages`, func() {
+			Convey(`Empty request`, func() {
+				rc := request(c, map[string]string{})

[nodir, 2017/05/25 15:23:04]

same

[Ryan Tseng, 2017/05/26 18:10:52]

Done.

+				BuilderHandler(rc)
+				response := rc.Writer.(*httptest.ResponseRecorder)
+				So(response.Code, ShouldEqual, http.StatusBadRequest)
+			})
+			Convey(`Missing builder`, func() {
+				rc := request(c, map[string]string{"master": "foo"})
+				BuilderHandler(rc)
+				response := rc.Writer.(*httptest.ResponseRecorder)
+				So(response.Code, ShouldEqual, http.StatusBadRequest)
+			})
+			Convey(`Failing builder page`, func() {
+				rc := request(c, map[string]string{"master": "debug", "builder": "newline"})
+				BuilderHandler(rc)
+				response := rc.Writer.(*httptest.ResponseRecorder)
+				So(response.Code, ShouldEqual, http.StatusInternalServerError)

[nodir, 2017/05/25 15:23:04]

i think "Failing" here actually means non-existent. In that case I think this
should be 404

[Ryan Tseng, 2017/05/26 18:10:52]

Done.

+			})
+		})
+	})
+}