Use BankForks on tests - Part 3 #34248
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #34248 +/- ##
=========================================
- Coverage 81.9% 81.9% -0.1%
=========================================
Files 819 819
Lines 219756 219741 -15
=========================================
- Hits 180115 180035 -80
- Misses 39641 39706 +65 |
runtime/src/bank.rs
Outdated
| @@ -966,7 +966,7 @@ pub(super) enum RewardInterval { | |||
| } | |||
|
|
|||
| impl Bank { | |||
| fn wrap_with_bank_forks_for_tests(self) -> (Arc<Self>, Arc<RwLock<BankForks>>) { | |||
| pub fn wrap_with_bank_forks_for_tests(self) -> (Arc<Self>, Arc<RwLock<BankForks>>) { | |||
There was a problem hiding this comment.
This PR seems like a good opportunity to move these public constructors/functions under DCOU. It'll be good cleanup for bank.rs.
There was a problem hiding this comment.
Let's use
solana/accounts-db/src/accounts_db.rs
Line 9549 in e1165aa
There was a problem hiding this comment.
Here in bank.rs, I can include this attribute in all functions whose name end with for_tests.
There was a problem hiding this comment.
How about define a new impl Bank {} and move all these test functions there?
There was a problem hiding this comment.
Yes, that's going to be cleaner.
There was a problem hiding this comment.
I have refactored many test functions, but not all. Some are indirectly called in tests through many other auxiliary functions, which made further alterations too intricate.
There was a problem hiding this comment.
Some functions with the for_test ending are not strictly under a test environment. program-test, for example, creates its own binary without a #[test] or a #[cfg(feature = "dev-context-only-utils")] attribute.
Problem
In order for us to implement #34169 (remove
WorkSlotfromLoadedPrograms::extract), we need to make sure all tests create aBankForkand use aBankinstance that has been added to the fork.Summary of Changes
This PR adds auxiliary functions to deal with
BankForksand fixes tests incore. This is the continuation of the work I started on #34234