From 67be4f5d06b5b176bc470b1506953ceafb8c3869 Mon Sep 17 00:00:00 2001
From: Aaron Crickenberger <spiffxp@google.com>
Date: Tue, 4 Jun 2019 11:03:23 -0700
Subject: [PATCH] Error on tests ineligible for promotion to conformance

If there are tags in the test name that describe qualities of the
test that make it ineligible for conformance, raise an error. This
is basically the "skip list" that heptio's e2e image used to use.

Thankfully all of our existing Conformance tests lack these tags. I
considered added [Slow] to the list, but let's save that for another
day.
---
 test/conformance/walk.go      | 17 +++++++++++
 test/conformance/walk_test.go | 55 +++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

diff --git a/test/conformance/walk.go b/test/conformance/walk.go
index 7492712ace1..a32e6df9a7b 100644
--- a/test/conformance/walk.go
+++ b/test/conformance/walk.go
@@ -41,6 +41,9 @@ var (
 	baseURL                                           = flag.String("url", "https://github.com/kubernetes/kubernetes/tree/master/", "location of the current source")
 	confDoc                                           = flag.Bool("conformance", false, "write a conformance document")
 	totalConfTests, totalLegacyTests, missingComments int
+
+	// If a test name contains any of these tags, it is ineligble for promotion to conformance
+	regexIneligibleTags = regexp.MustCompile(`\[(Alpha|Disruptive|Feature:[^\]]+|Flaky)\]`)
 )
 
 const regexDescribe = "Describe|KubeDescribe|SIGDescribe"
@@ -199,6 +202,12 @@ func (v *visitor) emit(arg ast.Expr) {
 			return
 		}
 
+		err := validateTestName(v.getDescription(at.Value))
+		if err != nil {
+			v.failf(at, err.Error())
+			return
+		}
+
 		at.Value = normalizeTestName(at.Value)
 		if *confDoc {
 			v.convertToConformanceData(at)
@@ -233,6 +242,14 @@ func normalizeTestName(s string) string {
 	return strings.TrimSpace(r)
 }
 
+func validateTestName(s string) error {
+	matches := regexIneligibleTags.FindAllString(s, -1)
+	if matches != nil {
+		return fmt.Errorf("'%s' cannot have invalid tags %v", s, strings.Join(matches, ","))
+	}
+	return nil
+}
+
 // funcName converts a selectorExpr with two idents into a string,
 // x.y -> "x.y"
 func funcName(n ast.Expr) string {
diff --git a/test/conformance/walk_test.go b/test/conformance/walk_test.go
index 1d4579e212a..2d3795d97ba 100644
--- a/test/conformance/walk_test.go
+++ b/test/conformance/walk_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package main
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 )
@@ -127,3 +128,57 @@ func TestNormalizeTestNames(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateTestName(t *testing.T) {
+	testCases := []struct {
+		testName  string
+		tagString string
+	}{
+		{
+			"a test case with no tags",
+			"",
+		},
+		{
+			"a test case with valid tags [LinuxOnly] [NodeConformance] [Serial]",
+			"",
+		},
+		{
+			"a flaky test case that is invalid [Flaky]",
+			"[Flaky]",
+		},
+		{
+			"a disruptive test case that is invalid [Disruptive]",
+			"[Disruptive]",
+		},
+		{
+			"a feature test case that is invalid [Feature:Awesome]",
+			"[Feature:Awesome]",
+		},
+		{
+			"an alpha test case that is invalid [Alpha]",
+			"[Alpha]",
+		},
+		{
+			"a test case with multiple invalid tags [Flaky][Disruptive] [Feature:Awesome] [Alpha]",
+			"[Flaky],[Disruptive],[Feature:Awesome],[Alpha]",
+		},
+		{
+			"[sig-awesome] [Disruptive] a test case with valid and invalid tags [Alpha] [Serial] [Flaky]",
+			"[Disruptive],[Alpha],[Flaky]",
+		},
+	}
+	for i, tc := range testCases {
+		err := validateTestName(tc.testName)
+		if err != nil {
+			if tc.tagString == "" {
+				t.Errorf("test case[%d]: expected no validate error, got %q", i, err.Error())
+			} else {
+				expectedMsg := fmt.Sprintf("'%s' cannot have invalid tags %s", tc.testName, tc.tagString)
+				actualMsg := err.Error()
+				if actualMsg != expectedMsg {
+					t.Errorf("test case[%d]: expected error message %q, got %q", i, expectedMsg, actualMsg)
+				}
+			}
+		}
+	}
+}