-
Couldn't load subscription status.
- Fork 1.6k
🌱 Skip E2E tests for documentation and YAML-only changes #5005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
🌱 Skip E2E tests for documentation and YAML-only changes #5005
Conversation
|
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Gautam3994 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Welcome @Gautam3994! |
|
Hi @Gautam3994. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
724f35d to
7338b6a
Compare
|
/ok-to-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you a lot for the nice work 🥇
We have an error here to solve see the job: https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_kubebuilder/5005/pull-kubebuilder-test/1955569142404747264
make[1]: Leaving directory '/home/prow/go/src/sigs.k8s.io/kubebuilder/docs/book/src/getting-started/testdata/project'
./test/check-license.sh
Checking for license header...
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
./test/check-license.sh: line 29: file: command not found
7338b6a to
c1bb6af
Compare
c1bb6af to
345cc41
Compare
|
@camilamacedo86 I noticed the missing file command in the build environment. I realize the guidelines recommend one issue per PR — should I open a separate PR for this fix, or is it okay to include it here since it’s a small change? |
|
Hi @camilamacedo86 just checking in — would you prefer I open a separate PR for the missing file command fix, or should I keep it in this one? |
|
@camilamacedo86 just checking in please let me know if anything else is required from my end |
|
Hi @Gautam3994 I had no time to review this one properly yet we need ensure that this changes will work fine and we will not skip when we should test |
| pattern := `(?i)(^docs/|\.md$|\.markdown$|^\.github/|` + | ||
| `(OWNERS|OWNERS_ALIASES|SECURITY_CONTACTS|LICENSE)(\.md)?$|\.ya?ml$)` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gautam3994 Sometimes we change workflows (which are .yaml files located in the .github dir) that have config that could possibly break the app and we would want to validate that in a full E2E test. Would this logic exclude those files?
|
@Gautam3994: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a conditional test execution mechanism to skip E2E test suites when changes are limited to documentation and YAML files, improving CI efficiency. The implementation adds a reusable utility for detecting file changes and determining whether tests should run based on configurable criteria.
Key changes:
- New
suite_filter.goutility that detects changed files from CI environment variables or git diff - Integration with alpha E2E test suites (
alphagenerateandalphaupdate) to skip execution when changes are outside relevant directories - Support for both CI and local development workflows with automatic fallback logic
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/e2e/utils/suite_filter.go | Core utility implementing change detection logic with support for regex/prefix matching and docs/YAML filtering |
| test/e2e/alphaupdate/e2e_suite_test.go | Adds BeforeSuite hook to skip suite when changes are outside pkg/cli/alpha/ or test/e2e/alphaupdate/ |
| test/e2e/alphagenerate/e2e_suite_test.go | Adds BeforeSuite hook to skip suite when changes are outside pkg/cli/alpha/ or test/e2e/alphagenerate/ |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| var _ = BeforeSuite(func() { | ||
| run, why, _ := utils.ShouldRun(utils.Options{ | ||
| RepoRoot: ".", |
Copilot
AI
Oct 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RepoRoot is hardcoded to . which assumes the test is always run from the repository root. If tests are executed from a different directory, this will fail. Consider using a relative path calculation or documenting this assumption clearly.
|
|
||
| var _ = BeforeSuite(func() { | ||
| run, why, _ := utils.ShouldRun(utils.Options{ | ||
| RepoRoot: ".", |
Copilot
AI
Oct 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RepoRoot is hardcoded to . which assumes the test is always run from the repository root. If tests are executed from a different directory, this will fail. Consider using a relative path calculation or documenting this assumption clearly.
| return false, "no files matched include prefixes", nil | ||
| } | ||
|
|
||
| func onlyDocsOrYAML(files []string) bool { |
Copilot
AI
Oct 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The complex regex pattern for detecting docs/YAML files lacks explanation. Add a comment documenting what file types are considered documentation-only (e.g., docs/, *.md, .github/, OWNERS files, .yml/.yaml).
| func onlyDocsOrYAML(files []string) bool { | |
| func onlyDocsOrYAML(files []string) bool { | |
| // Documentation-only files are considered to be: | |
| // - Any file under the docs/ directory | |
| // - Markdown files (*.md, *.markdown) | |
| // - Any file under the .github/ directory | |
| // - Special project files: OWNERS, OWNERS_ALIASES, SECURITY_CONTACTS, LICENSE (optionally with .md extension) | |
| // - YAML files (*.yml, *.yaml) | |
| // The following regex matches all of the above (case-insensitive): |
Issue: Proposal: Speed up E2E tests by skipping suites when changes are irrelevant - #5004
Problem
Currently, all E2E tests run even if a PR only modifies documentation (
*.md,*.markdown) or YAML (*.yml,*.yaml) files. This results in unnecessary CI resource usage and slower feedback cycles.Goal
alphagenerateandalphaupdate) only if changes are withinpkg/cli/alpha/.Proposed Solution
test/e2e/utilsthat:git diff.BeforeSuitehook andSkipitself when appropriate.This improves CI efficiency by avoiding needless E2E runs, speeding up feedback without sacrificing test coverage.