storage: make benchfmt.Reader use Scanner-style interface
Change-Id: I052c2b3cc3440f081ef11ba5d999bf8e7f230c6a
Reviewed-on: https://go-review.googlesource.com/35450
Reviewed-by: Russ Cox <rsc@golang.org>
diff --git a/storage/app/query_test.go b/storage/app/query_test.go
index ac689a9..d91e56c 100644
--- a/storage/app/query_test.go
+++ b/storage/app/query_test.go
@@ -6,7 +6,6 @@
import (
"fmt"
- "io"
"mime/multipart"
"net/http"
"net/url"
@@ -65,10 +64,10 @@
}
br := benchfmt.NewReader(resp.Body)
for i, num := range test.want {
- r, err := br.Next()
- if err != nil {
- t.Fatalf("#%d: Next() = %v, want nil", i, err)
+ if !br.Next() {
+ t.Fatalf("#%d: Next() = false, want true (Err() = %v)", i, br.Err())
}
+ r := br.Result()
if r.Labels["label0"] != fmt.Sprintf("%d", num) {
t.Errorf("#%d: label0 = %q, want %d", i, r.Labels["label0"], num)
}
@@ -79,9 +78,11 @@
t.Errorf("#%d: by = %q, want %q", i, r.Labels["uploader"], "user")
}
}
- _, err = br.Next()
- if err != io.EOF {
- t.Errorf("Next() = %v, want EOF", err)
+ if br.Next() {
+ t.Fatalf("Next() = true, want false")
+ }
+ if err := br.Err(); err != nil {
+ t.Errorf("Err() = %v, want nil", err)
}
})
}
diff --git a/storage/app/upload.go b/storage/app/upload.go
index 475ffca..4759f59 100644
--- a/storage/app/upload.go
+++ b/storage/app/upload.go
@@ -194,20 +194,17 @@
br := benchfmt.NewReader(tr)
br.AddLabels(meta)
i := 0
- for {
- result, err := br.Next()
- if err != nil {
- if err != io.EOF {
- return err
- }
- if i == 0 {
- return errors.New("no valid benchmark lines found")
- }
- return nil
- }
+ for br.Next() {
i++
- if err := upload.InsertRecord(result); err != nil {
+ if err := upload.InsertRecord(br.Result()); err != nil {
return err
}
}
+ if err := br.Err(); err != nil {
+ return err
+ }
+ if i == 0 {
+ return errors.New("no valid benchmark lines found")
+ }
+ return nil
}
diff --git a/storage/benchfmt/benchfmt.go b/storage/benchfmt/benchfmt.go
index e31b580..00e696a 100644
--- a/storage/benchfmt/benchfmt.go
+++ b/storage/benchfmt/benchfmt.go
@@ -18,6 +18,15 @@
)
// Reader reads benchmark results from an io.Reader.
+// Use Next to advance through the results.
+//
+// br := benchfmt.NewReader(r)
+// for br.Next() {
+// res := br.Result()
+// ...
+// }
+// err = br.Err() // get any error encountered during iteration
+// ...
type Reader struct {
s *bufio.Scanner
labels Labels
@@ -25,10 +34,11 @@
// file or provided by AddLabels. They cannot be overridden.
permLabels Labels
lineNum int
+ // cached from the last call to Next
+ result *Result
+ err error
}
-// TODO(quentin): Make Reader have a Scanner-style interface instead, to match db.Query.
-
// NewReader creates a BenchmarkReader that reads from r.
func NewReader(r io.Reader) *Reader {
return &Reader{
@@ -175,7 +185,10 @@
// Next returns the next benchmark result from the file. If there are
// no further results, it returns nil, io.EOF.
-func (r *Reader) Next() (*Result, error) {
+func (r *Reader) Next() bool {
+ if r.err != nil {
+ return false
+ }
copied := false
havePerm := r.permLabels != nil
for r.s.Scan() {
@@ -208,13 +221,29 @@
}
}
if fullName, ok := parseBenchmarkLine(line); ok {
- return newResult(r.labels, r.lineNum, fullName, line), nil
+ r.result = newResult(r.labels, r.lineNum, fullName, line)
+ return true
}
}
if err := r.s.Err(); err != nil {
- return nil, err
+ r.err = err
+ return false
}
- return nil, io.EOF
+ r.err = io.EOF
+ return false
+}
+
+// Result returns the most recent result generated by a call to Next.
+func (r *Reader) Result() *Result {
+ return r.result
+}
+
+// Err returns the error state of the reader.
+func (r *Reader) Err() error {
+ if r.err == io.EOF {
+ return nil
+ }
+ return r.err
}
// parseKeyValueLine attempts to parse line as a key: value pair. ok
diff --git a/storage/benchfmt/benchfmt_test.go b/storage/benchfmt/benchfmt_test.go
index 2bde473..17c9340 100644
--- a/storage/benchfmt/benchfmt_test.go
+++ b/storage/benchfmt/benchfmt_test.go
@@ -7,7 +7,6 @@
import (
"bytes"
"fmt"
- "io"
"io/ioutil"
"os"
"os/exec"
@@ -18,17 +17,13 @@
func readAllResults(t *testing.T, r *Reader) []*Result {
var out []*Result
- for {
- result, err := r.Next()
- switch err {
- case io.EOF:
- return out
- case nil:
- out = append(out, result)
- default:
- t.Fatal(err)
- }
+ for r.Next() {
+ out = append(out, r.Result())
}
+ if err := r.Err(); err != nil {
+ t.Fatal(err)
+ }
+ return out
}
func TestBenchmarkReader(t *testing.T) {
diff --git a/storage/cmd/reindex/reindex.go b/storage/cmd/reindex/reindex.go
index e18a5c6..b1020ba 100644
--- a/storage/cmd/reindex/reindex.go
+++ b/storage/cmd/reindex/reindex.go
@@ -16,7 +16,6 @@
"context"
"flag"
"fmt"
- "io"
"log"
"os"
"strings"
@@ -131,17 +130,13 @@
}
defer r.Close()
br := benchfmt.NewReader(r)
- for {
- res, err := br.Next()
- if err == io.EOF {
- break
- }
- if err != nil {
+ for br.Next() {
+ if err := u.InsertRecord(br.Result()); err != nil {
return err
}
- if err := u.InsertRecord(res); err != nil {
- return err
- }
+ }
+ if err := br.Err(); err != nil {
+ return err
}
return nil
}
diff --git a/storage/db/db.go b/storage/db/db.go
index 385dc09..a87be2e 100644
--- a/storage/db/db.go
+++ b/storage/db/db.go
@@ -474,7 +474,15 @@
return false
}
// TODO(quentin): Needs to change when one row contains multiple Results.
- q.result, q.err = benchfmt.NewReader(bytes.NewReader(content)).Next()
+ br := benchfmt.NewReader(bytes.NewReader(content))
+ if !br.Next() {
+ q.err = br.Err()
+ if q.err == nil {
+ q.err = io.ErrUnexpectedEOF
+ }
+ return false
+ }
+ q.result = br.Result()
return q.err == nil
}
diff --git a/storage/db/db_test.go b/storage/db/db_test.go
index 13ee26a..e376cda 100644
--- a/storage/db/db_test.go
+++ b/storage/db/db_test.go
@@ -188,13 +188,11 @@
key: value
BenchmarkName 1 ns/op
`))
-
- r, err := br.Next()
- if err != nil {
- t.Fatalf("BenchmarkReader.Next: %v", err)
+ if !br.Next() {
+ t.Fatalf("unable to read test string: %v", br.Err())
}
- if err := u.InsertRecord(r); err != nil {
+ if err := u.InsertRecord(br.Result()); err != nil {
t.Fatalf("InsertRecord: %v", err)
}
