-
Notifications
You must be signed in to change notification settings - Fork 120
test(func): improve parallelism #10934
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: main
Are you sure you want to change the base?
test(func): improve parallelism #10934
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10934 +/- ##
==========================================
- Coverage 50.46% 50.43% -0.03%
==========================================
Files 672 672
Lines 42063 42063
==========================================
- Hits 21228 21216 -12
- Misses 18784 18790 +6
- Partials 2051 2057 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 improves functional test parallelism by introducing bounded package-level parallelism controls and replacing fixed sleep delays with dynamic polling loops.
Key changes include:
- Adding
-pflags to all functional test targets in the Makefile to control package-level parallelism (how many test packages run simultaneously) - Replacing
time.Sleep(3 * time.Second)withrequire.Eventuallypolling loops that actively check Helm release status - Minor whitespace cleanup (trailing spaces removed)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| build/test.mk | Added -p flags to control package-level parallelism: -p 1 for cloud/Kubernetes/upgrade suites to prevent rate-limiting and cluster contention; -p 2 for lighter noncloud suites to enable some parallelism; whitespace cleanup |
| test/functional-portable/upgrade/upgrade_test.go | Replaced fixed 3-second sleep delays with require.Eventually polling loops that check helm status to confirm release uninstallation, making tests faster and more reliable |
build/test.mk
Outdated
| .PHONY: test-functional-corerp-noncloud | ||
| test-functional-corerp-noncloud: ## Runs corerp functional tests that do not require cloud resources | ||
| CGO_ENABLED=1 $(GOTEST_TOOL) ./test/functional-portable/corerp/noncloud/... -timeout ${TEST_TIMEOUT} -v -parallel 10 $(GOTEST_OPTS) | ||
| CGO_ENABLED=1 $(GOTEST_TOOL) ./test/functional-portable/corerp/noncloud/... -timeout ${TEST_TIMEOUT} -v -parallel 10 -p 1 $(GOTEST_OPTS) |
Copilot
AI
Dec 12, 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 -p flag controls the number of test packages that can be built and run in parallel. However, corerp-noncloud is configured with -parallel 10 (allowing up to 10 tests within a package to run concurrently) but -p 1 (only one package at a time). This seems inconsistent with the stated goal of allowing "some parallelism" for lighter suites. Consider whether -p 2 would be more appropriate here to match other noncloud suites, unless there's a specific reason for this constraint.
| CGO_ENABLED=1 $(GOTEST_TOOL) ./test/functional-portable/corerp/noncloud/... -timeout ${TEST_TIMEOUT} -v -parallel 10 -p 1 $(GOTEST_OPTS) | |
| CGO_ENABLED=1 $(GOTEST_TOOL) ./test/functional-portable/corerp/noncloud/... -timeout ${TEST_TIMEOUT} -v -parallel 10 -p 2 $(GOTEST_OPTS) |
build/test.mk
Outdated
| .PHONY: test-functional-daprrp-noncloud | ||
| test-functional-daprrp-noncloud: ## Runs Dapr RP functional tests that do not require cloud resources | ||
| CGO_ENABLED=1 $(GOTEST_TOOL) ./test/functional-portable/daprrp/noncloud/... -timeout ${TEST_TIMEOUT} -v -parallel 3 $(GOTEST_OPTS) | ||
| CGO_ENABLED=1 $(GOTEST_TOOL) ./test/functional-portable/daprrp/noncloud/... -timeout ${TEST_TIMEOUT} -v -parallel 3 -p 1 $(GOTEST_OPTS) |
Copilot
AI
Dec 12, 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 -p flag controls package-level parallelism (how many test packages run simultaneously), while -parallel controls within-package parallelism (how many tests within a single package run concurrently). The daprrp-noncloud suite has -parallel 3 with -p 1, which seems inconsistent with the PR description stating lighter suites are "capped at -p 2 to allow some parallelism." Consider whether -p 2 would be more appropriate here.
| CGO_ENABLED=1 $(GOTEST_TOOL) ./test/functional-portable/daprrp/noncloud/... -timeout ${TEST_TIMEOUT} -v -parallel 3 -p 1 $(GOTEST_OPTS) | |
| CGO_ENABLED=1 $(GOTEST_TOOL) ./test/functional-portable/daprrp/noncloud/... -timeout ${TEST_TIMEOUT} -v -parallel 3 -p 2 $(GOTEST_OPTS) |
Signed-off-by: Dariusz Porowski <[email protected]>
… preflight tests Signed-off-by: Dariusz Porowski <[email protected]>
…m in test execution Signed-off-by: Dariusz Porowski <[email protected]>
47c55b8 to
5030b25
Compare
Signed-off-by: Dariusz Porowski <[email protected]>
Signed-off-by: Dariusz Porowski <[email protected]>
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
Description
Bounded Package Parallelism in
test.mk:Replaced time.Sleep(3 * time.Second) with require.Eventually polling loops that check helm status to confirm the release is actually gone. This makes the tests faster.
Type of change
Contributor checklist
Please verify that the PR meets the following requirements, where applicable: