Add tests for store_account_and_update_capitalization#14008
Add tests for store_account_and_update_capitalization#14008ryoqun merged 1 commit intosolana-labs:masterfrom
Conversation
|
|
||
| #[cfg(test)] | ||
| fn add_account_and_update_capitalization(&self, pubkey: &Pubkey, new_account: &Account) { | ||
| fn store_account_and_update_capitalization(&self, pubkey: &Pubkey, new_account: &Account) { |
There was a problem hiding this comment.
Well, I renamed this to sound like a cousin to store_account. Introducing different verb add_ was a bad idea...
| } | ||
|
|
||
| #[test] | ||
| fn test_store_account_and_update_capitalization_missing() { |
There was a problem hiding this comment.
classical textbookish non-overlapping exhaustive shortly-chucked set of unit tests for store_account_and_update_capitalization, which will be newly promoted from cfg(test) at #13884 .
| assert_capitalization_diff( | ||
| &bank, | ||
| || bank.store_account_and_update_capitalization(&pubkey, &account), | ||
| |old, new| assert_eq!(old + 100, new), |
There was a problem hiding this comment.
trivia: Q: why do I this manual math hard code like this instead of old + (new_lamports - old_lamports) sometimes? Isn't a bad practice?
A: Well this is partly art of coding but these well nuanced hard coded math can catch really silly test failure. Say, I assigned same value to both old_lamports and new_lamports in a huste of refactoring by accident, then, old + (new_lamports - old_lamports) wouldn't detect that this test got broken meaning actually noting (including intended branch coverage in store_account_and_update_capitalization) is tested.
Also, note that these silly things could happen in the refactoring of cfg(not(test)) code.
|
@CriesofCarrots Could you review this at your convenient time? |
Codecov Report
@@ Coverage Diff @@
## master #14008 +/- ##
=========================================
- Coverage 82.1% 82.1% -0.1%
=========================================
Files 381 381
Lines 93727 93793 +66
=========================================
+ Hits 76975 77024 +49
- Misses 16752 16769 +17 |
(cherry picked from commit 28b014c)
3 participants
Problem
#13884 is too big. Let's split it.
Summary of Changes
Extracted pretty simple test additions and minor implementation modifications.