Stabilize flaky fault-tolerance build test by waiting for InvocationStat initialization - #1574
Open
sunhailin-Leo with Copilot wants to merge 12 commits into
Open
Stabilize flaky fault-tolerance build test by waiting for InvocationStat initialization#1574sunhailin-Leo with Copilot wants to merge 12 commits into
sunhailin-Leo with Copilot wants to merge 12 commits into
Conversation
Copilot
AI
changed the title
[WIP] Fix failing GitHub Actions job build
Stabilize flaky fault-tolerance build test by waiting for InvocationStat initialization
Jul 22, 2026
sunhailin-Leo
approved these changes
Jul 23, 2026
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce CI flakiness by hardening timing-sensitive integration/unit tests and adjusting build/CI settings so tests behave more deterministically.
Changes:
- Hardened fault-tolerance test
ServiceHorizontalRegulationStrategyTest#testAllwith bounded waits for transport/stat initialization. - Improved cleanup and startup stability in several integration/registry tests (consumer
unRefer(), Consul startup wait + null-safe teardown). - Updated build execution and Maven Surefire configuration (Surefire version bump, single-fork config, GitHub Actions workflow consolidation).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test-integration/src/test/java/com/alipay/sofa/rpc/triple/ark/MultiClassLoaderTest.java | Adds explicit consumer unRefer() under the intended context classloader to improve test cleanup. |
| test/test-integration/src/test/java/com/alipay/sofa/rpc/test/triple/TripleServerTest.java | Recreates consumer after a failure scenario to continue assertions; needs careful cleanup. |
| registry/registry-sofa/pom.xml | Changes test exclusion pattern for specific JDK profiles. |
| registry/registry-consul/src/test/java/com/alipay/sofa/rpc/registry/consul/ConsulRegistryTest.java | Adds Consul startup wait timeout and null-safe teardown. |
| registry/registry-consul/src/test/java/com/alipay/sofa/rpc/registry/consul/ConsulRegistryAclTest.java | Same as above, for ACL variant. |
| pom.xml | Upgrades Surefire plugin and pins fork behavior to improve determinism. |
| fault/fault-tolerance/src/test/java/com/alipay/sofa/rpc/client/aft/ServiceHorizontalRegulationStrategyTest.java | Adds retry logic for connection availability and InvocationStat initialization to reduce flakiness. |
| fault/fault-tolerance/src/test/java/com/alipay/sofa/rpc/client/aft/FaultBaseTest.java | Adjusts consumer reconnect period used by all fault-tolerance tests. |
| .github/workflows/maven.yml | Consolidates CI steps into a single clean verify run plus formatting check. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
48
to
+52
| final ProviderInfo providerInfo = getProviderInfoByHost(consumerConfig, "127.0.0.1"); | ||
| final InvocationStatDimension statDimension = new InvocationStatDimension(providerInfo, consumerConfig); | ||
| InvocationStat invocationStat = InvocationStatFactory.ALL_STATS.get(statDimension); | ||
| Assert.assertNotNull(invocationStat); | ||
| final int maxConnectionRetryAttempts = 50; | ||
| final int maxRetryAttempts = 10; | ||
| final int retryDelayMillis = 100; |
Comment on lines
+435
to
+439
| consumerConfig.unRefer(); | ||
| consumerConfig = new ConsumerConfig<>(); | ||
| consumerConfig.setInterfaceId(SampleService.class.getName()) | ||
| .setProtocol(RpcConstants.PROTOCOL_TYPE_TRIPLE) | ||
| .setDirectUrl("tri://127.0.0.1:" + port); |
Comment on lines
91
to
95
| consumerConfig = new ConsumerConfig<FaultHelloService>() | ||
| .setInterfaceId(FaultHelloService.class.getName()) | ||
| .setTimeout(500) | ||
| .setReconnectPeriod(2000) | ||
| .setDirectUrl("127.0.0.1:12299") |
sunhailin-Leo
approved these changes
Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
buildworkflow was failing insofa-rpc-fault-tolerancedue to a flaky assertion inServiceHorizontalRegulationStrategyTest#testAll. The test assumedInvocationStatexists immediately after a single timeout call, which is not always true under transient startup/connection timing in CI.Root-cause alignment
testAllcould fail when provider connection initialization lagged andInvocationStatFactory.ALL_STATS.get(...)returnednullon first check.Targeted test hardening
maxRetryAttempts,retryDelayMillis) for readability and maintainability.Failure diagnostics