Skip to content
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

Define PohRecorder set_bank related test helper methods #33626

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

ryoqun
Copy link
Member

@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
Member 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
Member 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
27 checks passed
@ryoqun
Copy link
Member 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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants