test(flagd): run the conformance suites from make tck, not an env var

`make tck` now runs every test matching "Conformance" and `make e2e` skips
exactly those, so these two suites have the step of their own that Appendix F
asks for and the TCK_RUN gate has nothing left to do. It is removed rather than
kept alongside, because two mechanisms for one exclusion leave the next person
to touch the pipeline guessing which is load-bearing.

Nothing is lost on the point the gate was defending. The split is a test-name
filter and not a build tag, so both targets still compile this file under
-tags=e2e and a signature change in tools/tck still cannot rot it unnoticed --
which was the whole reason the gate was a runtime skip in the first place.

testing.Short() stays. It is no longer the exclusion; it is the one guard left
for a developer who names this package directly, and the README now says why
neither it nor an environment variable is sufficient alone: -short defaults the
wrong way round and a variable is invisible from the build.

What a name filter costs is that a rename can move a suite from one target to
the other in silence. conformance_naming_test.go pays that off: it parses this
package and fails unless the tests that reach tck.Run are exactly the tests the
pattern selects, in both directions, and fails too if it finds no suite at all
so that it cannot pass vacuously. It carries no build tag, so `make test` runs
it with neither Docker nor -tags=e2e -- a guard that only ran in the build it
protects would be worth nothing.

Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>

Author: aepfli

providers/flagd/e2e/README.md

@@ -182,30 +182,57 @@ source has populated the store. The OFREP suite documents the race in full.
 effect when it returns, and a suite that sleeps instead of holding it to that promise stops being
 able to detect when it breaks. This belongs in the testbed.
 
-**These two suites are excluded from the default build.** They skip unless `TCK_RUN` is set, and a
-maintainer runs them by hand before merge:
+**These two suites have a step of their own, and it is the one command to run them:**
 
 ```bash
-TCK_RUN=1 go test -tags=e2e -run TestFlagdRPCConformance -timeout=10m ./...
-TCK_RUN=1 go test -tags=e2e -run Conformance -timeout=20m ./...
+make tck                                  # from the repository root
+go test -tags=e2e -run Conformance -timeout=20m ./...    # equivalently, from here
 ```
 
-Why an adoption suite is excluded rather than gating a merge is the same argument in every language,
-and it is settled in Appendix F's
+`make tck` is `go test -count=1 -timeout=20m -tags=e2e -run 'Conformance'` over every module in the
+workspace. `make e2e` is the same sweep with `-skip 'Conformance'` in place of `-run`, so these two
+suites do not run there and no pull request starts a Docker stack for them.
+
+Why an adoption suite is excluded rather than gating a merge, and why it gets a step of its own
+rather than a slice of an existing e2e suite, is the same argument in every language and is settled
+in Appendix F's
 ["Running the suite in CI"](https://github.com/open-feature/spec/blob/main/specification/appendix-f-provider-conformance.md#running-the-suite-in-ci)
-rather than restated here. The mechanism is Go's, and it is worth stating exactly because the
-appendix names two mistakes and this file had made the first of them:
-
-- The gate is a **runtime skip inside the test function**, reading `TCK_RUN`. Nothing in the build
-  re-enables it. A build tag would not have worked here — `make e2e` expands to a `go test
-  -tags=e2e` over every module in the workspace, so the tag is applied to everything and both these
-  suites were in fact running, red, on every pull request before the gate existed.
-- Because it is a runtime skip rather than `//go:build e2e && tck`, CI still compiles this file
-  against `tools/tck` under `-tags=e2e`, which is what the appendix asks for: a suite that has
-  quietly stopped building against its own harness is worse than one that runs and fails. Only the
-  container work is skipped, and the skip message names the variable.
-- It is written down here and in the harness's own README, which is the appendix's second mistake
-  avoided.
+rather than restated here. Its shape, because it is what decides the mechanism below: a red
+`make tck` says *conformance* failed, where the same scenarios inside `make e2e` would only say *a
+test* failed — and these two suites do carry failures by design, four of them today, each covered by
+a declared deviation above. That is correct output from a conformance run and it would be a
+regression in an e2e run.
+
+The mechanism is Go's, and it is worth stating exactly, because the two obvious choices are both
+wrong here and this file had made one of them:
+
+- **Not a build tag.** `//go:build e2e && tck` would take this file out of the build, and the
+  appendix asks for the opposite: the suite must keep *compiling* in the default build even when it
+  does not run, so a signature change in `tools/tck` cannot rot it unnoticed. A test-name filter
+  excludes the **run** and keeps the **build** — `make e2e` and `make tck` both compile this file
+  under `-tags=e2e`. A tag would also not have excluded anything on its own: `make e2e` applies
+  `-tags=e2e` to every module in the workspace, so a tag is not an exclusion in this repository, it
+  is the opposite, and both these suites were in fact running, red, on every pull request before any
+  gate existed.
+- **Not an environment variable.** These suites used to skip unless `TCK_RUN` was set. That is gone:
+  the target does the same job without hiding the exclusion inside a test function, and it is the
+  step the appendix asks for, which an environment variable is not. Keeping both would have left two
+  mechanisms for one exclusion and the next person to touch the pipeline guessing which is
+  load-bearing.
+- **`testing.Short()` is still checked**, and it is not the exclusion. Neither it nor an environment
+  variable is sufficient alone, and the reason is the default each one picks: `-short` defaults the
+  wrong way, since without the flag the suite *runs* and every pipeline would have to remember to opt
+  out, while a variable defaults to off but is invisible from the build — an exclusion nobody can see
+  is the appendix's second mistake. What the short-mode skip is now is the one guard left for a
+  developer who runs `go test -tags=e2e ./providers/flagd/e2e/` by hand, which is a deliberate gap:
+  naming this package is asking for it.
+- **The filter is a naming contract and `conformance_naming_test.go` holds it.** A `-run`/`-skip`
+  split matches names, so a suite renamed without `Conformance` in it would start running under
+  `make e2e` and stop running under `make tck`, silently. That test parses this package and fails
+  unless the tests that reach `tck.Run` are exactly the tests the pattern selects, in both
+  directions. It carries no build tag, so `make test` runs it with neither Docker nor `-tags=e2e`.
+- It is written down here, in the harness's own README and in `CONTRIBUTING.md` next to the two
+  targets it now sits beside, which is the appendix's second mistake avoided.
 
 ## Test Framework Components
 

providers/flagd/e2e/conformance_naming_test.go

@@ -0,0 +1,208 @@
+package e2e
+
+// This file has no build tag on purpose. It guards the repository's build
+// configuration rather than the provider, it needs neither Docker nor
+// -tags=e2e, and it is worth nothing if it only runs in the build it is
+// protecting. `make test` runs it.
+
+import (
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"os"
+	"path"
+	"path/filepath"
+	"sort"
+	"strconv"
+	"strings"
+	"testing"
+)
+
+// tckFilter is the test-name pattern the Makefile splits on. `make tck` is
+// `go test -run 'Conformance'` and `make e2e` is the same sweep with `-skip`
+// in its place, so the two targets are the two halves of one filter and every
+// test in this package runs in exactly one of them.
+//
+// That split is a test-name filter rather than a build tag so that both targets
+// keep compiling this package against tools/tck -- a conformance suite that has
+// quietly stopped building against its own harness is a worse failure than one
+// that runs and fails. The cost of choosing a name filter is that a rename can
+// move a suite from one target to the other in silence: `make e2e` would start
+// pulling Docker images on every pull request and `make tck` would stop running
+// the suite, and neither would say anything. This test is that cost paid off.
+const tckFilter = "Conformance"
+
+// tckImportPath is the harness. A test that reaches tck.Run, directly or
+// through a helper in this package, is a conformance suite; nothing else in
+// this package is one.
+const tckImportPath = "github.com/open-feature/go-sdk-contrib/tools/tck"
+
+// TestEveryTckSuiteIsSelectedByTheMakeTargetFilter fails unless the tests that
+// run the conformance suite are exactly the tests the Makefile's filter picks
+// out -- in both directions. A suite whose name loses the pattern stops being
+// run by `make tck` and starts being run by `make e2e`; a test that gains the
+// pattern without running the suite ends up in the conformance step, where a
+// red result would be reported as a conformance failure it is not.
+//
+// Note that this test must not itself be named for the pattern, and is not.
+func TestEveryTckSuiteIsSelectedByTheMakeTargetFilter(t *testing.T) {
+	tests, runsSuite, err := scanPackage(".")
+	if err != nil {
+		t.Fatalf("reading this package's own sources: %v", err)
+	}
+
+	var suites, selected []string
+	for _, name := range tests {
+		if runsSuite[name] {
+			suites = append(suites, name)
+		}
+		if strings.Contains(name, tckFilter) {
+			selected = append(selected, name)
+		}
+	}
+	sort.Strings(suites)
+	sort.Strings(selected)
+
+	// Without this the whole test passes vacuously the day the scan stops
+	// recognising a call -- an import alias, a suite moved behind another
+	// helper -- which is the one way a guard like this fails silently.
+	if len(suites) == 0 {
+		t.Fatalf("found no test calling tck.Run in this package; either the conformance suites are "+
+			"gone or %s can no longer recognise them, and in both cases `make tck` is running nothing",
+			"conformance_naming_test.go")
+	}
+
+	for _, name := range suites {
+		if !strings.Contains(name, tckFilter) {
+			t.Errorf("%s runs the conformance suite but its name does not contain %q, so `make tck` "+
+				"will not run it and `make e2e` will: rename it", name, tckFilter)
+		}
+	}
+	for _, name := range selected {
+		if !runsSuite[name] {
+			t.Errorf("%s does not run the conformance suite but its name contains %q, so `make tck` "+
+				"will run it and report any failure as a conformance failure: rename it", name, tckFilter)
+		}
+	}
+
+	t.Logf("conformance suites in this package: %s", strings.Join(suites, ", "))
+}
+
+// scanPackage parses every _test.go file in dir and returns the names of its
+// test functions, plus the set of package-level functions that reach tck.Run.
+// Build tags are irrelevant to it, which is the point: it reads the e2e-tagged
+// suites from an untagged test.
+func scanPackage(dir string) (tests []string, runsSuite map[string]bool, err error) {
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	fset := token.NewFileSet()
+	calls := map[string]map[string]bool{}
+	runsSuite = map[string]bool{}
+
+	for _, entry := range entries {
+		if entry.IsDir() || !strings.HasSuffix(entry.Name(), "_test.go") {
+			continue
+		}
+		file, parseErr := parser.ParseFile(fset, filepath.Join(dir, entry.Name()), nil, 0)
+		if parseErr != nil {
+			return nil, nil, parseErr
+		}
+		local := localNameOf(file, tckImportPath)
+
+		for _, decl := range file.Decls {
+			fn, ok := decl.(*ast.FuncDecl)
+			if !ok || fn.Recv != nil || fn.Body == nil {
+				continue
+			}
+			name := fn.Name.Name
+			if isTestFunc(fn) {
+				tests = append(tests, name)
+			}
+			ast.Inspect(fn.Body, func(node ast.Node) bool {
+				call, ok := node.(*ast.CallExpr)
+				if !ok {
+					return true
+				}
+				switch fun := call.Fun.(type) {
+				case *ast.SelectorExpr:
+					pkg, ok := fun.X.(*ast.Ident)
+					if ok && local != "" && pkg.Name == local && fun.Sel.Name == "Run" {
+						runsSuite[name] = true
+					}
+				case *ast.Ident:
+					if calls[name] == nil {
+						calls[name] = map[string]bool{}
+					}
+					calls[name][fun.Name] = true
+				}
+				return true
+			})
+		}
+	}
+
+	// A suite is usually reached through a helper -- runConformance here -- so
+	// follow same-package calls to a fixed point rather than only looking for
+	// tck.Run in the test function itself.
+	for changed := true; changed; {
+		changed = false
+		for caller, callees := range calls {
+			if runsSuite[caller] {
+				continue
+			}
+			for callee := range callees {
+				if runsSuite[callee] {
+					runsSuite[caller] = true
+					changed = true
+					break
+				}
+			}
+		}
+	}
+
+	sort.Strings(tests)
+	return tests, runsSuite, nil
+}
+
+// localNameOf returns the name importPath is bound to in file, honouring an
+// alias, or "" if the file does not import it.
+func localNameOf(file *ast.File, importPath string) string {
+	for _, spec := range file.Imports {
+		unquoted, err := strconv.Unquote(spec.Path.Value)
+		if err != nil || unquoted != importPath {
+			continue
+		}
+		if spec.Name != nil {
+			return spec.Name.Name
+		}
+		return path.Base(unquoted)
+	}
+	return ""
+}
+
+// isTestFunc applies the testing package's own rule for what `go test` will
+// run, so that this test and the test binary agree on the set of names.
+func isTestFunc(fn *ast.FuncDecl) bool {
+	name := fn.Name.Name
+	if !strings.HasPrefix(name, "Test") {
+		return false
+	}
+	if rest := name[len("Test"):]; rest != "" && rest[0] >= 'a' && rest[0] <= 'z' {
+		return false
+	}
+	if fn.Type.Params == nil || len(fn.Type.Params.List) != 1 {
+		return false
+	}
+	star, ok := fn.Type.Params.List[0].Type.(*ast.StarExpr)
+	if !ok {
+		return false
+	}
+	sel, ok := star.X.(*ast.SelectorExpr)
+	if !ok {
+		return false
+	}
+	pkg, ok := sel.X.(*ast.Ident)
+	return ok && pkg.Name == "testing" && sel.Sel.Name == "T"
+}

providers/flagd/e2e/tck_test.go

@@ -4,7 +4,6 @@ package e2e
 
 import (
 	"context"
-	"os"
 	"testing"
 	"time"
 
@@ -494,30 +493,20 @@ type conformanceSuite struct {
 	gracePeriod     int
 }
 
-// runEnv gates the conformance suites out of a default build.
+// runConformance is where both suites below come, and the name of the test that
+// calls it is load-bearing: `make tck` runs every test matching "Conformance"
+// and `make e2e` skips exactly those, so this is what keeps a Docker stack out
+// of every pull request while leaving the file compiled — and typechecked
+// against tools/tck — in both. There is no environment variable and no second
+// build tag; conformance_naming_test.go is what stops a rename from quietly
+// swapping which of the two targets these run in.
 //
-// `make e2e` runs every module's e2e-tagged tests, so without this the two
-// suites below would start a Docker stack each on every pull request. They are
-// also expected to be red while the fixture gap described in the README stands,
-// and a conformance report that records a deviation plus a CI job that fails on
-// it are two answers to the same question. The policy is therefore exclusion,
-// with a maintainer running these by hand before merge, and this is where it is
-// enforced rather than merely described.
-//
-// A runtime skip rather than a second build tag on purpose: the adoption stays
-// compiled under -tags=e2e, so CI still typechecks it against tools/tck and a
-// signature change there cannot rot this file unnoticed. Only the container
-// work is skipped, and the skip names the variable that turns it on.
-const runEnv = "TCK_RUN"
-
+// The short-mode skip below is not that exclusion. It is the one guard left for
+// someone who names this package directly, and it stays for that.
 func runConformance(t *testing.T, suite conformanceSuite) {
 	if testing.Short() {
 		t.Skip("skipping e2e tests in short mode")
 	}
-	if os.Getenv(runEnv) == "" {
-		t.Skipf("the provider conformance suite is excluded from the default build: set %s=1 to "+
-			"run it (it needs Docker and takes minutes). See README.md", runEnv)
-	}
 
 	tck.Run(t,
 		tck.WithName(suite.name),