[tools][cts] Default to immutable chunks
Switches the CTS roller's logic to assume all expectation chunks are
immutable unless one of the following is true:
1. The chunk is one of the auto-generated ones from the roller
2. The chunk is explicitly marked as MUTABLE
This is the opposite of the current behavior, where all chunks are
assumed to be mutable unless they are marked with KEEP. In practice,
this seems to make the roller behave more consistently, as it only
needs to produce new expectations instead of trying to refactor
existing ones.
This can potentially lead to cases where the roller will get stuck
such as if a RetryOnFailure expectation failure exists already and
the test starts to fail consistently. However, this should (hopefully)
still result in more consistent rolls since this is not a common
scenario. The resulting fix should also be simpler than trying to
resolve conflicts, as it will just require updating the existing
RetryOnFailure expectations to Failure.
The old KEEP annotations will be kept around for now in case this
CL ends up needing to be reverted, but they can be removed in the
future.
Bug: chromium:345280734
Change-Id: I531422dfc8f500ecab1b525a166a0abf72a8a08f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/196236
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Auto-Submit: Brian Sheedy <bsheedy@google.com>
Reviewed-by: dan sinclair <dsinclair@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/tools/src/cts/expectations/update.go b/tools/src/cts/expectations/update.go
index 5cc0934..0d24fda 100644
--- a/tools/src/cts/expectations/update.go
+++ b/tools/src/cts/expectations/update.go
@@ -354,24 +354,26 @@
func (u *updater) build() error {
progress := Progress{}
- immutableTokens := []string{
- "KEEP",
- "BEGIN TAG HEADER",
- "Last rolled",
+ // All chunks other than the ones for untriaged flakes/failures produced by
+ // the CTS roller are considered immutable unless they are explicitly
+ // declared as mutable.
+ mutableTokens := []string{
+ newFlakesComment,
+ newFailuresComment,
+ "MUTABLE",
}
// Bin the chunks into those that contain any of the strings in
- // immutableTokens in the comments and those that do not have these strings.
+ // mutableTokens in the comments and those that do not have these strings.
immutableChunks, mutableChunks := []Chunk{}, []Chunk{}
for _, chunk := range u.in.Chunks {
- // Does the chunk comment contain 'KEEP' or 'BEGIN TAG HEADER' ?
- keep := false
+ keep := true
comments:
- for _, l := range chunk.Comments {
- for _, s := range immutableTokens {
- if strings.Contains(l, s) {
- keep = true
+ for _, line := range chunk.Comments {
+ for _, token := range mutableTokens {
+ if strings.Contains(line, token) {
+ keep = false
break comments
}
}
diff --git a/tools/src/cts/expectations/update_test.go b/tools/src/cts/expectations/update_test.go
index 818dc2a..c138c3a 100644
--- a/tools/src/cts/expectations/update_test.go
+++ b/tools/src/cts/expectations/update_test.go
@@ -67,8 +67,46 @@
{ //////////////////////////////////////////////////////////////////////
name: "no results found",
expectations: `
+# MUTABLE
crbug.com/a/123 a:missing,test,result:* [ Failure ]
crbug.com/a/123 [ tag ] another:missing,test,result:* [ Failure ]
+some:other,test:* [ Failure ]
+`,
+ results: result.List{
+ result.Result{
+ Query: Q("some:other,test:*"),
+ Tags: result.NewTags("os-a", "gpu-a"),
+ Status: result.Failure,
+ },
+ result.Result{
+ Query: Q("some:other,test:*"),
+ Tags: result.NewTags("os-b", "gpu-b"),
+ Status: result.Failure,
+ },
+ },
+ updated: `
+# MUTABLE
+some:other,test:* [ Failure ]
+crbug.com/a/123 a:missing,test,result:* [ Failure ]
+crbug.com/a/123 [ tag ] another:missing,test,result:* [ Failure ]
+`,
+ diagnostics: expectations.Diagnostics{
+ {
+ Severity: expectations.Note,
+ Line: headerLines + 3,
+ Message: "no results found for query 'a:missing,test,result:*'",
+ },
+ {
+ Severity: expectations.Note,
+ Line: headerLines + 4,
+ Message: "no results found for query 'another:missing,test,result:*' with tags [tag]",
+ },
+ },
+ },
+ { //////////////////////////////////////////////////////////////////////
+ name: "no results found immutable",
+ expectations: `
+crbug.com/a/123 a:missing,test,result:* [ Failure ]
some:other,test:* [ Failure ]
`,
@@ -86,7 +124,6 @@
},
updated: `
crbug.com/a/123 a:missing,test,result:* [ Failure ]
-crbug.com/a/123 [ tag ] another:missing,test,result:* [ Failure ]
some:other,test:* [ Failure ]
`,
@@ -96,50 +133,12 @@
Line: headerLines + 2,
Message: "no results found for query 'a:missing,test,result:*'",
},
- {
- Severity: expectations.Note,
- Line: headerLines + 3,
- Message: "no results found for query 'another:missing,test,result:*' with tags [tag]",
- },
- },
- },
- { //////////////////////////////////////////////////////////////////////
- name: "no results found KEEP",
- expectations: `
-# KEEP
-crbug.com/a/123 a:missing,test,result:* [ Failure ]
-
-some:other,test:* [ Failure ]
-`,
- results: result.List{
- result.Result{
- Query: Q("some:other,test:*"),
- Tags: result.NewTags("os-a", "gpu-a"),
- Status: result.Failure,
- },
- result.Result{
- Query: Q("some:other,test:*"),
- Tags: result.NewTags("os-b", "gpu-b"),
- Status: result.Failure,
- },
- },
- updated: `
-# KEEP
-crbug.com/a/123 a:missing,test,result:* [ Failure ]
-
-some:other,test:* [ Failure ]
-`,
- diagnostics: expectations.Diagnostics{
- {
- Severity: expectations.Note,
- Line: headerLines + 3,
- Message: "no results found for query 'a:missing,test,result:*'",
- },
},
},
{ //////////////////////////////////////////////////////////////////////
name: "unknown test",
expectations: `
+# MUTABLE
crbug.com/a/123 an:unknown,test:* [ Failure ]
crbug.com/a/123 [ tag ] another:unknown:test [ Failure ]
@@ -163,20 +162,19 @@
diagnostics: expectations.Diagnostics{
{
Severity: expectations.Warning,
- Line: headerLines + 2,
+ Line: headerLines + 3,
Message: "no tests exist with query 'an:unknown,test:*' - removing",
},
{
Severity: expectations.Warning,
- Line: headerLines + 3,
+ Line: headerLines + 4,
Message: "no tests exist with query 'another:unknown:test' - removing",
},
},
},
{ //////////////////////////////////////////////////////////////////////
- name: "unknown test found KEEP",
+ name: "unknown test found in immutable chunk",
expectations: `
-# KEEP
crbug.com/a/123 an:unknown,test:* [ Failure ]
some:other,test:* [ Failure ]
@@ -199,7 +197,7 @@
diagnostics: expectations.Diagnostics{
{
Severity: expectations.Warning,
- Line: headerLines + 3,
+ Line: headerLines + 2,
Message: "no tests exist with query 'an:unknown,test:*' - removing",
},
},
@@ -207,6 +205,78 @@
{ //////////////////////////////////////////////////////////////////////
name: "simple expectation with tags",
expectations: `
+# MUTABLE
+[ os-a ] a:b,c:* [ Failure ]
+[ gpu-b ] a:b,c:* [ Failure ]
+`,
+ results: result.List{
+ result.Result{
+ Query: Q("a:b,c:d:e"),
+ Tags: result.NewTags("os-a", "os-c", "gpu-b"),
+ Status: result.Failure,
+ },
+ },
+ updated: `
+# MUTABLE
+a:b,c:* [ Failure ]
+`,
+ diagnostics: expectations.Diagnostics{
+ {
+ Severity: expectations.Note,
+ Line: headerLines + 4,
+ Message: "expectation is fully covered by previous expectations",
+ },
+ },
+ },
+ { //////////////////////////////////////////////////////////////////////
+ name: "simple expectation with tags new flakes implicitly mutable",
+ expectations: `
+################################################################################
+# New flakes. Please triage:
+################################################################################
+[ os-a ] a:b,c:* [ RetryOnFailure ]
+[ gpu-b ] a:b,c:* [ RetryOnFailure ]
+`,
+ results: result.List{
+ result.Result{
+ Query: Q("a:b,c:d:e"),
+ Tags: result.NewTags("os-a", "os-c", "gpu-b"),
+ Status: result.RetryOnFailure,
+ },
+ },
+ updated: `
+################################################################################
+# New flakes. Please triage:
+################################################################################
+crbug.com/dawn/0000 a:* [ RetryOnFailure ]
+`,
+ },
+ { //////////////////////////////////////////////////////////////////////
+ name: "simple expectation with tags new failures implicitly mutable",
+ expectations: `
+################################################################################
+# New failures. Please triage:
+################################################################################
+[ os-a ] a:b,c:* [ Failure ]
+[ gpu-b ] a:b,c:* [ Failure ]
+`,
+ results: result.List{
+ result.Result{
+ Query: Q("a:b,c:d:e"),
+ Tags: result.NewTags("os-a", "os-c", "gpu-b"),
+ Status: result.Failure,
+ },
+ },
+ updated: `
+################################################################################
+# New failures. Please triage:
+################################################################################
+crbug.com/dawn/0000 a:* [ Failure ]
+`,
+ },
+ { //////////////////////////////////////////////////////////////////////
+ name: "simple expectation with tags immutable",
+ expectations: `
[ os-a ] a:b,c:* [ Failure ]
[ gpu-b ] a:b,c:* [ Failure ]
`,
@@ -218,19 +288,14 @@
},
},
updated: `
-a:b,c:* [ Failure ]
+[ gpu-b ] a:b,c:* [ Failure ]
+[ os-a ] a:b,c:* [ Failure ]
`,
- diagnostics: expectations.Diagnostics{
- {
- Severity: expectations.Note,
- Line: headerLines + 3,
- Message: "expectation is fully covered by previous expectations",
- },
- },
},
{ //////////////////////////////////////////////////////////////////////
name: "expectation test now passes",
expectations: `
+# MUTABLE
crbug.com/a/123 [ gpu-a os-a ] a:b,c:* [ Failure ]
crbug.com/a/123 [ gpu-b os-b ] a:b,c:* [ Failure ]
`,
@@ -247,12 +312,13 @@
},
},
updated: `
+# MUTABLE
crbug.com/a/123 [ os-b ] a:b,c:* [ Failure ]
`,
diagnostics: expectations.Diagnostics{
{
Severity: expectations.Note,
- Line: headerLines + 3,
+ Line: headerLines + 4,
Message: "expectation is fully covered by previous expectations",
},
},
@@ -260,6 +326,7 @@
{ //////////////////////////////////////////////////////////////////////
name: "expectation case now passes",
expectations: `
+# MUTABLE
crbug.com/a/123 [ gpu-a os-a ] a:b,c:d:* [ Failure ]
crbug.com/a/123 [ gpu-b os-b ] a:b,c:d:* [ Failure ]
`,
@@ -276,12 +343,13 @@
},
},
updated: `
+# MUTABLE
crbug.com/a/123 [ os-b ] a:b,c:d:* [ Failure ]
`,
diagnostics: expectations.Diagnostics{
{
Severity: expectations.Note,
- Line: headerLines + 3,
+ Line: headerLines + 4,
Message: "expectation is fully covered by previous expectations",
},
},
@@ -330,9 +398,8 @@
`,
},
{ //////////////////////////////////////////////////////////////////////
- name: "expectation case now passes KEEP - single",
+ name: "expectation case now passes immutable - single",
expectations: `
-# KEEP
crbug.com/a/123 [ gpu-a os-a ] a:b,c:d:* [ Failure ]
crbug.com/a/123 [ gpu-b os-b ] a:b,c:d:* [ Failure ]
`,
@@ -349,22 +416,20 @@
},
},
updated: `
-# KEEP
crbug.com/a/123 [ gpu-a os-a ] a:b,c:d:* [ Failure ]
crbug.com/a/123 [ gpu-b os-b ] a:b,c:d:* [ Failure ]
`,
diagnostics: expectations.Diagnostics{
{
Severity: expectations.Note,
- Line: headerLines + 3,
+ Line: headerLines + 2,
Message: "test now passes",
},
},
},
{ //////////////////////////////////////////////////////////////////////
- name: "expectation case now passes KEEP - multiple",
+ name: "expectation case now passes immutable - multiple",
expectations: `
-# KEEP
crbug.com/a/123 a:b,c:d:* [ Failure ]
`,
results: result.List{
@@ -374,13 +439,12 @@
result.Result{Query: Q("a:b,c:d:d"), Status: result.Pass},
},
updated: `
-# KEEP
crbug.com/a/123 a:b,c:d:* [ Failure ]
`,
diagnostics: expectations.Diagnostics{
{
Severity: expectations.Note,
- Line: headerLines + 3,
+ Line: headerLines + 2,
Message: "all 4 tests now pass",
},
},
