Milo: Increase test coverage for appengine/buildbot

milo/appengine/buildbot/builder.go

 // Copyright 2016 The LUCI Authors. All rights reserved.
 // Use of this source code is governed under the Apache License, Version 2.0
 // that can be found in the LICENSE file.
 
 package buildbot
 
 import (
         "crypto/sha1"
         "encoding/base64"
         "errors"
@@ skipping 134 matching lines @@
                         return &prevCursor, true
                 }
         }
         return nil, false
 }
 
 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.
 // * Recent Builds from a cron job that backfills the recent builds.
 func builderImpl(
         c context.Context, masterName, builderName string, limit int, cursor string) (
         *resp.Builder, error) {
 
         var thisCursor *datastore.Cursor
         if cursor != "" {
@@ skipping 20 matching lines @@
 
         p, ok := master.Builders[builderName]
         if !ok {
                 // This long block is just to return a good error message when an invalid
                 // buildbot builder is specified.
                 keys := make([]string, 0, len(master.Builders))
                 for k := range master.Builders {
                         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))
         logging.Debugf(c, "Number of pending builds: %d", len(p.PendingBuildStates))
         for i, pb := range p.PendingBuildStates {
                 start := time.Unix(int64(pb.SubmittedAt), 0)
                 result.PendingBuilds[i] = &resp.BuildSummary{
                         PendingTime: resp.Interval{
                                 Started:  start,
                                 Duration: time.Now().Sub(start),
@@ skipping 35 matching lines @@
         }
         result.CurrentBuilds = currentBuilds
 
         for _, fb := range finishedBuilds {
                 if fb != nil {
                         result.FinishedBuilds = append(result.FinishedBuilds, fb)
                 }
         }
         return result, nil
 }