UPSTREAM: <carry>: Ensure balanced brackets in annotated test names

We recently started marking tests with apigroups, and in one case we
missed the closing bracket on the annotation resulting in the test being
erroneously skipped.

This adds a check in the annotation generation, and errors when brackets
are unbalanced.

```
Example:
$ ./hack/verify-generated.sh
FAILURE after 12.870s: hack/verify-generated.sh:13: executing '/home/stbenjam/go/src/github.com/openshift/origin/hack/update-generated.sh' expecting success: the command returned the wrong error code
Standard output from the command:
Nov  4 14:11:25.026: INFO: Enabling in-tree volume drivers
Nov  4 14:11:25.026: INFO: Warning: deprecated ENABLE_STORAGE_GCE_PD_DRIVER used. This will be removed in a future release. Use --enabled-volume-drivers=gcepd instead
Nov  4 14:11:25.026: INFO: Enabled gcepd and windows-gcepd in-tree volume drivers

Standard error from the command:
failed: unbalanced brackets in test name:
[Top Level] [sig-scheduling][Early] The openshift-console console pods [apigroup:console.openshift.io should be scheduled on different nodes
                                                                       ^
```

Author: stbenjam

openshift-hack/e2e/annotate/annotate.go

@@ -17,6 +17,8 @@ var reHasSig = regexp.MustCompile(`\[sig-[\w-]+\]`)
 
 // Run generates tests annotations for the targeted package.
 func Run() {
+	var errors []string
+
 	if len(os.Args) != 2 && len(os.Args) != 3 {
 		fmt.Fprintf(os.Stderr, "error: requires exactly one argument\n")
 		os.Exit(1)
@@ -26,6 +28,9 @@ func Run() {
 	generator := newGenerator()
 	ginkgo.GetSuite().BuildTree()
 	ginkgo.GetSuite().WalkTests(generator.generateRename)
+	if len(generator.errors) > 0 {
+		errors = append(errors, generator.errors...)
+	}
 
 	renamer := newRenamerFromGenerated(generator.output)
 	// generated file has a map[string]string in the following format:
@@ -49,7 +54,6 @@ func Run() {
 	// Upstream sigs map to teams (if you have representation on that sig, you
 	//   own those tests in origin)
 	// Downstream sigs: sig-imageregistry, sig-builds, sig-devex
-	var errors []string
 	for from, to := range generator.output {
 		if !reHasSig.MatchString(from) && !reHasSig.MatchString(to) {
 			errors = append(errors, fmt.Sprintf("all tests must define a [sig-XXXX] tag or have a rule %q", from))
@@ -131,8 +135,7 @@ func newGenerator() *ginkgoTestRenamer {
 		stringMatches:       stringMatches,
 		matches:             matches,
 		excludedTestsFilter: excludedTestsFilter,
-
-		output: make(map[string]string),
+		output:              make(map[string]string),
 	}
 }
 
@@ -158,6 +161,8 @@ type ginkgoTestRenamer struct {
 	output map[string]string
 	// map of unmatched test names
 	missing map[string]struct{}
+	// a list of errors to display
+	errors []string
 }
 
 func (r *ginkgoTestRenamer) updateNodeText(name string, node types.TestSpec) {
@@ -226,6 +231,9 @@ func (r *ginkgoTestRenamer) generateRename(name string, node types.TestSpec) {
 		newLabels += " [Suite:k8s]"
 	}
 
+	if err := checkBalancedBrackets(newName); err != nil {
+		r.errors = append(r.errors, err.Error())
+	}
 	r.output[name] = newLabels
 }
 
@@ -239,3 +247,39 @@ func (r *ginkgoTestRenamer) generateRename(name string, node types.TestSpec) {
 func isGoModulePath(packagePath, module, modulePath string) bool {
 	return regexp.MustCompile(fmt.Sprintf(`\b%s(@[^/]*|)/%s\b`, regexp.QuoteMeta(module), regexp.QuoteMeta(modulePath))).MatchString(packagePath)
 }
+
+// checkBalancedBrackets ensures that square brackets are balanced in generated test
+// names. If they are not, it returns an error with the name of the test and a guess
+// where the unmatched bracket(s) are.
+func checkBalancedBrackets(testName string) error {
+	stack := make([]int, 0, len(testName))
+	for idx, c := range testName {
+		if c == '[' {
+			stack = append(stack, idx)
+		} else if c == ']' {
+			// case when we start off with a ]
+			if len(stack) == 0 {
+				stack = append(stack, idx)
+			} else {
+				stack = stack[:len(stack)-1]
+			}
+		}
+	}
+
+	if len(stack) > 0 {
+		msg := testName + "\n"
+	outerLoop:
+		for i := 0; i < len(testName); i++ {
+			for _, loc := range stack {
+				if i == loc {
+					msg += "^"
+					continue outerLoop
+				}
+			}
+			msg += " "
+		}
+		return fmt.Errorf("unbalanced brackets in test name:\n%s\n", msg)
+	}
+
+	return nil
+}

openshift-hack/e2e/annotate/annotate_test.go

@@ -0,0 +1,55 @@
+package annotate
+
+import (
+	"fmt"
+	"os"
+	"testing"
+)
+
+func Test_checkBalancedBrackets(t *testing.T) {
+	tests := []struct {
+		testCase string
+		testName string
+		wantErr  bool
+	}{
+		{
+			testCase: "balanced brackets succeeds",
+			testName: "[sig-storage] Test that storage [apigroup:storage.openshift.io] actually works [Driver:azure][Serial][Late]",
+			wantErr:  false,
+		},
+		{
+			testCase: "unbalanced brackets errors",
+			testName: "[sig-storage] Test that storage [apigroup:storage.openshift.io actually works [Driver:azure][Serial][Late]",
+			wantErr:  true,
+		},
+		{
+			testCase: "start with close bracket errors",
+			testName: "[sig-storage] test with a random bracket ]",
+			wantErr:  true,
+		},
+		{
+			testCase: "multiple unbalanced brackets errors",
+			testName: "[sig-storage Test that storage [apigroup:storage.openshift.io actually works [Driver:azure]",
+			wantErr:  true,
+		},
+		{
+			testCase: "balanced deeply nested brackets succeeds",
+			testName: "[[[[[[some weird test with deeply nested brackets]]]]]]",
+			wantErr:  false,
+		},
+		{
+			testCase: "unbalanced deeply nested brackets errors",
+			testName: "[[[[[[some weird test with deeply nested brackets]]]]]",
+			wantErr:  true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.testCase, func(t *testing.T) {
+			if err := checkBalancedBrackets(tt.testName); (err != nil) != tt.wantErr {
+				t.Errorf("checkBalancedBrackets() error = %v, wantErr %v", err, tt.wantErr)
+			} else if err != nil {
+				fmt.Fprintf(os.Stderr, "checkBalancedBrackets() success, found expected err = \n%s\n", err.Error())
+			}
+		})
+	}
+}