[tools][cts] Add GetRawResults test coverage
Adds test coverage for the results.GetRawResults function.
Bug: chromium:342554800, chromium:342446313
Change-Id: Ib926c539dc3a3a63e86999b1a205aaf3a588d24a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/189693
Auto-Submit: Brian Sheedy <bsheedy@google.com>
Commit-Queue: Austin Eng <enga@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/go.mod b/go.mod
index e6f538b..e3391d7 100644
--- a/go.mod
+++ b/go.mod
@@ -15,6 +15,7 @@
github.com/mzohreva/gographviz v0.0.0-20180226085351-533f4a37d9c6
github.com/sergi/go-diff v1.3.1
github.com/shirou/gopsutil v3.21.11+incompatible
+ github.com/stretchr/testify v1.8.2
github.com/tidwall/jsonc v0.3.2
go.chromium.org/luci v0.0.0-20230311013728-313c8e2205bc
golang.org/x/net v0.17.0
@@ -34,6 +35,7 @@
github.com/apache/thrift v0.16.0 // indirect
github.com/chromedp/cdproto v0.0.0-20231011050154-1d073bb38998 // indirect
github.com/chromedp/sysutil v1.0.0 // indirect
+ github.com/davecgh/go-spew v1.1.1 // indirect
github.com/go-ole/go-ole v1.2.6 // indirect
github.com/gobwas/httphead v0.1.0 // indirect
github.com/gobwas/pool v0.2.1 // indirect
@@ -63,6 +65,7 @@
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mzohreva/GoGraphviz v0.0.0-20180226085351-533f4a37d9c6 // indirect
github.com/pierrec/lz4/v4 v4.1.15 // indirect
+ github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rogpeppe/go-internal v1.9.0 // indirect
github.com/texttheater/golang-levenshtein v1.0.1 // indirect
github.com/tklauser/go-sysconf v0.3.10 // indirect
@@ -82,6 +85,7 @@
google.golang.org/genproto v0.0.0-20230706204954-ccb25ca9f130 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20230706204954-ccb25ca9f130 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20230711160842-782d3b101e98 // indirect
+ gopkg.in/yaml.v3 v3.0.1 // indirect
)
exclude github.com/sergi/go-diff v1.2.0
diff --git a/go.sum b/go.sum
index 2e106ed..80595c5 100644
--- a/go.sum
+++ b/go.sum
@@ -187,6 +187,8 @@
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
+github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8=
+github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
github.com/texttheater/golang-levenshtein v1.0.1 h1:+cRNoVrfiwufQPhoMzB6N0Yf/Mqajr6t1lOv8GyGE2U=
github.com/texttheater/golang-levenshtein v1.0.1/go.mod h1:PYAKrbF5sAiq9wd+H82hs7gNaen0CplQ9uvm6+enD/8=
github.com/tidwall/jsonc v0.3.2 h1:ZTKrmejRlAJYdn0kcaFqRAKlxxFIC21pYq8vLa4p2Wc=
diff --git a/tools/src/cmd/cts/common/config.go b/tools/src/cmd/cts/common/config.go
index 978cedf..2b4e1ad 100644
--- a/tools/src/cmd/cts/common/config.go
+++ b/tools/src/cmd/cts/common/config.go
@@ -43,12 +43,7 @@
// tools/src/cmd/cts/config.json.
type Config struct {
// Test holds configuration data for test results.
- Tests []struct {
- // Mode used to refer to tests
- ExecutionMode result.ExecutionMode
- // The ResultDB string prefix for CTS tests.
- Prefixes []string
- }
+ Tests []TestConfig
// Gerrit holds configuration for Dawn's Gerrit server.
Gerrit struct {
// The host URL
@@ -79,6 +74,14 @@
}
}
+// TestConfig holds configuration data for a single test type.
+type TestConfig struct {
+ // Mode used to refer to tests
+ ExecutionMode result.ExecutionMode
+ // The ResultDB string prefix for CTS tests.
+ Prefixes []string
+}
+
// GitProject holds a git host URL and project.
type GitProject struct {
Host string
diff --git a/tools/src/cmd/cts/common/results.go b/tools/src/cmd/cts/common/results.go
index c2b6240..21e2a34 100644
--- a/tools/src/cmd/cts/common/results.go
+++ b/tools/src/cmd/cts/common/results.go
@@ -163,7 +163,7 @@
cfg Config,
ps gerrit.Patchset,
cacheDir string,
- client *resultsdb.BigQueryClient,
+ client resultsdb.Querier,
builds BuildsByName) (result.ResultsByExecutionMode, error) {
var cachePath string
@@ -202,7 +202,7 @@
func GetResults(
ctx context.Context,
cfg Config,
- client *resultsdb.BigQueryClient,
+ client resultsdb.Querier,
builds BuildsByName) (result.ResultsByExecutionMode, error) {
resultsByExecutionMode, err := GetRawResults(ctx, cfg, client, builds)
@@ -225,7 +225,7 @@
func GetRawResults(
ctx context.Context,
cfg Config,
- client *resultsdb.BigQueryClient,
+ client resultsdb.Querier,
builds BuildsByName) (result.ResultsByExecutionMode, error) {
fmt.Printf("fetching results from resultdb...")
@@ -350,7 +350,7 @@
cacheDir string,
g *gerrit.Gerrit,
bb *buildbucket.Buildbucket,
- client *resultsdb.BigQueryClient,
+ client resultsdb.Querier,
change int) (result.ResultsByExecutionMode, gerrit.Patchset, error) {
ps, err := LatestPatchset(g, change)
diff --git a/tools/src/cmd/cts/common/results_test.go b/tools/src/cmd/cts/common/results_test.go
new file mode 100644
index 0000000..77ce620
--- /dev/null
+++ b/tools/src/cmd/cts/common/results_test.go
@@ -0,0 +1,219 @@
+// Copyright 2024 The Dawn & Tint Authors
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+// 1. Redistributions of source code must retain the above copyright notice, this
+// list of conditions and the following disclaimer.
+//
+// 2. Redistributions in binary form must reproduce the above copyright notice,
+// this list of conditions and the following disclaimer in the documentation
+// and/or other materials provided with the distribution.
+//
+// 3. Neither the name of the copyright holder nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+// FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+package common
+
+import (
+ "context"
+ "testing"
+ "time"
+
+ "dawn.googlesource.com/dawn/tools/src/buildbucket"
+ "dawn.googlesource.com/dawn/tools/src/cts/query"
+ "dawn.googlesource.com/dawn/tools/src/cts/result"
+ "dawn.googlesource.com/dawn/tools/src/resultsdb"
+ "github.com/stretchr/testify/assert"
+)
+
+/*******************************************************************************
+ * Fake implementations
+ ******************************************************************************/
+
+// A fake version of dawn/tools/src/resultsdb's BigQueryClient.
+type mockedBigQueryClient struct {
+ returnValues []resultsdb.QueryResult
+}
+
+func (bq mockedBigQueryClient) QueryTestResults(
+ ctx context.Context, builds []buildbucket.BuildID, testPrefix string, f func(*resultsdb.QueryResult) error) error {
+ for _, result := range bq.returnValues {
+ if err := f(&result); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+/*******************************************************************************
+ * GetRawResults tests
+ ******************************************************************************/
+
+func generateGoodGetRawResultsInputs() (context.Context, Config, *mockedBigQueryClient, BuildsByName) {
+ ctx := context.Background()
+
+ cfg := Config{
+ Tests: []TestConfig{
+ TestConfig{
+ ExecutionMode: result.ExecutionMode("execution_mode"),
+ Prefixes: []string{"prefix"},
+ },
+ },
+ }
+
+ client := &mockedBigQueryClient{
+ returnValues: []resultsdb.QueryResult{
+ resultsdb.QueryResult{
+ TestId: "prefix_test",
+ Status: "PASS",
+ Tags: []resultsdb.TagPair{},
+ Duration: 1.0,
+ },
+ },
+ }
+
+ builds := make(BuildsByName)
+
+ return ctx, cfg, client, builds
+}
+
+// Tests that valid results are properly parsed and returned.
+func TestGetRawResultsHappyPath(t *testing.T) {
+ ctx, cfg, client, builds := generateGoodGetRawResultsInputs()
+ client.returnValues = []resultsdb.QueryResult{
+ resultsdb.QueryResult{
+ TestId: "prefix_test_1",
+ Status: "PASS",
+ Tags: []resultsdb.TagPair{},
+ Duration: 1.0,
+ },
+ resultsdb.QueryResult{
+ TestId: "prefix_test_2",
+ Status: "FAIL",
+ Tags: []resultsdb.TagPair{
+ resultsdb.TagPair{
+ Key: "javascript_duration",
+ Value: "0.5s",
+ },
+ },
+ Duration: 2.0,
+ },
+ resultsdb.QueryResult{
+ TestId: "prefix_test_3",
+ Status: "SKIP",
+ Tags: []resultsdb.TagPair{
+ resultsdb.TagPair{
+ Key: "may_exonerate",
+ Value: "true",
+ },
+ },
+ Duration: 3.0,
+ },
+ resultsdb.QueryResult{
+ TestId: "prefix_test_4",
+ Status: "SomeStatus",
+ Tags: []resultsdb.TagPair{
+ resultsdb.TagPair{
+ Key: "typ_tag",
+ Value: "linux",
+ },
+ resultsdb.TagPair{
+ Key: "typ_tag",
+ Value: "intel",
+ },
+ },
+ Duration: 4.0,
+ },
+ }
+
+ expectedResultsList := result.List{
+ result.Result{
+ Query: query.Parse("_test_1"),
+ Status: result.Pass,
+ Tags: result.NewTags(),
+ Duration: time.Second,
+ MayExonerate: false,
+ },
+ result.Result{
+ Query: query.Parse("_test_2"),
+ Status: result.Failure,
+ Tags: result.NewTags(),
+ Duration: 500 * time.Millisecond,
+ MayExonerate: false,
+ },
+ result.Result{
+ Query: query.Parse("_test_3"),
+ Status: result.Skip,
+ Tags: result.NewTags(),
+ Duration: 3 * time.Second,
+ MayExonerate: true,
+ },
+ result.Result{
+ Query: query.Parse("_test_4"),
+ Status: result.Unknown,
+ Tags: result.NewTags("linux", "intel"),
+ Duration: 4 * time.Second,
+ MayExonerate: false,
+ },
+ }
+
+ expectedResults := make(result.ResultsByExecutionMode)
+ expectedResults["execution_mode"] = expectedResultsList
+
+ results, err := GetRawResults(ctx, cfg, client, builds)
+ assert.Nil(t, err)
+ assert.Equal(t, results, expectedResults)
+}
+
+// Tests that a mismatched prefix results in an error.
+func TestGetRawResultsPrefixMismatch(t *testing.T) {
+ ctx, cfg, client, builds := generateGoodGetRawResultsInputs()
+ client.returnValues[0].TestId = "bad_test"
+
+ results, err := GetRawResults(ctx, cfg, client, builds)
+ assert.Nil(t, results)
+ assert.ErrorContains(t, err, "Test ID bad_test did not start with prefix even though query should have filtered.")
+}
+
+// Tests that a JavaScript duration that cannot be parsed results in an error.
+func TestGetRawResultsBadJavaScriptDuration(t *testing.T) {
+ ctx, cfg, client, builds := generateGoodGetRawResultsInputs()
+ client.returnValues[0].Tags = []resultsdb.TagPair{
+ resultsdb.TagPair{
+ Key: "javascript_duration",
+ Value: "1000foo",
+ },
+ }
+
+ results, err := GetRawResults(ctx, cfg, client, builds)
+ assert.Nil(t, results)
+ assert.ErrorContains(t, err, `time: unknown unit "foo" in duration "1000foo"`)
+}
+
+// Tests that a non-boolean may_exonerate value results in an error.
+func TestGetRawResultsBadMayExonerate(t *testing.T) {
+ ctx, cfg, client, builds := generateGoodGetRawResultsInputs()
+ client.returnValues[0].Tags = []resultsdb.TagPair{
+ resultsdb.TagPair{
+ Key: "may_exonerate",
+ Value: "yesnt",
+ },
+ }
+
+ results, err := GetRawResults(ctx, cfg, client, builds)
+ assert.Nil(t, results)
+ assert.ErrorContains(t, err, `strconv.ParseBool: parsing "yesnt": invalid syntax`)
+}
diff --git a/tools/src/cts/expectations/expectations_test.go b/tools/src/cts/expectations/expectations_test.go
index c12945e..3790d38 100644
--- a/tools/src/cts/expectations/expectations_test.go
+++ b/tools/src/cts/expectations/expectations_test.go
@@ -28,15 +28,15 @@
package expectations
import (
- "testing"
+ "testing"
- "github.com/google/go-cmp/cmp"
+ "github.com/google/go-cmp/cmp"
)
// Tests behavior of Content.Format()
func TestContentFormat(t *testing.T) {
- // Intentionally includes extra whitespace since that should be removed.
- content := `# OS
+ // Intentionally includes extra whitespace since that should be removed.
+ content := `# OS
# tags: [ linux mac win ]
# GPU
# tags: [ amd intel nvidia ]
@@ -77,7 +77,7 @@
crbug.com/2 [ amd ] b [ Failure ]
`
- expected_content := `# OS
+ expected_content := `# OS
# tags: [ linux mac win ]
# GPU
# tags: [ amd intel nvidia ]
@@ -113,13 +113,13 @@
crbug.com/3 [ intel ] c [ Failure ]
crbug.com/3 [ intel ] d [ Failure ]
`
- expectations, err := Parse("", content)
- if err != nil {
- t.Errorf("Parsing content failed: %s", err.Error())
- }
- expectations.Format()
+ expectations, err := Parse("", content)
+ if err != nil {
+ t.Errorf("Parsing content failed: %s", err.Error())
+ }
+ expectations.Format()
- if diff := cmp.Diff(expectations.String(), expected_content); diff != "" {
- t.Errorf("Format produced unexpected output: %v", diff)
- }
+ if diff := cmp.Diff(expectations.String(), expected_content); diff != "" {
+ t.Errorf("Format produced unexpected output: %v", diff)
+ }
}
diff --git a/tools/src/resultsdb/resultsdb.go b/tools/src/resultsdb/resultsdb.go
index 66a3b9b..9c6d972 100644
--- a/tools/src/resultsdb/resultsdb.go
+++ b/tools/src/resultsdb/resultsdb.go
@@ -38,6 +38,10 @@
"google.golang.org/api/iterator"
)
+type Querier interface {
+ QueryTestResults(ctx context.Context, builds []buildbucket.BuildID, testPrefix string, f func(*QueryResult) error) error
+}
+
// BigQueryClient is a wrapper around bigquery.Client so that we can define new
// methods.
type BigQueryClient struct {
@@ -47,15 +51,18 @@
// QueryResult contains all of the data for a single test result from a ResultDB
// BigQuery query.
type QueryResult struct {
- TestId string
- Status string
- Tags []struct {
- Key string
- Value string
- }
+ TestId string
+ Status string
+ Tags []TagPair
Duration float64
}
+// TagPair is a key/value pair representing a ResultDB tag.
+type TagPair struct {
+ Key string
+ Value string
+}
+
// DefaultQueryProject is the default BigQuery project to use when running
// queries.
const DefaultQueryProject string = "chrome-unexpected-pass-data"
@@ -83,7 +90,7 @@
//
// f is called once per result and is expected to handle any processing or
// storage of results.
-func (bq *BigQueryClient) QueryTestResults(
+func (bq BigQueryClient) QueryTestResults(
ctx context.Context, builds []buildbucket.BuildID, testPrefix string, f func(*QueryResult) error) error {
// test_id gets renamed since the column names need to match the struct names
// unless we want to get results in a generic bigquery.Value slice and