-
Notifications
You must be signed in to change notification settings - Fork 997
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
Removed active count and shuffling cache #4266
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4266 +/- ##
==========================================
+ Coverage 9.94% 42.45% +32.5%
==========================================
Files 76 50 -26
Lines 5109 3550 -1559
==========================================
+ Hits 508 1507 +999
+ Misses 4538 1844 -2694
- Partials 63 199 +136 |
shared/testutil/block_test.go
Outdated
@@ -45,7 +44,7 @@ func TestGenerateFullBlock_ThousandValidators(t *testing.T) { | |||
} | |||
|
|||
func TestGenerateFullBlock_Passes4Epochs(t *testing.T) { | |||
helpers.ClearAllCaches() | |||
|
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.
Are these empty lines at the start of the function intentional? I think we usually don't have an empty line at the start of a function.
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.
I agree with you, the new lines show inconsistency. Fixed in the latest commit
@@ -1299,7 +1274,6 @@ func TestVerifyIndexedAttestation_OK(t *testing.T) { | |||
} | |||
|
|||
for _, tt := range tests { | |||
helpers.ClearAllCaches() | |||
|
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.
Can you remove this line also? Or is putting a empty line after the for loop usual? I see it done sometimes, sometimes not. I prefer it without the empty line.
@@ -107,7 +107,6 @@ func TestUnslashedAttestingIndices_DuplicatedAttestations(t *testing.T) { | |||
} | |||
|
|||
func TestAttestingBalance_CorrectBalance(t *testing.T) { | |||
helpers.ClearAllCaches() | |||
|
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.
This one also
@@ -1216,7 +1194,6 @@ func TestConvertToIndexed_OK(t *testing.T) { | |||
}, | |||
} | |||
for _, tt := range tests { | |||
helpers.ClearAllCaches() | |||
|
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.
Here too
@@ -315,7 +310,7 @@ func TestBaseReward_AccurateRewards(t *testing.T) { | |||
{40 * 1e9, params.BeaconConfig().MaxEffectiveBalance, 2862174}, | |||
} | |||
for _, tt := range tests { | |||
helpers.ClearAllCaches() | |||
|
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.
Here as well please
@@ -97,7 +96,6 @@ func TestProcessSlashingsPrecompute_SlashedLess(t *testing.T) { | |||
|
|||
for i, tt := range tests { | |||
t.Run(string(i), func(t *testing.T) { | |||
helpers.ClearAllCaches() | |||
|
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.
Here as well
@@ -100,7 +99,6 @@ func TestExecuteStateTransition_FullProcess(t *testing.T) { | |||
} | |||
|
|||
func TestProcessBlock_IncorrectProposerSlashing(t *testing.T) { | |||
helpers.ClearAllCaches() | |||
|
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.
Missing a few around here
shared/testutil/helpers_test.go
Outdated
@@ -11,7 +11,6 @@ import ( | |||
) | |||
|
|||
func TestBlockSignature(t *testing.T) { | |||
helpers.ClearAllCaches() | |||
|
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.
And here
@@ -419,7 +411,6 @@ func TestProcessSlashings_SlashedLess(t *testing.T) { | |||
|
|||
for i, tt := range tests { | |||
t.Run(string(i), func(t *testing.T) { | |||
helpers.ClearAllCaches() | |||
|
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.
Last one (?)
@@ -37,7 +37,6 @@ func TestExecuteStateTransition_IncorrectSlot(t *testing.T) { | |||
} | |||
|
|||
func TestExecuteStateTransition_FullProcess(t *testing.T) { | |||
helpers.ClearAllCaches() | |||
|
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.
This one
@@ -181,7 +178,6 @@ func TestProcessBlock_IncorrectProcessBlockAttestations(t *testing.T) { | |||
} | |||
|
|||
func TestProcessBlock_IncorrectProcessExits(t *testing.T) { | |||
helpers.ClearAllCaches() | |||
|
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.
Over here
Please deprecate the feature flags. |
* Removed * All tests pass * Gaz * Removed new lines * A few more lines... * I think i got them all * and I didnt : ) * Could this be last...
* Removed * All tests pass * Gaz * Removed new lines * A few more lines... * I think i got them all * and I didnt : ) * Could this be last...
Following #4264
This PR removed unused
active count cache
andshuffling cache
Change list:
helpers.ClearAllCaches()
, it's no longer needed