Skip to content

ci: skip -race on integration tests #12994

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ci: skip -race on integration tests #12994

wants to merge 1 commit into from

Conversation

@howardjohn
Copy link
Contributor

@howardjohn howardjohn commented Nov 26, 2025 •
edited
Loading

Description

This requires building the codebase twice, and provides no value as the only code executed is test code.

See https://blog.howardjohn.info/posts/go-build-times/#caching-tests

From sample size=1, old vs new times for e2e tests

pass    13m45s  fail    8m12s 
pass    13m52s  pass    12m11s
pass    17m0s   pass    12m43s
pass    19m15s  pass    14m40s
pass    20m31s  pass    17m54s
pass    22m43s  pass    18m15s
pass    22m2s   pass    20m25s
pass    24m21s  pass    22m38s
pass    26m24s  pass    22m42s
pass    26m8s   pass    22m37s

Change Type

/kind fix

Changelog

NONE

Additional Notes

Copilot AI review requested due to automatic review settings November 26, 2025 17:15
@gateway-bot gateway-bot added kind/fix Categorizes issue or PR as related to a bug. release-note-none labels Nov 26, 2025
Copilot finished reviewing on behalf of howardjohn November 26, 2025 17:16
Copilot AI reviewed Nov 26, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a 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 optimizes CI build times by removing the -race flag from e2e (end-to-end) integration tests. The rationale is that running race detection on integration tests requires building the codebase twice but provides minimal value since only test code is executed.

Key Changes:

  • Introduced E2E_GO_TEST_ARGS variable containing common test flags without -race
  • Refactored GO_TEST_ARGS to include E2E_GO_TEST_ARGS plus -race for unit tests
  • Modified e2e-test target to use dependencies and target-specific variables instead of inline commands

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Makefile Outdated
Comment on lines 187 to 188
#e2e-test: ## Run only e2e tests, and only run the test package at {TEST_PKG} if it is specified
@:
Copy link

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commented-out line and the no-op command (@:) appear to be remnants from refactoring. Since the e2e-test target now has go-test as a dependency (line 183) and sets TEST_TAG and GO_TEST_ARGS via target-specific variables (lines 184-185), this commented line and no-op command should be removed entirely. The target will execute properly through its dependencies without needing any recipe commands.

Suggested change
#e2e-test: ## Run only e2e tests, and only run the test package at {TEST_PKG} if it is specified
@:

Copilot uses AI. Check for mistakes.
@howardjohn
ci: skip -race on integration tests
84c286c 
This requires building the codebase twice, and provides no value as the only code executed is test code.

See https://blog.howardjohn.info/posts/go-build-times/#caching-tests

Signed-off-by: John Howard <john.howard@solo.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@timflannagan timflannagan

Labels

kind/fix Categorizes issue or PR as related to a bug. release-note-none

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@howardjohn @timflannagan @gateway-bot