Milo: Increase test coverage for appengine/buildbot

milo/appengine/common/middleware.go

Index: milo/appengine/common/middleware.go
diff --git a/milo/appengine/common/middleware.go b/milo/appengine/common/middleware.go
index 3397d6e808d3ef7de65f1297f5e7e77caeb93592..0a6482cbc8c0094875d973d7cad82523f5b8a08e 100644
--- a/milo/appengine/common/middleware.go
+++ b/milo/appengine/common/middleware.go
@@ -30,7 +30,7 @@ var authconfig *auth.Config
 // passed to all templates.
 func GetTemplateBundle() *templates.Bundle {
 	return &templates.Bundle{
-		Loader:          templates.FileSystemLoader("templates"),
+		Loader:          templates.FileSystemLoader("../frontend/templates"),

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

when running a test, cwd is test location, and I assume this fixes usage of
GetTemplateBundle() in test. Note that it assumes that the test directory is a
sibling of frontend dir.
when running in prod, cwd is location of module yaml file. I am not sure current
directory will be called "frontend". Is it?

in general, personally i'd prefer tests in a package to test only code in that
package. That means buildbot package should check resp.MiloBuild that it
returns, not HTML. The frontend package should check that a valid resp.MiloBuild
provided by a test can be rendered by HTML templates properly. In that case you
don't have this problem, and your tests are more targeted (i.e. one bug causes
less tests to fail, not more)

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

Acknowledged.  Yeah I think the main value of the template tests are probably to
see "does this template render without error?".  We can probably remove most of
the integration testing code without value loss.  That can be done in a separate
CL.

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

yeah, a separate CL is fine.

I don't think we pay close enough attention to the values in the expectation
files. I remember a couple of values in expectation files were incorrect. It is
hard to ensure they are correct because they have a lot of text. I think
targeted (=small) assertion tests that check the values would be more effective.

I definitely see the value of ensuring that a template renders without errors.
It does not require checking expectation files in though.

 		DebugMode:       info.IsDevAppServer,
 		DefaultTemplate: "base",
 		DefaultArgs: func(c context.Context) (templates.Args, error) {