-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ci: streamline Rocky dependencies and improve build failure diagnostics #6961
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
Open
3for
wants to merge
14
commits into
tronprotocol:develop
Choose a base branch
from
3for:fix/ci-gradle-only
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,754
−1,466
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
a8e2f8f
ci: slim down Rocky Linux build dependencies
3for df37173
ci: upload unit test logs and JUnit reports for failure diagnosis
3for c26cf99
ci: remove deleted multinode workflow from PR cancellation list
3for c21e168
ci: capture Gradle console logs and upload HTML test reports
3for f2a986a
test: remove automatic retries from framework tests
3for 15ef3ea
ci: upload only essential logs on failure
3for 28a108c
ci: increase Rocky download concurrency and retain retry config comments
3for 500ba48
fix: resolve CI test state leaks and backup shutdown race
3for d7b3c12
test: fix VM snapshot leaks and shielded test block production races
3for 3ca9945
test: fix flaky tests, hidden failures and resource leaks
3for 39256d3
test: improve CI stability through state isolation and resource cleanup
3for 8866306
test: isolate failure fixtures and stabilize transaction handler tests
3for e8b8878
test: eliminate order dependencies and improve CI failure diagnostics
3for 7efa49c
test: isolate event plugin state and assert metric deltas
3for File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
35 changes: 35 additions & 0 deletions
35
framework/src/test/java/com/google/common/util/concurrent/FakeTimeRateLimiter.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package com.google.common.util.concurrent; | ||
|
|
||
| import org.tron.common.math.StrictMathWrapper; | ||
|
|
||
| /** Test-only clock for exercising real Guava permit accounting without wall-clock sleeps. */ | ||
| public final class FakeTimeRateLimiter { | ||
|
|
||
| private FakeTimeRateLimiter() { | ||
| } | ||
|
|
||
| public static RateLimiter create(double permitsPerSecond) { | ||
| return RateLimiter.create(permitsPerSecond, new Stopwatch()); | ||
| } | ||
|
|
||
| public static RateLimiter createWithStoredPermit(double permitsPerSecond) { | ||
| Stopwatch clock = new Stopwatch(); | ||
| RateLimiter limiter = RateLimiter.create(permitsPerSecond, clock); | ||
| clock.sleepMicrosUninterruptibly((long) StrictMathWrapper.ceil(1_000_000 / permitsPerSecond)); | ||
| return limiter; | ||
| } | ||
|
|
||
| private static final class Stopwatch extends RateLimiter.SleepingStopwatch { | ||
| private long micros; | ||
|
|
||
| @Override | ||
| protected long readMicros() { | ||
| return micros; | ||
| } | ||
|
|
||
| @Override | ||
| protected void sleepMicrosUninterruptibly(long sleepMicros) { | ||
| micros += sleepMicros; | ||
| } | ||
| } | ||
| } |
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
31 changes: 31 additions & 0 deletions
31
framework/src/test/java/org/tron/common/BaseMethodTestLifecycleTest.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| package org.tron.common; | ||
|
|
||
| import org.junit.Assert; | ||
| import org.junit.Test; | ||
| import org.mockito.Mockito; | ||
| import org.tron.common.application.TronApplicationContext; | ||
| import org.tron.core.config.args.Args; | ||
|
|
||
| public class BaseMethodTestLifecycleTest { | ||
|
|
||
| @Test | ||
| public void closesContextWhenSubclassCleanupFails() { | ||
| BaseMethodTest fixture = new BaseMethodTest() { | ||
| @Override | ||
| protected void beforeDestroy() { | ||
| throw new IllegalStateException("intentional cleanup failure"); | ||
| } | ||
| }; | ||
| fixture.context = Mockito.mock(TronApplicationContext.class); | ||
| Args.setParam(new String[0], TestConstants.TEST_CONF); | ||
| try { | ||
| IllegalStateException failure = Assert.assertThrows(IllegalStateException.class, | ||
| fixture::destroyContext); | ||
| Assert.assertEquals("intentional cleanup failure", failure.getMessage()); | ||
| Mockito.verify(fixture.context).close(); | ||
| Assert.assertEquals(0, Args.getInstance().getHttpMaxMessageSize()); | ||
| } finally { | ||
| Args.clearParam(); | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
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.
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.
[Question] What's the motivation for making
channelvolatile here? Reading the surrounding code, this appears to fix a close-vs-bind race:close()can setshutdownwhilebind()is still in flight and observe a nullchannel, and the post-bind guard at L79–82 then closes the late-bound channel. Two questions:Is this race reachable in production, or only in tests? The new
BackupServerLifecycleTest#testCloseDuringBindsuggests the failing scenario came from the test side. If this production change is primarily to make unit tests pass, is that the right trade-off — or would fixing the test lifecycle be cleaner, or should this be split into a separatefix:PR with its own justification? It also isn't mentioned in the PR title/body, which currently only describes CI changes.If the cross-thread visibility fix is warranted, should
executor(~L35) be volatile as well? It follows the same pattern: written byinitServer(), read unsynchronized byclose(). A cross-threadclose()can still observe a stale-nullexecutorand skipshutdownAndAwaitTermination, leaking the non-daemon worker thread even though the channel is now closed.