NewDataFrameIterator
Change-Id: I94d8743a85869e884badb7603ddba177164e0817
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/301839
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
diff --git a/perf/go/regression/dfiter.go b/perf/go/regression/dfiter.go
index 6c177d8..9dd8c70 100644
--- a/perf/go/regression/dfiter.go
+++ b/perf/go/regression/dfiter.go
@@ -85,11 +85,29 @@
// sure that the size of the origin dataframe is the same size as the
// slicer size, so we set them both to 2*Radius+1.
n := int32(2*req.Alert.Radius + 1)
- // Need to find an End time, which is the commit time of the commit at Offset+Radius.
- commit, err := perfGit.CommitFromCommitNumber(ctx, types.CommitNumber(int(req.Domain.Offset)+req.Alert.Radius-1))
+ // Need to find an End time, which is the commit time of the commit at
+ // Offset+Radius.
+ //
+ // That is, for example, we are looking at commit 21 with a Radius of 3
+ // to request an endpoint of 24:
+ //
+ // [ 18, 19, 20, 21, 22, 23, 24]
+ //
+ // That way we have the right number of points for types.OriginalStep
+ // (2*n+1), and by chopping down the length of the result by 1 we can
+ // get a dataframe of the right length for the rest of the step finding
+ // algorithms, i.e.:
+ //
+ // [ 18, 19, 20, 21, 22, 23 ]
+ //
+ // All of these contortions are to keep the detection algorithms
+ // consistent. Eventually types.OriginalStep should be changed to work
+ // on a dataframe of length 2*n like all the rest.
+ endCommit := types.CommitNumber(int(req.Domain.Offset) + req.Alert.Radius)
+ commit, err := perfGit.CommitFromCommitNumber(ctx, endCommit)
if err != nil {
if regressionStateCallback != nil {
- regressionStateCallback(fmt.Sprintf("Not a valid commit number %d. Make sure you choose a commit old enough to have Radius results before and after it.", int(req.Domain.Offset)+req.Alert.Radius-1))
+ regressionStateCallback(fmt.Sprintf("Not a valid commit number %d. Make sure you choose a commit old enough to have Radius results before and after it.", endCommit))
}
return nil, skerr.Wrapf(err, "Failed to look up CommitNumber of a single cluster request")
diff --git a/perf/go/regression/dfiter_test.go b/perf/go/regression/dfiter_test.go
index c83b865..3bc4914 100644
--- a/perf/go/regression/dfiter_test.go
+++ b/perf/go/regression/dfiter_test.go
@@ -180,6 +180,33 @@
require.Contains(t, err.Error(), "Failed to look up CommitNumber")
}
+func TestNewDataFrameIterator_ExactDataframeRequest_Success(t *testing.T) {
+ unittest.LargeTest(t)
+ ctx, dfb, g, cleanup := newForTest(t)
+ defer cleanup()
+
+ // This is an ExactDataframeRequest because Offset != 0.
+ request := &RegressionDetectionRequest{
+ Alert: &alerts.Alert{
+ Radius: 1,
+ },
+ Domain: types.Domain{
+ N: 2,
+ Offset: 6, // Start at 6 with a radius of 1 to get the commit at 7.
+ },
+ Query: "arch=x86",
+ }
+ iter, err := NewDataFrameIterator(ctx, nil, request, dfb, g, nil)
+ require.NoError(t, err)
+ require.True(t, iter.Next())
+ df, err := iter.Value(ctx)
+ require.NoError(t, err)
+ assert.Equal(t, 3, len(df.Header))
+ assert.Equal(t, int64(0), df.Header[0].Offset)
+ assert.Equal(t, int64(1), df.Header[1].Offset)
+ assert.Equal(t, int64(7), df.Header[2].Offset)
+}
+
func TestNewDataFrameIterator_ExactDataframeRequest_ErrIfWeSearchBeforeFirstCommit(t *testing.T) {
unittest.LargeTest(t)
ctx, dfb, g, cleanup := newForTest(t)
