Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Define PohRecorder set_bank related test helper methods #33626

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

ryoqun
Copy link
Contributor

@ryoqun ryoqun commented Oct 10, 2023

Problem

my scheduler thing needs to introduce rather invasive code changes including the BankWithScheduler wrapping, which touches poh recorder code.

Summary of Changes

Prepare some helper functions.

extracted from: #33070

note that this pr is quite similar to the previous prep. pr: #33537

@ryoqun ryoqun requested a review from apfitzge October 10, 2023 15:05
@ryoqun ryoqun changed the title Define PohRecorder set_bank related test methods Define PohRecorder set_bank related test helper methods Oct 10, 2023
@@ -642,6 +642,16 @@ impl PohRecorder {
let _ = self.flush_cache(false);
}

#[cfg(feature = "dev-context-only-utils")]
pub fn set_bank_for_test(&mut self, bank: Arc<Bank>) {
self.set_bank(bank, false)
Copy link
Contributor Author

@ryoqun ryoqun Oct 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i eliminated these falses at all call-sites as a bonus.

}

#[cfg(test)]
pub fn set_bank_with_transaction_index_for_test(&mut self, bank: Arc<Bank>) {
Copy link
Contributor Author

@ryoqun ryoqun Oct 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi, this is only used once.... another justification for hiding the track_transaction_indexes bool arg at this wrapper...

Copy link
Contributor

@apfitzge apfitzge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ryoqun ryoqun merged commit 2f090a5 into solana-labs:master Oct 11, 2023
@ryoqun
Copy link
Contributor Author

ryoqun commented Oct 11, 2023

merged this knowing the ci failure is fixed in the master. rebase would need another full ci run and lgtm from someone. ;)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants