Repository: hashicorp/nomad
File: acl/policy_test.go (Go)

Code before:
		})
	}
}

// TestPolicy_isCapabilityValid reads through the source to get all our
// fine-grained capability constants and makes sure they're all marked as
// valid. This prevents us from adding a new capability without marking it as
// valid
func TestPolicy_isCapabilityValid(t *testing.T) {
	filename := "policy.go"
	src, err := os.ReadFile(filename)
	must.NoError(t, err)

	// namespace
	caps := []string{}
	re := regexp.MustCompile(`(?m)^(?:\tNamespaceCapability[A-Za-z]+\s+= ")(.*)"$`)
	subs := re.FindAllSubmatch(src, -1)
	for _, sub := range subs {
		if len(sub) > 1 {
			caps = append(caps, string(sub[1]))
		}
	}
	must.Greater(t, 0, len(caps)) // prevents us from breaking the regex later
	for _, cap := range caps {
		cap = strings.TrimSpace(cap)
		test.True(t, isNamespaceCapabilityValid(cap))
	}

	// variables
	caps = []string{}
	re = regexp.MustCompile(`(?m)^(?:\tVariablesCapability[A-Za-z]+\s+= ")(.*)"$`)
	subs = re.FindAllSubmatch(src, -1)
	for _, sub := range subs {
		if len(sub) > 1 {
			caps = append(caps, string(sub[1]))
		}
	}
	must.Greater(t, 0, len(caps))
	for _, cap := range caps {
		cap = strings.TrimSpace(cap)
		test.True(t, isPathCapabilityValid(cap))
	}

	// host volumes
	caps = []string{}
	re = regexp.MustCompile(`(?m)^(?:\tHostVolumeCapability[A-Za-z]+\s+= ")(.*)"$`)
	subs = re.FindAllSubmatch(src, -1)
	for _, sub := range subs {
		if len(sub) > 1 {
			caps = append(caps, string(sub[1]))
		}

Review comment (bug):
This catches the inverse of my above concern, but produces a cryptic error if tripped:

> policy_test.go:1129: expected 0 > 0

Ditto `test.True()`:

> expected condition to be true; is false


```suggestion
	must.Greater(t, 0, len(caps), must.Sprint("regex failed to find any NamespaceCapability*"))
	for _, cap := range caps {
		cap = strings.TrimSpace(cap)
		test.True(t, isNamespaceCapabilityValid(cap), must.Sprintf("%q capability not valid"), cap)
```

---

Or, this whole thing could be loopified in subtests with helpful names


```go
prefixes := []string{"NamespaceCapability", "VariablesCapability", ...}

for _, p := range prefixes {
	t.Run(p, func(t *testing.T) {
		find := fmt.Sprintf(`(?m)^(?:\t%s[A-Za-z]+\s+= ")(.*)"$`, p)
		re := regexp.MustCompile(find)
		// ......
		must.Greater(t, 0, len(caps), must.Sprint("no regex matches found")
		for _, cap := range caps {
			cap = strings.TrimSpace(cap)
			test.True(t, isPathCapabilityValid(cap), must.Sprintf("%q capability not valid"), cap)
		}
	})
}
```

Admittedly, that `find := fmt.Sprintf` is even harder to parse with human eyes...

Just food for thought.

Code after:
		})
	}
}

// TestPolicy_isCapabilityValid reads through the source to get all our
// fine-grained capability constants and makes sure they're all marked as
// valid. This prevents us from adding a new capability without marking it as
// valid
func TestPolicy_isCapabilityValid(t *testing.T) {
	filename := "policy.go"
	src, err := os.ReadFile(filename)
	must.NoError(t, err)

	// namespace
	caps := []string{}
	re := regexp.MustCompile(`(?m)^(?:\tNamespaceCapability[A-Za-z0-9]+\s+= ")(.*)"$`)
	subs := re.FindAllSubmatch(src, -1)
	for _, sub := range subs {
		if len(sub) > 1 {
			caps = append(caps, string(sub[1]))
		}
	}
	must.Greater(t, 0, len(caps), must.Sprint("no constants match NamespaceCapability regex"))
	for _, cap := range caps {
		cap = strings.TrimSpace(cap)
		test.True(t, isNamespaceCapabilityValid(cap), test.Sprintf("%q not valid", cap))
	}

	// variables
	caps = []string{}
	re = regexp.MustCompile(`(?m)^(?:\tVariablesCapability[A-Za-z0-9]+\s+= ")(.*)"$`)
	subs = re.FindAllSubmatch(src, -1)
	for _, sub := range subs {
		if len(sub) > 1 {
			caps = append(caps, string(sub[1]))
		}
	}
	must.Greater(t, 0, len(caps), must.Sprint("no constants match VariablesCapability regex"))
	for _, cap := range caps {
		cap = strings.TrimSpace(cap)
		test.True(t, isPathCapabilityValid(cap), test.Sprintf("%q not valid", cap))
	}

	// host volumes
	caps = []string{}
	re = regexp.MustCompile(`(?m)^(?:\tHostVolumeCapability[A-Za-z0-9]+\s+= ")(.*)"$`)
	subs = re.FindAllSubmatch(src, -1)
	for _, sub := range subs {
		if len(sub) > 1 {
			caps = append(caps, string(sub[1]))
		}