internal/cuetest: add support for issue-based conditions/checkers

Often tests (whether pure Go tests or testscript scripts) need to be
skipped until such a time as a linked issue is fixed.

This change adds support for a testscript condition and a Go testing
helper function that take a Go/CUE issue link as an argument. In the
case of a testscript condition, it is satisfied unless the
CUE_NON_ISSUES environment variable contains a regular expression that
matches the argument, indicating such issues are no longer issues. This
condition is generally used in conjunction with the skip command. The Go
testing helping function skips a test under the same condition.

The CUE_NON_ISSUES environment variable can therefore be set to
different patterns in order to vary which tests are run (i.e. not
skipped). CUE_NON_ISSUES=. will cause all tests guarded by this check to
be run (assuming the testscript condition is used in conjunction with
skip). Et cetera.

Having structured skips of this sort makes it easier in future to
analyse and verify that issues remain issues, etc.

Change-Id: I6545c87b1c60d3fdf13abb301d90eca6d615153d
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8761
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>

internal/cuetest/cuetest.go

@@ -19,22 +19,105 @@ package cuetest
 import (
 	"fmt"
 	"os"
+	"regexp"
+	"testing"
+)
+
+const (
+	// envUpdate is used in the definition of UpdateGoldenFiles
+	envUpdate = "CUE_UPDATE"
+
+	// envNonIssues can be set to a regular expression which indicates what
+	// issues we no longer consider issues, i.e. they should have been fixed.
+	// This should generally result in tests that would otherwise be skipped no
+	// longer being skipped.  e.g.  CUE_NON_ISSUES=. will cause all issue
+	// tracker conditions (e.g. [golang.org/issues/1234]) to be considered
+	// non-issues.
+	envNonIssues = "CUE_NON_ISSUES"
+)
+
+var (
+	// issuesConditions is a set of regular expressions that defines the set of
+	// conditions that can be used to declare links to issues in various issue
+	// trackers. e.g. in testscript condition form
+	//
+	//     [golang.org/issues/1234]
+	//     [github.com/govim/govim/issues/4321]
+	issuesConditions = []*regexp.Regexp{
+		regexp.MustCompile(`^golang\.org/issues?/\d+$`),
+		regexp.MustCompile(`^cuelang\.org/issues?/\d+$`),
+	}
 )
 
 // UpdateGoldenFiles determines whether testscript scripts should update txtar
 // archives in the event of cmp failures. It corresponds to
 // testscript.Params.UpdateGoldenFiles. See the docs for
 // testscript.Params.UpdateGoldenFiles for more details.
-var UpdateGoldenFiles = os.Getenv("CUE_UPDATE") != ""
+var UpdateGoldenFiles = os.Getenv(envUpdate) != ""
 
 // Condition adds support for CUE-specific testscript conditions within
-// testscript scripts. The canonical case being [long] which evalutates to true
-// when the long build tag is specified, as is used to indicate that long tests
-// should be run.
+// testscript scripts. Supported conditions include:
+//
+// [long] - evalutates to true when the long build tag is specified
+//
+// [golang.org/issue/N] - evaluates to true unless CUE_NON_ISSUES
+// is set to a regexp that matches the condition, i.e. golang.org/issue/N
+// in this case
+//
+// [cuelang.org/issue/N] - evaluates to true unless CUE_NON_ISSUES
+// is set to a regexp that matches the condition, i.e. cuelang.org/issue/N
+// in this case
+//
 func Condition(cond string) (bool, error) {
+	isIssue, nonIssue, err := checkIssueCondition(cond)
+	if err != nil {
+		return false, err
+	}
+	if isIssue {
+		return !nonIssue, nil
+	}
 	switch cond {
 	case "long":
 		return Long, nil
 	}
 	return false, fmt.Errorf("unknown condition %v", cond)
 }
+
+// IssueSkip causes the test t to be skipped unless the issue identified
+// by s is deemed to be a non-issue by CUE_NON_ISSUES.
+func IssueSkip(t *testing.T, s string) {
+	isIssue, nonIssue, err := checkIssueCondition(s)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !isIssue {
+		t.Fatalf("issue %q does not match a known issue pattern", s)
+	}
+	if nonIssue {
+		t.Skipf("issue %s", s)
+	}
+}
+
+// checkIssueCondition examines s to determine whether it is an issue
+// condition, in which case isIssue is true. If isIssue, then we check
+// CUE_NON_ISSUES for a match, in which case nonIssue is true (a value of true
+// indicates roughly that we don't believe issue s is an issue any more). In
+// case of any errors err is set.
+func checkIssueCondition(s string) (isIssue bool, nonIssue bool, err error) {
+	var r *regexp.Regexp
+	if v := os.Getenv(envNonIssues); v != "" {
+		r, err = regexp.Compile(v)
+		if err != nil {
+			return false, false, fmt.Errorf("failed to compile regexp %q specified via %v: %v", v, envNonIssues, err)
+		}
+	}
+	for _, c := range issuesConditions {
+		if c.MatchString(s) {
+			isIssue = true
+		}
+	}
+	if !isIssue {
+		return false, false, nil
+	}
+	return isIssue, r != nil && r.MatchString(s), nil
+}

internal/cuetest/cuetest_test.go

@@ -0,0 +1,108 @@
+package cuetest
+
+import (
+	"os"
+	"testing"
+)
+
+func TestCondition(t *testing.T) {
+	cases := []struct {
+		name string
+		env  string
+		con  string
+		want bool
+		err  string
+	}{
+		// issue cases covered by TestCheckIssueCondition
+		{
+			name: "long",
+			con:  "long",
+			want: Long, // not really testing much here
+		},
+		{
+			name: "bad condition",
+			env:  ".",
+			con:  "golang.org/Issue/1234", // note typo Issue
+			want: false,
+			err:  "unknown condition golang.org/Issue/1234",
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			os.Setenv(envNonIssues, c.env)
+			got, err := Condition(c.con)
+			if got != c.want {
+				t.Errorf("expected %v; got %v", c.want, got)
+			}
+			if c.err != "" {
+				if err == nil {
+					t.Errorf("expected error %q; got nil", c.err)
+				} else if e := err.Error(); e != c.err {
+					t.Errorf("expected error %q; got %q", c.err, e)
+				}
+			} else if err != nil {
+				t.Errorf("expected no error; got %v", err)
+			}
+		})
+	}
+}
+
+func TestCheckIssueCondition(t *testing.T) {
+	cases := []struct {
+		name     string
+		env      string
+		con      string
+		isIssue  bool
+		nonIssue bool
+		err      string
+	}{
+		{
+			name:     "empty env",
+			con:      "golang.org/issue/1234",
+			isIssue:  true,
+			nonIssue: false,
+		},
+		{
+			name:     "match all issues",
+			env:      ".",
+			con:      "golang.org/issue/1234",
+			isIssue:  true,
+			nonIssue: true,
+		},
+		{
+			name:    "bad issue URL",
+			con:     "golang.org/Issue/1234", // note typo
+			isIssue: false,
+		},
+		{
+			name:    "bad env",
+			env:     `\`,
+			con:     "golang.org/issue/1234",
+			isIssue: false,
+			err:     "failed to compile regexp \"\\\\\" specified via CUE_NON_ISSUES: error parsing regexp: trailing backslash at end of expression: ``",
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			os.Setenv(envNonIssues, c.env)
+			isIssue, nonIssue, err := checkIssueCondition(c.con)
+			if isIssue != c.isIssue {
+				t.Errorf("expected isIssue %v; got %v", c.isIssue, isIssue)
+			}
+			if nonIssue != c.nonIssue {
+				t.Errorf("expected nonIssue %v; got %v", c.nonIssue, nonIssue)
+			}
+			if c.err != "" {
+				if err == nil {
+					t.Errorf("expected error %q; got nil", c.err)
+				} else if e := err.Error(); e != c.err {
+					t.Errorf("expected error %q; got %q", c.err, e)
+				}
+			} else if err != nil {
+				t.Errorf("expected no error; got %q", err)
+			}
+		})
+	}
+
+}