[tools][cts] Add results test coverage Makes the following changes: * Renames results.CleanTags() to results.CleanResults() since it modifies more than just tags. * Adds test coverage for results.GetResults() * Adds test coverage for results.CleanResults() Bug: chromium:342554800 Change-Id: I348e75a1b64d787181ef6fecbbc2dab2e7a2fb44 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/190565 Commit-Queue: Austin Eng <enga@chromium.org> Reviewed-by: Austin Eng <enga@chromium.org> Auto-Submit: Brian Sheedy <bsheedy@google.com>
diff --git a/tools/src/cmd/cts/common/results.go b/tools/src/cmd/cts/common/results.go index 21e2a34..bc50695 100644 --- a/tools/src/cmd/cts/common/results.go +++ b/tools/src/cmd/cts/common/results.go
@@ -154,10 +154,10 @@ } // CacheResults looks in the cache at 'cacheDir' for the results for the given patchset. -// If the cache contains the results, then these are loaded, transformed with CleanTags() and +// If the cache contains the results, then these are loaded, transformed with CleanResults() and // returned. // If the cache does not contain the results, then they are fetched using GetRawResults(), saved to -// the cache directory, transformed with CleanTags() and then returned. +// the cache directory, transformed with CleanResults() and then returned. func CacheResults( ctx context.Context, cfg Config, @@ -189,7 +189,7 @@ // Expand aliased tags, remove specific tags for i, results := range resultsByExecutionMode { - CleanTags(cfg, &results) + CleanResults(cfg, &results) results.Sort() resultsByExecutionMode[i] = results } @@ -197,7 +197,7 @@ return resultsByExecutionMode, nil } -// GetResults calls GetRawResults() to fetch the build results from ResultDB and applies CleanTags(). +// GetResults calls GetRawResults() to fetch the build results from ResultDB and applies CleanResults(). // GetResults does not trigger new builds. func GetResults( ctx context.Context, @@ -212,7 +212,7 @@ // Expand aliased tags, remove specific tags for i, results := range resultsByExecutionMode { - CleanTags(cfg, &results) + CleanResults(cfg, &results) results.Sort() resultsByExecutionMode[i] = results } @@ -220,7 +220,7 @@ return resultsByExecutionMode, err } -// GetRawResults fetches the build results from ResultDB, without applying CleanTags(). +// GetRawResults fetches the build results from ResultDB, without applying CleanResults(). // GetRawResults does not trigger new builds. func GetRawResults( ctx context.Context, @@ -383,10 +383,10 @@ return nil, gerrit.Patchset{}, fmt.Errorf("no builds found for change %v", change) } -// CleanTags modifies each result so that tags in cfg.Tag.Remove are removed and +// CleanResults modifies each result so that tags in cfg.Tag.Remove are removed and // duplicate results are removed by erring towards Failure. // See: crbug.com/dawn/1387, crbug.com/dawn/1401 -func CleanTags(cfg Config, results *result.List) { +func CleanResults(cfg Config, results *result.List) { // Remove any tags found in cfg.Tag.Remove remove := result.NewTags(cfg.Tag.Remove...) for _, r := range *results {
diff --git a/tools/src/cmd/cts/common/results_test.go b/tools/src/cmd/cts/common/results_test.go index 77ce620..5e041f6 100644 --- a/tools/src/cmd/cts/common/results_test.go +++ b/tools/src/cmd/cts/common/results_test.go
@@ -39,6 +39,13 @@ "github.com/stretchr/testify/assert" ) +// TODO(crbug.com/342554800): Add test coverage for: +// ResultSource.GetResults (requires breaking out into helper functions) +// CacheResults +// LatestCTSRoll +// LatestPatchset +// MostRecentResultsForChange + /******************************************************************************* * Fake implementations ******************************************************************************/ @@ -59,10 +66,10 @@ } /******************************************************************************* - * GetRawResults tests + * GetResults tests ******************************************************************************/ -func generateGoodGetRawResultsInputs() (context.Context, Config, *mockedBigQueryClient, BuildsByName) { +func generateGoodGetResultsInputs() (context.Context, Config, *mockedBigQueryClient, BuildsByName) { ctx := context.Background() cfg := Config{ @@ -90,9 +97,85 @@ return ctx, cfg, client, builds } +// Tests that valid results are properly cleaned, sorted, and returned. +func TestGetResultsHappyPath(t *testing.T) { + ctx, cfg, client, builds := generateGoodGetResultsInputs() + + cfg.Tag.Remove = []string{ + "remove_me", + } + + client.returnValues = []resultsdb.QueryResult{ + resultsdb.QueryResult{ + TestId: "prefix_test_2", + Status: "PASS", + Tags: []resultsdb.TagPair{ + resultsdb.TagPair{ + Key: "typ_tag", + Value: "remove_me", + }, + resultsdb.TagPair{ + Key: "typ_tag", + Value: "win", + }, + }, + Duration: 2.0, + }, + resultsdb.QueryResult{ + TestId: "prefix_test_1", + Status: "PASS", + Tags: []resultsdb.TagPair{ + resultsdb.TagPair{ + Key: "typ_tag", + Value: "linux", + }, + }, + Duration: 1.0, + }, + } + + expectedResultsList := result.List{ + result.Result{ + Query: query.Parse("_test_1"), + Status: result.Pass, + Tags: result.NewTags("linux"), + Duration: time.Second, + MayExonerate: false, + }, + result.Result{ + Query: query.Parse("_test_2"), + Status: result.Pass, + Tags: result.NewTags("win"), + Duration: 2 * time.Second, + MayExonerate: false, + }, + } + + expectedResults := make(result.ResultsByExecutionMode) + expectedResults["execution_mode"] = expectedResultsList + + results, err := GetResults(ctx, cfg, client, builds) + assert.Nil(t, err) + assert.Equal(t, results, expectedResults) +} + +// Tests that errors from GetRawResults are properly surfaced. +func TestGetResultsGetRawResultsErrorSurfaced(t *testing.T) { + ctx, cfg, client, builds := generateGoodGetResultsInputs() + client.returnValues[0].TestId = "bad_test" + + results, err := GetResults(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.") +} + +/******************************************************************************* + * GetRawResults tests + ******************************************************************************/ + // Tests that valid results are properly parsed and returned. func TestGetRawResultsHappyPath(t *testing.T) { - ctx, cfg, client, builds := generateGoodGetRawResultsInputs() + ctx, cfg, client, builds := generateGoodGetResultsInputs() client.returnValues = []resultsdb.QueryResult{ resultsdb.QueryResult{ TestId: "prefix_test_1", @@ -180,7 +263,7 @@ // Tests that a mismatched prefix results in an error. func TestGetRawResultsPrefixMismatch(t *testing.T) { - ctx, cfg, client, builds := generateGoodGetRawResultsInputs() + ctx, cfg, client, builds := generateGoodGetResultsInputs() client.returnValues[0].TestId = "bad_test" results, err := GetRawResults(ctx, cfg, client, builds) @@ -190,7 +273,7 @@ // Tests that a JavaScript duration that cannot be parsed results in an error. func TestGetRawResultsBadJavaScriptDuration(t *testing.T) { - ctx, cfg, client, builds := generateGoodGetRawResultsInputs() + ctx, cfg, client, builds := generateGoodGetResultsInputs() client.returnValues[0].Tags = []resultsdb.TagPair{ resultsdb.TagPair{ Key: "javascript_duration", @@ -205,7 +288,7 @@ // Tests that a non-boolean may_exonerate value results in an error. func TestGetRawResultsBadMayExonerate(t *testing.T) { - ctx, cfg, client, builds := generateGoodGetRawResultsInputs() + ctx, cfg, client, builds := generateGoodGetResultsInputs() client.returnValues[0].Tags = []resultsdb.TagPair{ resultsdb.TagPair{ Key: "may_exonerate", @@ -217,3 +300,194 @@ assert.Nil(t, results) assert.ErrorContains(t, err, `strconv.ParseBool: parsing "yesnt": invalid syntax`) } + +/******************************************************************************* + * CleanResults tests + ******************************************************************************/ + +// Tests that tags specified for removal are properly removed. +func TestCleanResultsTagRemoval(t *testing.T) { + cfg := Config{} + cfg.Tag.Remove = []string{ + "remove_1", + "remove_2", + "remove_3", + "missing", + } + + results := result.List{ + result.Result{ + Query: query.Parse("test_1"), + Status: result.Pass, + Tags: result.NewTags("remove_1", "remove_2", "linux"), + Duration: time.Second, + MayExonerate: false, + }, + result.Result{ + Query: query.Parse("test_2"), + Status: result.Pass, + Tags: result.NewTags("remove_2", "remove_3"), + Duration: time.Second, + MayExonerate: false, + }, + } + + expectedResults := result.List{ + result.Result{ + Query: query.Parse("test_1"), + Status: result.Pass, + Tags: result.NewTags("linux"), + Duration: time.Second, + MayExonerate: false, + }, + result.Result{ + Query: query.Parse("test_2"), + Status: result.Pass, + Tags: result.NewTags(), + Duration: time.Second, + MayExonerate: false, + }, + } + + CleanResults(cfg, &results) + assert.Equal(t, results, expectedResults) +} + +// Tests that duplicate results with the same status always use that status. +func TestCleanResultsDuplicateResultSameStatus(t *testing.T) { + cfg := Config{} + for _, status := range []result.Status{result.Abort, result.Crash, + result.Failure, result.Pass, result.RetryOnFailure, result.Skip, result.Slow, result.Unknown} { + results := result.List{ + result.Result{ + Query: query.Parse("test_1"), + Status: status, + Tags: result.NewTags(), + Duration: time.Second, + MayExonerate: false, + }, + result.Result{ + Query: query.Parse("test_1"), + Status: status, + Tags: result.NewTags(), + Duration: 3 * time.Second, + MayExonerate: false, + }, + } + + expectedResults := result.List{ + result.Result{ + Query: query.Parse("test_1"), + Status: status, + Tags: result.NewTags(), + Duration: 2 * time.Second, + MayExonerate: false, + }, + } + + CleanResults(cfg, &results) + assert.Equal(t, results, expectedResults) + } +} + +func runPriorityTest(t *testing.T, testedStatus result.Status, lowerPriorityStatuses *[]result.Status) { + cfg := Config{} + for _, status := range *lowerPriorityStatuses { + results := result.List{ + result.Result{ + Query: query.Parse("test_1"), + Status: status, + Tags: result.NewTags(), + Duration: time.Second, + MayExonerate: false, + }, + result.Result{ + Query: query.Parse("test_1"), + Status: testedStatus, + Tags: result.NewTags(), + Duration: 3 * time.Second, + MayExonerate: false, + }, + } + + expectedResults := result.List{ + result.Result{ + Query: query.Parse("test_1"), + Status: testedStatus, + Tags: result.NewTags(), + Duration: 2 * time.Second, + MayExonerate: false, + }, + } + + CleanResults(cfg, &results) + assert.Equal(t, results, expectedResults) + } +} + +// Tests that Crash has the highest priority among statuses. +func TestCleanResultsReplaceResultCrash(t *testing.T) { + lowerPriorityStatuses := []result.Status{result.Abort, result.Failure, result.Slow, result.Pass, result.RetryOnFailure, result.Skip, result.Unknown} + runPriorityTest(t, result.Crash, &lowerPriorityStatuses) +} + +// Tests that Abort has the second highest priority among statuses. +func TestCleanResultsReplaceResultAbort(t *testing.T) { + lowerPriorityStatuses := []result.Status{result.Failure, result.Slow, result.Pass, result.RetryOnFailure, result.Skip, result.Unknown} + runPriorityTest(t, result.Abort, &lowerPriorityStatuses) +} + +// Tests that Failure has the third highest priority among statuses. +func TestCleanResultsReplaceResultFailure(t *testing.T) { + lowerPriorityStatuses := []result.Status{result.Slow, result.Pass, result.RetryOnFailure, result.Skip, result.Unknown} + runPriorityTest(t, result.Failure, &lowerPriorityStatuses) +} + +// Tests that Slow has the fourth highest priority among statuses. +func TestCleanResultsReplaceResultSlow(t *testing.T) { + lowerPriorityStatuses := []result.Status{result.Pass, result.RetryOnFailure, result.Skip, result.Unknown} + runPriorityTest(t, result.Slow, &lowerPriorityStatuses) +} + +// Tests that statuses without explicit priority default to Failure. +func TestCleanResultsReplacEresultDefault(t *testing.T) { + cfg := Config{} + statuses := []result.Status{result.Pass, result.RetryOnFailure, result.Skip, result.Unknown} + for i, leftStatus := range statuses { + for j, rightStatus := range statuses { + if i == j { + continue + } + + results := result.List{ + result.Result{ + Query: query.Parse("test_1"), + Status: leftStatus, + Tags: result.NewTags(), + Duration: time.Second, + MayExonerate: false, + }, + result.Result{ + Query: query.Parse("test_1"), + Status: rightStatus, + Tags: result.NewTags(), + Duration: 3 * time.Second, + MayExonerate: false, + }, + } + + expectedResults := result.List{ + result.Result{ + Query: query.Parse("test_1"), + Status: result.Failure, + Tags: result.NewTags(), + Duration: 2 * time.Second, + MayExonerate: false, + }, + } + + CleanResults(cfg, &results) + assert.Equal(t, results, expectedResults) + } + } +}