ci: run opt-in integration tests for affected plugins

.github/workflows/integration.yaml

@@ -0,0 +1,74 @@
+name: Plugin Integration
+
+on:
+  pull_request:
+    paths:
+      - "plugins/**"
+      - "sdk/**"
+      - "go.mod"
+      - "go.sum"
+      - "cmd/ci/integration-matrix/**"
+      - ".github/workflows/integration.yaml"
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+jobs:
+  select:
+    name: Select integration tests
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    outputs:
+      matrix: ${{ steps.select.outputs.matrix }}
+      has-tests: ${{ steps.select.outputs.has-tests }}
+    steps:
+      - name: Harden the runner
+        uses: step-security/harden-runner@e14015d583714f6e62063499dc959a02595150a1 # v2.21.1
+        with:
+          egress-policy: audit
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          fetch-depth: 2
+          persist-credentials: false
+      - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
+        with:
+          go-version-file: go.mod
+      - name: Select affected plugins
+        id: select
+        env:
+          EVENT_NAME: ${{ github.event_name }}
+        run: |
+          if [ "$EVENT_NAME" = workflow_dispatch ]; then
+            matrix=$(go run ./cmd/ci/integration-matrix --all)
+          else
+            matrix=$(go run ./cmd/ci/integration-matrix --base HEAD^1 --head HEAD)
+          fi
+          echo "matrix=$matrix" >> "$GITHUB_OUTPUT"
+          echo "has-tests=$(jq '.plugin | length > 0' <<< "$matrix")" >> "$GITHUB_OUTPUT"
+  integration:
+    name: Integration (${{ matrix.plugin }})
+    needs: select
+    if: needs.select.outputs.has-tests == 'true'
+    runs-on: ubuntu-latest
+    timeout-minutes: 15
+    strategy:
+      fail-fast: false
+      max-parallel: 2
+      matrix: ${{ fromJSON(needs.select.outputs.matrix) }}
+    steps:
+      - name: Harden the runner
+        uses: step-security/harden-runner@e14015d583714f6e62063499dc959a02595150a1 # v2.21.1
+        with:
+          egress-policy: audit
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          persist-credentials: false
+      - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
+        with:
+          go-version-file: go.mod
+      - name: Run integration tests
+        env:
+          SHELL_PLUGINS_INTEGRATION: "1"
+          PLUGIN: ${{ matrix.plugin }}
+        run: go test -count=1 -timeout=10m "./plugins/$PLUGIN"

CONTRIBUTING.md

@@ -44,6 +44,16 @@ The plugin ecosystem allows you to [locally build](#make-plugin-build) your plug
 You can add tests to your plugin using the SDK's [`plugintest` package](sdk/plugintest/), which provides helpers so that you only have to care about the test cases themselves.
 You can use the [`example-secrets` command](#make-plugin-example-secrets) to help create test fixtures.
 
+### Tests with external dependencies
+
+Keep tests that start containers or require installed CLIs in a plugin's root directory, in files named `*_integration_test.go`. Skip them unless `SHELL_PLUGINS_INTEGRATION=1`; ordinary `make test` must not require those dependencies. Use pinned tool or container versions, temporary test credentials, bounded waits, and cleanup on failure. Tests must not require a real account or repository secrets.
+
+Run one plugin locally with `SHELL_PLUGINS_INTEGRATION=1 go test -count=1 -timeout=10m ./plugins/<name>`.
+
+The optional **Plugin Integration** workflow runs only affected integration-test packages on pull requests. Its selector compares the PR merge commit with its first parent, including deleted/renamed paths and local Go test dependencies. SDK, module or workflow changes run every integration package. A manual workflow run also tests every integration package. Keep this workflow non-required: path filters can leave required checks pending, and external dependency availability should not block unrelated changes. Existing unit, lint and validation checks remain separate.
+
+The CockroachDB test also accepts its existing `COCKROACHDB_INTEGRATION=1` opt-in for compatibility.
+
 <!----><a name="makefile-commands"></a>
 ## 👷 Makefile Commands
 

cmd/ci/integration-matrix/main.go

@@ -0,0 +1,170 @@
+// integration-matrix selects dependency-backed plugin tests for an optional CI job.
+package main
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"flag"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"regexp"
+	"sort"
+	"strings"
+	"time"
+)
+
+var pluginName = regexp.MustCompile(`^[a-z][a-z0-9_]*$`)
+
+func main() {
+	base := flag.String("base", "HEAD^1", "Base commit for the comparison")
+	head := flag.String("head", "HEAD", "Head commit for the comparison")
+	all := flag.Bool("all", false, "Select every plugin with integration tests")
+	flag.Parse()
+	root, err := os.Getwd()
+	if err == nil {
+		err = run(root, *base, *head, *all)
+	}
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
+	}
+}
+
+func run(root, base, head string, all bool) error {
+	plugins, err := discoverPlugins(root)
+	if err != nil {
+		return err
+	}
+	selected := plugins
+	if !all && len(plugins) > 0 {
+		changed, err := changedFiles(root, base, head)
+		if err != nil {
+			return err
+		}
+		if len(changed) == 0 {
+			selected = []string{}
+		} else if !sharedChange(changed) {
+			dependencies := make(map[string][]string)
+			for _, plugin := range plugins {
+				directories, err := dependencyDirectories(root, plugin)
+				if err != nil {
+					return err
+				}
+				dependencies[plugin] = directories
+			}
+			selected = selectPlugins(plugins, dependencies, changed)
+		}
+	}
+	return json.NewEncoder(os.Stdout).Encode(struct {
+		Plugin []string `json:"plugin"`
+	}{selected})
+}
+
+func discoverPlugins(root string) ([]string, error) {
+	files, err := filepath.Glob(filepath.Join(root, "plugins", "*", "*_integration_test.go"))
+	if err != nil {
+		return nil, err
+	}
+	names := make(map[string]bool)
+	for _, file := range files {
+		name := filepath.Base(filepath.Dir(file))
+		if !pluginName.MatchString(name) {
+			return nil, fmt.Errorf("invalid integration plugin directory %q", name)
+		}
+		names[name] = true
+	}
+	result := make([]string, 0, len(names))
+	for name := range names {
+		result = append(result, name)
+	}
+	sort.Strings(result)
+	return result, nil
+}
+
+func commandOutput(root, name string, args ...string) ([]byte, error) {
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
+	defer cancel()
+	command := exec.CommandContext(ctx, name, args...)
+	command.Dir = root
+	var stderr bytes.Buffer
+	command.Stderr = &stderr
+	output, err := command.Output()
+	if err != nil {
+		return nil, fmt.Errorf("%s failed: %w: %s", name, err, stderr.String())
+	}
+	return output, nil
+}
+
+func changedFiles(root, base, head string) ([]string, error) {
+	revisions := make([]string, 0, 2)
+	for _, ref := range []string{base, head} {
+		output, err := commandOutput(root, "git", "rev-parse", "--verify", "--end-of-options", ref+"^{commit}")
+		if err != nil {
+			return nil, err
+		}
+		revisions = append(revisions, strings.TrimSpace(string(output)))
+	}
+	output, err := commandOutput(root, "git", "diff", "--name-only", "-z", "--no-renames", revisions[0], revisions[1], "--")
+	if err != nil {
+		return nil, err
+	}
+	if len(output) == 0 {
+		return nil, nil
+	}
+	return strings.Split(strings.TrimSuffix(string(output), "\x00"), "\x00"), nil
+}
+
+func sharedChange(paths []string) bool {
+	for _, path := range paths {
+		if path == "go.mod" || path == "go.sum" || path == ".github/workflows/integration.yaml" ||
+			strings.HasPrefix(path, "sdk/") || strings.HasPrefix(path, "cmd/ci/integration-matrix/") {
+			return true
+		}
+	}
+	return false
+}
+
+func dependencyDirectories(root, plugin string) ([]string, error) {
+	output, err := commandOutput(root, "go", "list", "-deps", "-test", "-f", "{{.Dir}}", "./plugins/"+plugin)
+	if err != nil {
+		return nil, err
+	}
+	directories := make(map[string]bool)
+	for _, dir := range strings.Split(strings.TrimSpace(string(output)), "\n") {
+		if dir == "" {
+			continue
+		}
+		relative, err := filepath.Rel(root, dir)
+		if err != nil || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) {
+			continue
+		}
+		directories[filepath.ToSlash(relative)] = true
+	}
+	result := make([]string, 0, len(directories))
+	for dir := range directories {
+		result = append(result, dir)
+	}
+	sort.Strings(result)
+	return result, nil
+}
+
+func selectPlugins(plugins []string, dependencies map[string][]string, changed []string) []string {
+	selected := make([]string, 0)
+	for _, plugin := range plugins {
+		affected := false
+		for _, directory := range dependencies[plugin] {
+			for _, path := range changed {
+				if directory == "." || strings.HasPrefix(path, directory+"/") {
+					affected = true
+				}
+			}
+		}
+		if affected {
+			selected = append(selected, plugin)
+		}
+	}
+	return selected
+}

cmd/ci/integration-matrix/main_test.go

@@ -0,0 +1,136 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"reflect"
+	"testing"
+)
+
+func writeFixture(t *testing.T, root, path, content string) {
+	t.Helper()
+	name := filepath.Join(root, path)
+	if err := os.MkdirAll(filepath.Dir(name), 0700); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(name, []byte(content), 0600); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestSelection(t *testing.T) {
+	plugins := []string{"alpha", "beta"}
+	dependencies := map[string][]string{
+		"alpha": {"plugins/alpha", "plugins/shared", "sdk"},
+		"beta":  {"plugins/beta", "sdk"},
+	}
+	cases := []struct {
+		name          string
+		changed, want []string
+	}{
+		{"direct change", []string{"plugins/alpha/plugin.go"}, []string{"alpha"}},
+		{"fixture change", []string{"plugins/alpha/test-fixtures/with space\nand newline.json"}, []string{"alpha"}},
+		{"imported plugin change", []string{"plugins/shared/provisioner.go"}, []string{"alpha"}},
+		{"prefix lookalike", []string{"plugins/alpha-other/plugin.go"}, []string{}},
+		{"unrelated plugin", []string{"plugins/gamma/plugin.go"}, []string{}},
+		{"shared dependency", []string{"sdk/provisioner.go"}, []string{"alpha", "beta"}},
+		{"rename affects both", []string{"plugins/alpha/old.go", "plugins/beta/new.go"}, []string{"alpha", "beta"}},
+		{"empty change", nil, []string{}},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if got := selectPlugins(plugins, dependencies, c.changed); !reflect.DeepEqual(got, c.want) {
+				t.Fatalf("got %v; want %v", got, c.want)
+			}
+		})
+	}
+	for _, path := range []string{"go.mod", "go.sum", "sdk/importer/file.go", "cmd/ci/integration-matrix/main.go", ".github/workflows/integration.yaml"} {
+		if !sharedChange([]string{path}) {
+			t.Errorf("shared change was not recognized: %s", path)
+		}
+	}
+	if sharedChange([]string{"README.md", "sdk-other/file.go"}) {
+		t.Fatal("unrelated paths were marked shared")
+	}
+}
+
+func TestDiscovery(t *testing.T) {
+	root := t.TempDir()
+	for _, path := range []string{"plugins/beta/one_integration_test.go", "plugins/alpha/one_integration_test.go", "plugins/alpha/two_integration_test.go", "plugins/other/unit_test.go"} {
+		writeFixture(t, root, path, "package example\n")
+	}
+	got, err := discoverPlugins(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !reflect.DeepEqual(got, []string{"alpha", "beta"}) {
+		t.Fatal(got)
+	}
+	writeFixture(t, root, "plugins/invalid;name/test_integration_test.go", "package example\n")
+	if _, err := discoverPlugins(root); err == nil {
+		t.Fatal("invalid directory accepted")
+	}
+	empty, err := discoverPlugins(t.TempDir())
+	if err != nil || empty == nil || len(empty) != 0 {
+		t.Fatalf("empty result must encode as [], got %v, %v", empty, err)
+	}
+}
+
+func TestGitDiffAndGoDependencies(t *testing.T) {
+	root := t.TempDir()
+	writeFixture(t, root, "go.mod", "module example.test/integration\n\ngo 1.24\n")
+	writeFixture(t, root, "plugins/shared/shared.go", "package shared\nconst Value = 1\n")
+	writeFixture(t, root, "plugins/alpha/test_integration_test.go", "package alpha\nimport (\"testing\"; \"example.test/integration/plugins/shared\")\nfunc TestIntegration(t *testing.T) { _ = shared.Value }\n")
+	runGit := func(args ...string) {
+		t.Helper()
+		if _, err := commandOutput(root, "git", args...); err != nil {
+			t.Fatal(err)
+		}
+	}
+	runGit("init", "--quiet")
+	commit := func() {
+		runGit("add", ".")
+		runGit("-c", "user.name=Integration Test", "-c", "user.email=integration@example.invalid", "-c", "commit.gpgsign=false", "commit", "--quiet", "-m", "fixture")
+	}
+	oldPath := "plugins/shared/old space\nand newline.txt"
+	newPath := "plugins/shared/new.txt"
+	writeFixture(t, root, oldPath, "fixture")
+	commit()
+	runGit("checkout", "--quiet", "-b", "integration-change")
+	if err := os.Rename(filepath.Join(root, oldPath), filepath.Join(root, newPath)); err != nil {
+		t.Fatal(err)
+	}
+	commit()
+	writeFixture(t, root, "README.md", "A later commit changes only documentation.")
+	commit()
+	runGit("checkout", "--quiet", "-b", "base-fixture", "HEAD~2")
+	runGit("-c", "user.name=Integration Test", "-c", "user.email=integration@example.invalid", "-c", "commit.gpgsign=false", "merge", "--quiet", "--no-ff", "-m", "merge fixture", "integration-change")
+	changed, err := changedFiles(root, "HEAD^1", "HEAD")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(changed) != 3 {
+		t.Fatalf("merge comparison must include both rename paths and the later documentation change: %q", changed)
+	}
+	seen := make(map[string]bool)
+	for _, path := range changed {
+		seen[path] = true
+	}
+	if !seen[oldPath] || !seen[newPath] {
+		t.Fatalf("NUL-delimited paths were corrupted: %q", changed)
+	}
+	dirs, err := dependencyDirectories(root, "alpha")
+	if err != nil {
+		t.Fatal(err)
+	}
+	selected := selectPlugins([]string{"alpha"}, map[string][]string{"alpha": dirs}, changed)
+	if !reflect.DeepEqual(selected, []string{"alpha"}) {
+		t.Fatalf("real test dependency was missed: %v", dirs)
+	}
+	if _, err := changedFiles(root, "does-not-exist", "HEAD"); err == nil {
+		t.Fatal("missing base must fail, not silently skip tests")
+	}
+	if _, err := dependencyDirectories(root, "does-not-exist"); err == nil {
+		t.Fatal("broken dependency lookup must fail")
+	}
+}