Milo: Increase test coverage for appengine/buildbot

milo/appengine/buildbot/builder.go

Index: milo/appengine/buildbot/builder.go
diff --git a/milo/appengine/buildbot/builder.go b/milo/appengine/buildbot/builder.go
index b89682a45e450c66ea6f93037d5009f836b58b4e..ba9a4d4f46948fc4ae303a291e82f4f18b2beb8a 100644
--- a/milo/appengine/buildbot/builder.go
+++ b/milo/appengine/buildbot/builder.go
@@ -152,6 +152,18 @@ var errMasterNotFound = errors.New(
 	"Either the request resource was not found or you have insufficient permissions")
 var errNotAuth = errors.New("You are not authenticated, try logging in")
 
+type errBuilderNotFound struct {
+	master    string
+	builder   string
+	available []string
+}
+
+func (e errBuilderNotFound) Error() string {
+	avail := strings.Join(e.available, "\n")
+	return fmt.Sprintf("Cannot find builder %q in master %q.\nAvailable builders: \n%q",

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

i think the list of builders should be %s, otherwise it will be `Available
builders: "builder1\nbuilder2\nbuilder3"

also trailing space after "builder:" seems unnecessary

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

Done.

+		e.master, e.builder, avail)

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

swap e.master and e.builder, see order in the format string

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

Shoot I'm blind, I even checked for this.

+}
+
 // builderImpl is the implementation for getting a milo builder page from buildbot.
 // This gets:
 // * Current Builds from querying the master json from the datastore.
@@ -192,10 +204,7 @@ func builderImpl(
 			keys = append(keys, k)
 		}
 		sort.Strings(keys)
-		avail := strings.Join(keys, "\n")
-		return nil, fmt.Errorf(
-			"Cannot find builder %s in master %s.\nAvailable builders: \n%s",
-			builderName, masterName, avail)
+		return nil, errBuilderNotFound{masterName, builderName, keys}
 	}
 	// Extract pending builds out of the master json.
 	result.PendingBuilds = make([]*resp.BuildSummary, len(p.PendingBuildStates))