Skip to content

Proposal: Speed up E2E tests by skipping suites when changes are irrelevant #5004

@camilamacedo86

Description

@camilamacedo86

Problem

Right now, our E2E tests run even when a PR only changes documentation or YAML files.
This wastes CI time and slows down feedback.

Goal

  • Skip all E2E tests if a PR changes only:
    • Documentation files (*.md, .markdown, and yml or yaml, etc.)
    • YAML files (*.yml, *.yaml) — case-insensitive
  • Run alpha E2E suites (alphagenerate, alphaupdate) only if changes are in:
    • pkg/cli/alpha/
  • Let each suite decide for itself whether to run, so the logic is per-suite, atomic, and works locally and in CI.

Proposed Solution

We add a small Go helper in test/e2e/utils to:

  1. Get the list of changed files from CI env vars or from git diff.
  2. Check if the changes are only docs/YAML.
  3. Check if any changed file matches a list of required paths for a suite.

Each E2E suite calls this helper in a BeforeSuite and skips itself if not needed.

The following code example was generated assisted by AI to help understand how it should be.

## Helper: test/e2e/utils/changes.go
package utils

import (
	"bytes"
	"fmt"
	"os"
	"os/exec"
	"path/filepath"
	"regexp"
	"strings"
)

type Options struct {
	RepoRoot           string
	Include            []string
	IncludeIsRegex     bool
	SkipIfOnlyDocsYAML bool
	BaseEnvVar         string
	HeadEnvVar         string
	ChangedFilesEnvVar string
}

type changed struct{ files []string }

func ShouldRun(opts Options) (bool, string, error) {
	if opts.RepoRoot == "" {
		opts.RepoRoot = "."
	}
	if opts.BaseEnvVar == "" {
		opts.BaseEnvVar = "PULL_BASE_SHA"
	}
	if opts.HeadEnvVar == "" {
		opts.HeadEnvVar = "PULL_PULL_SHA"
	}
	if opts.ChangedFilesEnvVar == "" {
		opts.ChangedFilesEnvVar = "KUBEBUILDER_CHANGED_FILES"
	}

	if raw := os.Getenv(opts.ChangedFilesEnvVar); strings.TrimSpace(raw) != "" {
		return decide(parseChanged(raw), opts)
	}

	base := os.Getenv(opts.BaseEnvVar)
	head := os.Getenv(opts.HeadEnvVar)
	if head == "" {
		head = "HEAD"
	}

	cwd, _ := os.Getwd()
	defer os.Chdir(cwd)
	_ = os.Chdir(opts.RepoRoot)

	if base == "" {
		_ = exec.Command("git", "fetch", "origin", "master", "--quiet").Run()
		if out, err := exec.Command("git", "rev-parse", "--verify", "--quiet", "origin/master").CombinedOutput(); err == nil && len(bytes.TrimSpace(out)) > 0 {
			if mb, err := exec.Command("git", "merge-base", head, "origin/master").Output(); err == nil {
				base = strings.TrimSpace(string(mb))
			}
		}
		if base == "" {
			base = head + "~1"
		}
	}

	out, err := exec.Command("git", "diff", "--name-only", base, head).Output()
	if err != nil {
		out, err = exec.Command("git", "diff", "--name-only", head+"~1", head).Output()
		if err != nil {
			return true, "diff failed; default to run", fmt.Errorf("diff: %w", err)
		}
	}

	return decide(parseChanged(string(out)), opts)
}

func parseChanged(raw string) changed {
	lines := strings.Split(strings.TrimSpace(raw), "\n")
	files := make([]string, 0, len(lines))
	for _, l := range lines {
		l = strings.TrimSpace(l)
		if l == "" {
			continue
		}
		files = append(files, filepath.ToSlash(l))
	}
	return changed{files: files}
}

func decide(ch changed, opts Options) (bool, string, error) {
	if len(ch.files) == 0 {
		return true, "no changes; run", nil
	}

	if opts.SkipIfOnlyDocsYAML && onlyDocsOrYAML(ch.files) {
		return false, "only docs/yaml changed", nil
	}

	if len(opts.Include) == 0 {
		return true, "no include filter; run", nil
	}

	if opts.IncludeIsRegex {
		re := regexp.MustCompile("^(" + strings.Join(opts.Include, "|") + ")")
		for _, f := range ch.files {
			if re.MatchString(f) {
				return true, "matched " + re.String(), nil
			}
		}
		return false, "no regex include matched", nil
	}

	for _, f := range ch.files {
		for _, p := range opts.Include {
			if strings.HasPrefix(f, p) {
				return true, "matched prefix " + p, nil
			}
		}
	}
	return false, "no include prefixes matched", nil
}

func onlyDocsOrYAML(files []string) bool {
	re := regexp.MustCompile(`(?i)(^docs/|\.md$|\.markdown$|^\.github/|^(CHANGELOG|CODEOWNERS|CONTRIBUTING|LICENSE)(\.md)?$|\.ya?ml$)`)
	for _, f := range files {
		if !re.MatchString(f) {
			return false
		}
	}
	return true


}

How would that be called

## Alpha Update Suite: test/e2e/alphaupdate/e2e_suite_test.go
package alphaupdate

import (
	"testing"

	. "github.com/onsi/ginkgo/v2"
	. "github.com/onsi/gomega"

	"sigs.k8s.io/kubebuilder/v4/test/e2e/utils"
)

func TestE2E(t *testing.T) {
	RegisterFailHandler(Fail)
	_, _ = GinkgoWriter.Write([]byte("Starting alpha update e2e\n"))
	RunSpecs(t, "Alpha Update E2E")
}

var _ = BeforeSuite(func() {
	run, why, _ := utils.ShouldRun(utils.Options{
		RepoRoot:           ".",
		Include:            []string{"pkg/cli/alpha/"},
		SkipIfOnlyDocsYAML: true,
	})
	if !run {
		Skip("skip: " + why)
	}
})

## Alpha Generate Suite: test/e2e/alphagenerate/e2e_suite_test.go
package alphagenerate

import (
	"testing"

	. "github.com/onsi/ginkgo/v2"
	. "github.com/onsi/gomega"

	"sigs.k8s.io/kubebuilder/v4/test/e2e/utils"
)

func TestE2E(t *testing.T) {
	RegisterFailHandler(Fail)
	_, _ = GinkgoWriter.Write([]byte("Starting alpha generate e2e\n"))
	RunSpecs(t, "Alpha Generate E2E")
}

var _ = BeforeSuite(func() {
	run, why, _ := utils.ShouldRun(utils.Options{
		RepoRoot:           ".",
		Include:            []string{"pkg/cli/alpha/"},
		SkipIfOnlyDocsYAML: true,
	})
	if !run {
		Skip("skip: " + why)
	}
})

Metadata

Metadata

Assignees

Labels

good first issueDenotes an issue ready for a new contributor, according to the "help wanted" guidelines.size/MDenotes a PR that changes 30-99 lines, ignoring generated files.testing

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions