-
Notifications
You must be signed in to change notification settings - Fork 337
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
accounts db/refactor accounts db test - add macro for accounts-db panic test #786
accounts db/refactor accounts db test - add macro for accounts-db panic test #786
Conversation
The actual change is in the last commit. |
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.
Looks good. Please re-request a review once the previous PR has merged and this PR has been rebased.
#[test_case(AccountsFileProvider::AppendVec; "append_vec")] | ||
#[test_case(AccountsFileProvider::HotStorage; "hot_storage")] | ||
fn $name(accounts_file_provider: AccountsFileProvider) { | ||
(@testfn $name:ident, $accounts_file_provider: ident, |$accounts_db:ident| $inner: tt) => { |
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.
Nice 👍
convert double remove test for both account file provider
97c1b2e
to
4b17876
Compare
No code change. Rebase to master to pick up #784. |
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.
@jeffwashington Can you help to review this PR? Thanks! |
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.
lgtm
How many tests panic?
There are 9 more tests that expect panics. |
…ic test (anza-xyz#786) * add macro for accounts-db panic test convert double remove test for both account file provider * refactor to share accounts_db_test and accounts_db_panic_test macro * rebase --------- Co-authored-by: HaoranYi <[email protected]>
Problem
add macro for accounts-db panic test
Continued on #784
Summary of Changes
Fixes #