test-results: Add merge and trim methods

go/src/infra/appengine/test-results/model/merge.go

Index: go/src/infra/appengine/test-results/model/merge.go
diff --git a/go/src/infra/appengine/test-results/model/merge.go b/go/src/infra/appengine/test-results/model/merge.go
new file mode 100644
index 0000000000000000000000000000000000000000..72d2d53742d6bc09f5af3dc37ac6d187ccdb74ec
--- /dev/null
+++ b/go/src/infra/appengine/test-results/model/merge.go
@@ -0,0 +1,167 @@
+package model

[martiniss, 2016/08/12 00:14:24]

I don't see the need for a merge.go file; why don't these live in the files
where these structs are defined? If you're calling a common function in here,
that would make more sense, but it looks like there's no reason these functions
need to be in this file.

[nishanths, 2016/08/12 02:37:39]

As discussed offline, will move the contents of merge.go and trim.go to
aggregate_result.go after https://codereview.chromium.org/2234353002/ lands.

[nishanths, 2016/08/16 01:01:53]

Done in patch set 7.

+
+import "errors"
+
+var (
+	// ErrBuildNumberConflict is returned when the build numbers
+	// are the same when merging.
+	ErrBuildNumberConflict = errors.New("build number conflict")

[martiniss, 2016/08/12 00:14:24]

It's not obvious to me from reading this file why this can't be the case. Maybe
expand more in the function documentation?

[nishanths, 2016/08/12 02:37:39]

Done.

+
+	// ErrBuilderNameConflict is returned when the builder names
+	// do not match when merging.
+	ErrBuilderNameConflict = errors.New("builder name conflict")
+)
+
+// Merge merges x into ar. After merging, ar's fields may reference the same
+// objects referenced by x.
+func (ar *AggregateResult) Merge(x *AggregateResult) error {

[martiniss, 2016/08/12 00:14:24]

nit: "other" is a bit more descriptive than "x".

[nishanths, 2016/08/12 02:37:39]

Done, thanks.

+	if ar == nil {
+		return errors.New("model: Merge: nil *AggregateResult")
+	}
+	if ar.Builder != x.Builder {
+		return ErrBuilderNameConflict
+	}
+	if ar.BuilderInfo == nil {
+		ar.BuilderInfo = &BuilderInfo{}
+	}
+	ar.Version = ResultsVersion
+	return ar.BuilderInfo.Merge(x.BuilderInfo)
+}
+
+// Merge merges x into info. After merging, info's fields may reference the same

[martiniss, 2016/08/12 00:14:24]

Does the reference comment matter? Would that ever be confusing for callers?

[nishanths, 2016/08/12 02:37:39]

Removed, thanks.

+// objects referenced by x.
+func (info *BuilderInfo) Merge(x *BuilderInfo) error {

[martiniss, 2016/08/12 00:14:24]

nit: "x" variable name.

[nishanths, 2016/08/12 02:37:39]

Done: used "other".

+	if info == nil {

[martiniss, 2016/08/12 00:14:24]

Why would info ever be nil?

[nishanths, 2016/08/12 02:37:39]

Removed the check (also removed at similar locations).

Unlikely `info` would be nil. 
If it were, we would panic below when dereferencing. 
panic seems to be acceptable behavior in this case.

+		return errors.New("model: Merge: nil *BuilderInfo")
+	}
+
+	if len(info.BuildNumbers) > 0 && len(x.BuildNumbers) > 0 {
+		if info.BuildNumbers[0] == x.BuildNumbers[0] {
+			return ErrBuildNumberConflict
+		}
+	}
+
+	info.SecondsEpoch = append(x.SecondsEpoch, info.SecondsEpoch...)
+	info.BlinkRevs = append(x.BlinkRevs, info.BlinkRevs...)
+	info.BuildNumbers = append(x.BuildNumbers, info.BuildNumbers...)
+	info.ChromeRevs = append(x.ChromeRevs, info.ChromeRevs...)
+
+	for k, v := range x.FailuresByType {
+		if info.FailuresByType == nil {

[martiniss, 2016/08/12 00:14:24]

Move outside for loop.

[nishanths, 2016/08/12 02:37:39]

Done.

+			info.FailuresByType = make(map[string][]int)
+		}
+		info.FailuresByType[k] = append(v, info.FailuresByType[k]...)
+	}
+
+	info.FailureMap = FailureLongNames
+
+	if info.Tests == nil {
+		info.Tests = AggregateTest{}
+	}
+
+	info.Tests.WalkLeaves(func(_ string, leaf *AggregateTestLeaf) {
+		leaf.Expected = nil
+		leaf.Bugs = nil
+	})
+
+	return info.Tests.Merge(x.Tests)
+}
+
+// Merge merges x into at. After merging, at's fields may reference the same
+// objects referenced by x.
+func (at *AggregateTest) Merge(x AggregateTest) error {
+	if at == nil {
+		return errors.New("model: Merge: nil *AggregateTest")
+	}
+
+	// Shallow copy but OK. We take care to not modify xcopy
+	// values; instead always create new objects
+	// and assign to xcopy[key].
+	xcopy := make(AggregateTest, len(x))

[martiniss, 2016/08/12 00:14:24]

shouldn't this be make([]AggregateTest, len(x)) ? Or does this work to create a
slice

[nishanths, 2016/08/12 02:37:39]

AggregateTest is a map[string]Node.

+	for k, v := range x {
+		xcopy[k] = v
+	}
+
+	for k, v := range *at {
+		if _, ok := xcopy[k]; !ok {
+			switch v.(type) {
+			case *AggregateTestLeaf:
+				l := &AggregateTestLeaf{}
+				l.defaultFields()
+				xcopy[k] = l
+			case AggregateTest:
+				xcopy[k] = AggregateTest{}
+			}
+		}
+	}
+
+	for k, v := range xcopy {
+		// Key does not exist: assign entire subtree.
+		if _, ok := (*at)[k]; !ok {
+			if *at == nil {
+				*at = AggregateTest{}
+			}
+			(*at)[k] = v
+			continue
+		}
+
+		// Leaf node.
+		if leaf1, ok := (*at)[k].(*AggregateTestLeaf); ok {
+			leaf2, ok := v.(*AggregateTestLeaf)
+			if !ok {
+				return errors.New("model: Merge: expected *AggregateTestLeaf")
+			}
+			if err := leaf1.Merge(leaf2); err != nil {
+				return err
+			}
+			continue
+		}
+
+		// Not leaf node: merge subtree recursively.
+		at1, ok := (*at)[k].(AggregateTest)
+		if !ok {
+			return errors.New("model: Merge: expected AggregateTest")
+		}
+		at2, ok := v.(AggregateTest)
+		if !ok {
+			return errors.New("model: Merge: expected AggregateTest")
+		}
+		if err := at1.Merge(at2); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// Merge merges x into leaf. After merging, leaf's fields may reference the same
+// objects referenced by x.
+func (leaf *AggregateTestLeaf) Merge(x *AggregateTestLeaf) error {
+	if leaf == nil {

[martiniss, 2016/08/12 00:14:24]

Same comment; seems unnecessary

[nishanths, 2016/08/12 02:37:39]

Done, removed check.

+		return errors.New("model: Merge: nil *AggregateTestLeaf")
+	}
+
+	// Bugs and Expected should come from from x only.
+	leaf.Bugs = x.Bugs
+	if len(x.Expected) == 1 && x.Expected[0] != "PASS" {
+		leaf.Expected = x.Expected
+	}
+
+	for _, r := range x.Results {
+		if len(leaf.Results) > 0 && r.Type == leaf.Results[0].Type {
+			leaf.Results[0].Count += r.Count
+		} else {
+			leaf.Results = append([]ResultSummary{r}, leaf.Results...)
+		}
+	}
+
+	for _, r := range x.Runtimes {
+		if len(leaf.Runtimes) > 0 && r.Runtime == leaf.Runtimes[0].Runtime {
+			leaf.Runtimes[0].Count += r.Count
+		} else {
+			leaf.Runtimes = append([]RuntimeSummary{r}, leaf.Runtimes...)
+		}
+	}
+
+	return nil
+}