From b191f20843e474742d7a7d6b5fccd55b5a7684c2 Mon Sep 17 00:00:00 2001 From: balkashynov Date: Wed, 14 Oct 2026 16:43:05 -0500 Subject: [PATCH] ci: run opt-in integration tests for affected plugins Select affected plugin integration tests and document the shared opt-in convention. Retain the existing CockroachDB opt-in for compatibility. --- .github/workflows/integration.yaml | 74 ++++++++ CONTRIBUTING.md | 10 ++ cmd/ci/integration-matrix/main.go | 170 ++++++++++++++++++ cmd/ci/integration-matrix/main_test.go | 136 ++++++++++++++ .../database_credentials_integration_test.go | 6 +- 5 files changed, 393 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/integration.yaml create mode 100644 cmd/ci/integration-matrix/main.go create mode 100644 cmd/ci/integration-matrix/main_test.go diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml new file mode 100644 index 000000000..da5231641 --- /dev/null +++ b/.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" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1971c44d1..ff69acb83 100644 --- a/CONTRIBUTING.md +++ b/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/`. + +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. + ## 👷 Makefile Commands diff --git a/cmd/ci/integration-matrix/main.go b/cmd/ci/integration-matrix/main.go new file mode 100644 index 000000000..dc8d55ea7 --- /dev/null +++ b/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 +} diff --git a/cmd/ci/integration-matrix/main_test.go b/cmd/ci/integration-matrix/main_test.go new file mode 100644 index 000000000..74d876bcb --- /dev/null +++ b/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") + } +} diff --git a/plugins/cockroachdb/database_credentials_integration_test.go b/plugins/cockroachdb/database_credentials_integration_test.go index dc2fc06ba..1f51767d0 100644 --- a/plugins/cockroachdb/database_credentials_integration_test.go +++ b/plugins/cockroachdb/database_credentials_integration_test.go @@ -13,10 +13,10 @@ import ( "github.com/1Password/shell-plugins/sdk/schema/fieldname" ) -// Run with COCKROACHDB_INTEGRATION=1 make test. Docker pulls the pinned image if needed. +// Run with SHELL_PLUGINS_INTEGRATION=1 make test. Docker pulls the pinned image if needed. func TestDatabaseCredentialsIntegration(t *testing.T) { - if os.Getenv("COCKROACHDB_INTEGRATION") != "1" { - t.Skip("Set COCKROACHDB_INTEGRATION=1 to test against CockroachDB in Docker") + if os.Getenv("SHELL_PLUGINS_INTEGRATION") != "1" && os.Getenv("COCKROACHDB_INTEGRATION") != "1" { + t.Skip("Set SHELL_PLUGINS_INTEGRATION=1 to test against CockroachDB in Docker") } for _, secure := range []bool{false, true} {