Milo: Don't float step times if build has finished

milo/appengine/buildbot/build.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 (
         "encoding/json"
         "errors"
         "fmt"
@@ skipping 48 matching lines @@
         case 4:
                 status = resp.InfraFailure // Exception
         case 5:
                 status = resp.WaitingDependency // Retry
         default:
                 panic(fmt.Errorf("Unknown status %d", s))
         }
         return
 }
 
+// buildbotTimeToTime converts a buildbot time representation (pointer to float
+// of seconds since epoch) to a natime time.Time object.

[nodir, 2017/03/24 06:23:49]

native

[hinoka, 2017/03/28 18:18:36]

Done.

+func buildbotTimeToTime(t *float64) (result time.Time) {
+        if t != nil {
+                result = time.Unix(int64(*t), int64(*t*1e9)%1e9).UTC()

[nodir, 2017/03/24 06:23:49]

I think this needs gofmt

[hinoka, 2017/03/28 18:18:36]

Just did... nothing changed. (It's also a write hook on my vim setup)

+        }
+        return
+}
+
 // parseTimes translates a buildbot time tuple (start/end) into a triplet

[nodir, 2017/03/24 06:23:49]

nit: standard notation is that tuple item separator is comma, not slash

[hinoka, 2017/03/28 18:18:36]

Done.

 // of (Started time, Ending time, duration).
-func parseTimes(times []*float64) (started, ended time.Time, duration time.Duration) {
+// If buildFinished is not nil, then all non-specified end duration will be set to

[nodir, 2017/03/24 06:23:49]

it says "all" like there are multiple end duration. Also, what is "end
duration"?

Consider something along:

If times[1] is nil and buildFinished is not, ended will be set to buildFinished
time.

[hinoka, 2017/03/28 18:18:36]

Done.

+// buildFinished.  This is so that completed builds never have open step ending
+// times.
+func parseTimes(buildFinished *float64, times []*float64) (started, ended time.Time, duration time.Duration) {
         if len(times) != 2 {
                 panic(fmt.Errorf("Expected 2 floats for times, got %v", times))
         }
         if times[0] == nil {
                 // Some steps don't have timing info.  In that case, just return nils.
                 return
         }
-        started = time.Unix(int64(*times[0]), int64(*times[0]*1e9)%1e9).UTC()
-        if times[1] != nil {
-                ended = time.Unix(int64(*times[1]), int64(*times[1]*1e9)%1e9).UTC()
+        started = buildbotTimeToTime(times[0])
+        switch {
+        case times[1] != nil:
+                ended = buildbotTimeToTime(times[1])
                 duration = ended.Sub(started)
-        } else {
+        case buildFinished != nil:
+                ended = buildbotTimeToTime(buildFinished)
+                duration = ended.Sub(started)
+        default:
                 duration = time.Since(started)
         }
         return
 }
 
 // getBanner parses the OS information from the build and maybe returns a banner.
 func getBanner(c context.Context, b *buildbotBuild) *resp.LogoBanner {
         logging.Infof(c, "OS: %s/%s", b.OSFamily, b.OSVersion)
         osLogo := func() *resp.Logo {
                 result := &resp.Logo{}
@@ skipping 25 matching lines @@
         // TODO(hinoka): use b.toStatus()
         // Status
         var status resp.Status
         if b.Currentstep != nil {
                 status = resp.Running
         } else {
                 status = result2Status(b.Results)
         }
 
         // Timing info
-        started, ended, duration := parseTimes(b.Times)
+        started, ended, duration := parseTimes(nil, b.Times)
 
         // Link to bot and original build.
         server := "build.chromium.org/p"
         if b.Internal {
                 server = "uberchromegw.corp.google.com/i"
         }
         bot := &resp.Link{
                 Label: b.Slave,
                 URL:   fmt.Sprintf("https://%s/%s/buildslaves/%s", server, b.Master, b.Slave),
         }
@@ skipping 34 matching lines @@
         }
 
         return sum
 }
 
 var rLineBreak = regexp.MustCompile("<br */?>")
 
 // components takes a full buildbot build struct and extract step info from all
 // of the steps and returns it as a list of milo Build Components.
 func components(b *buildbotBuild) (result []*resp.BuildComponent) {
+        endingTime := b.Times[1]
         for i, step := range b.Steps {
                 if step.Hidden == true {
                         continue
                 }
                 bc := &resp.BuildComponent{
                         Label: step.Name,
                 }
                 // Step text sometimes contains <br>, which we want to parse into new lines.
                 for _, t := range step.Text {
                         for _, line := range rLineBreak.Split(t, -1) {
@@ skipping 119 matching lines @@
                                         baseLink = append(baseLink, aliasLink)
                                 }
 
                                 if len(baseLink) > 0 {
                                         bc.SubLink = append(bc.SubLink, baseLink)
                                 }
                         }
                 }
 
                 // Figure out the times.
-                bc.Started, bc.Finished, bc.Duration = parseTimes(step.Times)
+                bc.Started, bc.Finished, bc.Duration = parseTimes(endingTime, step.Times)
 
                 result = append(result, bc)
         }
         return
 }
 
 // parseProp returns a representation of v based off k, and a boolean to
 // specify whether or not to hide it altogether.
 func parseProp(prop map[string]Prop, k string, v interface{}) (string, bool) {
         switch k {
@@ skipping 190 matching lines @@
 
         // TODO(hinoka): Do all fields concurrently.
         return &resp.MiloBuild{
                 SourceStamp:   sourcestamp(c, b),
                 Summary:       summary(c, b),
                 Components:    components(b),
                 PropertyGroup: properties(b),
                 Blame:         blame(b),
         }, nil
 }